Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid parsing with Salsa #13437

Merged
merged 3 commits into from
Sep 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/ruff_graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ruff_db = { workspace = true, features = ["os", "serde"] }
ruff_linter = { workspace = true }
ruff_macros = { workspace = true }
ruff_python_ast = { workspace = true }
ruff_python_parser = { workspace = true }

anyhow = { workspace = true }
clap = { workspace = true, optional = true }
Expand Down
10 changes: 6 additions & 4 deletions crates/ruff_graph/src/collector.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use red_knot_python_semantic::ModuleName;
use ruff_python_ast::visitor::source_order::{walk_body, walk_expr, walk_stmt, SourceOrderVisitor};
use ruff_python_ast::{self as ast, Expr, ModModule, Stmt};
use ruff_python_ast::visitor::source_order::{
walk_expr, walk_module, walk_stmt, SourceOrderVisitor,
};
use ruff_python_ast::{self as ast, Expr, Mod, Stmt};

/// Collect all imports for a given Python file.
#[derive(Default, Debug)]
Expand All @@ -23,8 +25,8 @@ impl<'a> Collector<'a> {
}

#[must_use]
pub(crate) fn collect(mut self, module: &ModModule) -> Vec<CollectedImport> {
walk_body(&mut self, &module.body);
pub(crate) fn collect(mut self, module: &Mod) -> Vec<CollectedImport> {
walk_module(&mut self, module);
self.imports
}
}
Expand Down
12 changes: 5 additions & 7 deletions crates/ruff_graph/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ pub use crate::db::ModuleDb;
use crate::resolver::Resolver;
pub use crate::settings::{AnalyzeSettings, Direction};
use anyhow::Result;
use red_knot_python_semantic::SemanticModel;
use ruff_db::files::system_path_to_file;
use ruff_db::parsed::parsed_module;
use ruff_db::system::{SystemPath, SystemPathBuf};
use ruff_python_ast::helpers::to_module_path;
use ruff_python_parser::{parse, Mode};
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet};

Expand All @@ -29,11 +27,11 @@ impl ModuleImports {
string_imports: bool,
) -> Result<Self> {
// Read and parse the source code.
let file = system_path_to_file(db, path)?;
let parsed = parsed_module(db, file);
let source = std::fs::read_to_string(path)?;
let parsed = parse(&source, Mode::Module)?;

let module_path =
package.and_then(|package| to_module_path(package.as_std_path(), path.as_std_path()));
let model = SemanticModel::new(db, file);

// Collect the imports.
let imports =
Expand All @@ -42,7 +40,7 @@ impl ModuleImports {
// Resolve the imports.
let mut resolved_imports = ModuleImports::default();
for import in imports {
let Some(resolved) = Resolver::new(&model).resolve(import) else {
let Some(resolved) = Resolver::new(db).resolve(import) else {
continue;
};
let Some(path) = resolved.as_system_path() else {
Expand Down
29 changes: 14 additions & 15 deletions crates/ruff_graph/src/resolver.rs
Original file line number Diff line number Diff line change
@@ -1,37 +1,36 @@
use red_knot_python_semantic::SemanticModel;
use red_knot_python_semantic::resolve_module;
use ruff_db::files::FilePath;

use crate::collector::CollectedImport;
use crate::ModuleDb;

/// Collect all imports for a given Python file.
pub(crate) struct Resolver<'a> {
semantic: &'a SemanticModel<'a>,
db: &'a ModuleDb,
}

impl<'a> Resolver<'a> {
/// Initialize a [`Resolver`] with a given [`SemanticModel`].
pub(crate) fn new(semantic: &'a SemanticModel<'a>) -> Self {
Self { semantic }
/// Initialize a [`Resolver`] with a given [`ModuleDb`].
pub(crate) fn new(db: &'a ModuleDb) -> Self {
Self { db }
}

/// Resolve the [`CollectedImport`] into a [`FilePath`].
pub(crate) fn resolve(&self, import: CollectedImport) -> Option<&'a FilePath> {
match import {
CollectedImport::Import(import) => self
.semantic
.resolve_module(import)
.map(|module| module.file().path(self.semantic.db())),
CollectedImport::Import(import) => {
resolve_module(self.db, import).map(|module| module.file().path(self.db))
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaReiser -- Would you prefer that I create a struct that exposes this? (I can't use SemanticModel because that requires a File, and I no longer have a File upfront.)

CollectedImport::ImportFrom(import) => {
// Attempt to resolve the member (e.g., given `from foo import bar`, look for `foo.bar`).
let parent = import.parent();
self.semantic
.resolve_module(import)
.map(|module| module.file().path(self.semantic.db()))

resolve_module(self.db, import)
.map(|module| module.file().path(self.db))
.or_else(|| {
// Attempt to resolve the module (e.g., given `from foo import bar`, look for `foo`).
self.semantic
.resolve_module(parent?)
.map(|module| module.file().path(self.semantic.db()))

resolve_module(self.db, parent?).map(|module| module.file().path(self.db))
})
}
}
Expand Down
Loading