Skip to content

Commit

Permalink
Use a single db
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Sep 20, 2024
1 parent c7ba18b commit 1c7e262
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 130 deletions.
30 changes: 10 additions & 20 deletions crates/ruff/src/commands/analyze_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use ruff_linter::{warn_user, warn_user_once};
use ruff_python_ast::{PySourceType, SourceType};
use ruff_workspace::resolver::{python_files_in_path, ResolvedFile};
use rustc_hash::FxHashMap;
use std::collections::BTreeMap;
use std::path::Path;
use std::sync::Arc;

Expand Down Expand Up @@ -53,13 +52,14 @@ pub(crate) fn analyze_graph(
.collect::<FxHashMap<_, _>>();

// Create a database for each source root.
let databases = package_roots
.values()
.filter_map(|package| package.as_deref())
.filter_map(|package| package.parent())
.map(Path::to_path_buf)
.map(|source_root| Ok((source_root.clone(), ModuleDb::from_src_root(source_root)?)))
.collect::<Result<BTreeMap<_, _>>>()?;
let db = ModuleDb::from_src_roots(
package_roots
.values()
.filter_map(|package| package.as_deref())
.filter_map(|package| package.parent())
.map(Path::to_path_buf)
.filter_map(|path| SystemPathBuf::from_path_buf(path).ok()),
)?;

// Collect and resolve the imports for each file.
let result = Arc::new(std::sync::Mutex::new(Vec::new()));
Expand All @@ -76,17 +76,6 @@ pub(crate) fn analyze_graph(
.parent()
.and_then(|parent| package_roots.get(parent))
.and_then(Clone::clone);
let Some(src_root) = package
.as_ref()
.and_then(|package| package.parent())
.map(Path::to_path_buf)
else {
debug!("Ignoring file outside of source root: {}", path.display());
continue;
};
let Some(db) = databases.get(&src_root).map(ModuleDb::snapshot) else {
continue;
};

// Resolve the per-file settings.
let settings = resolver.resolve(&path);
Expand Down Expand Up @@ -118,8 +107,9 @@ pub(crate) fn analyze_graph(
warn!("Failed to convert path to system path");
continue;
};
let root = root.clone();

let db = db.snapshot();
let root = root.clone();
let result = inner_result.clone();
scope.spawn(move |_| {
// Identify any imports via static analysis.
Expand Down
213 changes: 119 additions & 94 deletions crates/ruff/tests/analyze_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ fn command() -> Command {
command
}

const INSTA_FILTERS: &[(&str, &str)] = &[
// Rewrite Windows output to Unix output
(r"\\", "/"),
];

#[test]
fn dependencies() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down Expand Up @@ -51,29 +56,33 @@ fn dependencies() -> Result<()> {
def f(): pass
"#})?;

assert_cmd_snapshot!(command().current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [
"ruff/c.py"
],
"ruff/c.py": [
"ruff/d.py"
],
"ruff/d.py": [
"ruff/e.py"
],
"ruff/e.py": []
}
insta::with_settings!({
filters => INSTA_FILTERS.to_vec(),
}, {
assert_cmd_snapshot!(command().current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [
"ruff/c.py"
],
"ruff/c.py": [
"ruff/d.py"
],
"ruff/d.py": [
"ruff/e.py"
],
"ruff/e.py": []
}
----- stderr -----
"###);
----- stderr -----
"###);
});

Ok(())
}
Expand Down Expand Up @@ -111,29 +120,33 @@ fn dependents() -> Result<()> {
def f(): pass
"#})?;

assert_cmd_snapshot!(command().arg("--direction").arg("dependents").current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [],
"ruff/b.py": [
"ruff/a.py"
],
"ruff/c.py": [
"ruff/b.py"
],
"ruff/d.py": [
"ruff/c.py"
],
"ruff/e.py": [
"ruff/d.py"
]
}
insta::with_settings!({
filters => INSTA_FILTERS.to_vec(),
}, {
assert_cmd_snapshot!(command().arg("--direction").arg("dependents").current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [],
"ruff/b.py": [
"ruff/a.py"
],
"ruff/c.py": [
"ruff/b.py"
],
"ruff/d.py": [
"ruff/c.py"
],
"ruff/e.py": [
"ruff/d.py"
]
}
----- stderr -----
"###);
----- stderr -----
"###);
});

Ok(())
}
Expand All @@ -159,39 +172,47 @@ fn string_detection() -> Result<()> {
"#})?;
root.child("ruff").child("c.py").write_str("")?;

assert_cmd_snapshot!(command().current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [],
"ruff/c.py": []
}
insta::with_settings!({
filters => INSTA_FILTERS.to_vec(),
}, {
assert_cmd_snapshot!(command().current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [],
"ruff/c.py": []
}
----- stderr -----
"###);
----- stderr -----
"###);
});

assert_cmd_snapshot!(command().arg("--detect-string-imports").current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [
"ruff/c.py"
],
"ruff/c.py": []
}
insta::with_settings!({
filters => INSTA_FILTERS.to_vec(),
}, {
assert_cmd_snapshot!(command().arg("--detect-string-imports").current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [
"ruff/c.py"
],
"ruff/c.py": []
}
----- stderr -----
"###);
----- stderr -----
"###);
});

Ok(())
}
Expand All @@ -212,26 +233,30 @@ fn globs() -> Result<()> {
root.child("ruff").child("b.py").write_str("")?;
root.child("ruff").child("c.py").write_str("")?;

assert_cmd_snapshot!(command().current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [
"ruff/__init__.py",
"ruff/a.py",
"ruff/b.py",
"ruff/c.py"
],
"ruff/c.py": []
}
----- stderr -----
"###);
insta::with_settings!({
filters => INSTA_FILTERS.to_vec(),
}, {
assert_cmd_snapshot!(command().current_dir(&root), @r###"
success: true
exit_code: 0
----- stdout -----
{
"ruff/__init__.py": [],
"ruff/a.py": [
"ruff/b.py"
],
"ruff/b.py": [
"ruff/__init__.py",
"ruff/a.py",
"ruff/b.py",
"ruff/c.py"
],
"ruff/c.py": []
}
----- stderr -----
"###);
});

Ok(())
}
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,10 @@ formatter.magic_trailing_comma = respect
formatter.docstring_code_format = disabled
formatter.docstring_code_line_width = dynamic

# Import Map Settings
import_map.detect_string_imports = false
import_map.extension = ExtensionMapping({})
import_map.include_dependencies = {}
# Analyze Settings
analyze.preview = disabled
analyze.detect_string_imports = false
analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {}

----- stderr -----
26 changes: 20 additions & 6 deletions crates/ruff_graph/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use ruff_db::files::{File, Files};
use ruff_db::system::{OsSystem, System, SystemPathBuf};
use ruff_db::vendored::VendoredFileSystem;
use ruff_db::{Db as SourceDb, Upcast};
use std::path::PathBuf;

#[salsa::db]
#[derive(Default)]
Expand All @@ -17,21 +16,36 @@ pub struct ModuleDb {

impl ModuleDb {
/// Initialize a [`ModuleDb`] from the given source root.
pub fn from_src_root(src_root: PathBuf) -> Result<Self> {
pub fn from_src_roots(mut src_roots: impl Iterator<Item = SystemPathBuf>) -> Result<Self> {
let search_paths = {
// Use the first source root.
let src_root = src_roots
.next()
.ok_or_else(|| anyhow::anyhow!("No source roots provided"))?;

let mut search_paths = SearchPathSettings::new(src_root.to_path_buf());

// Add the remaining source roots as extra paths.
for src_root in src_roots {
search_paths.extra_paths.push(src_root.to_path_buf());
}

search_paths
};

let db = Self::default();
Program::from_settings(
&db,
&ProgramSettings {
target_version: PythonVersion::default(),
search_paths: SearchPathSettings::new(
SystemPathBuf::from_path_buf(src_root)
.map_err(|path| anyhow::anyhow!("Invalid path: {}", path.display()))?,
),
search_paths,
},
)?;

Ok(db)
}

/// Create a snapshot of the current database.
#[must_use]
pub fn snapshot(&self) -> Self {
Self {
Expand Down
Loading

0 comments on commit 1c7e262

Please sign in to comment.