diff --git a/Cargo.lock b/Cargo.lock index c820506c4c..074fd197a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2960,6 +2960,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", + "memchr", "ruff_cache", "ruff_db", "ruff_linter", diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 45db84d053..aef5280955 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -169,6 +169,9 @@ pub struct AnalyzeGraphCommand { /// Attempt to detect imports from string literals. #[clap(long)] detect_string_imports: bool, + /// The minimum number of dots in a string import to consider it a valid import. + #[clap(long)] + min_dots: Option, /// Enable preview mode. Use `--no-preview` to disable. #[arg(long, overrides_with("no_preview"))] preview: bool, @@ -808,6 +811,7 @@ impl AnalyzeGraphCommand { } else { None }, + string_imports_min_dots: self.min_dots, preview: resolve_bool_arg(self.preview, self.no_preview).map(PreviewMode::from), target_version: self.target_version.map(ast::PythonVersion::from), ..ExplicitConfigOverrides::default() @@ -1305,6 +1309,7 @@ struct ExplicitConfigOverrides { show_fixes: Option, extension: Option>, detect_string_imports: Option, + string_imports_min_dots: Option, } impl ConfigurationTransformer for ExplicitConfigOverrides { @@ -1392,6 +1397,9 @@ impl ConfigurationTransformer for ExplicitConfigOverrides { if let Some(detect_string_imports) = &self.detect_string_imports { config.analyze.detect_string_imports = Some(*detect_string_imports); } + if let Some(string_imports_min_dots) = &self.string_imports_min_dots { + config.analyze.string_imports_min_dots = Some(*string_imports_min_dots); + } config } diff --git a/crates/ruff/src/commands/analyze_graph.rs b/crates/ruff/src/commands/analyze_graph.rs index 75ff28d0dc..ffd7cc2d15 100644 --- a/crates/ruff/src/commands/analyze_graph.rs +++ b/crates/ruff/src/commands/analyze_graph.rs @@ -102,7 +102,7 @@ pub(crate) fn analyze_graph( // Resolve the per-file settings. let settings = resolver.resolve(path); - let string_imports = settings.analyze.detect_string_imports; + let string_imports = settings.analyze.string_imports; let include_dependencies = settings.analyze.include_dependencies.get(path).cloned(); // Skip excluded files. diff --git a/crates/ruff/tests/analyze_graph.rs b/crates/ruff/tests/analyze_graph.rs index 485aeb511b..437702a2a8 100644 --- a/crates/ruff/tests/analyze_graph.rs +++ b/crates/ruff/tests/analyze_graph.rs @@ -197,23 +197,43 @@ fn string_detection() -> Result<()> { 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": [] - } + 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": [] + } - ----- stderr ----- - "###); + ----- stderr ----- + "#); + }); + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec(), + }, { + assert_cmd_snapshot!(command().arg("--detect-string-imports").arg("--min-dots").arg("1").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 ----- + "#); }); Ok(()) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 5223f49536..40c1eb4ab7 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -2422,7 +2422,7 @@ requires-python = ">= 3.11" analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.11 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -2734,7 +2734,7 @@ requires-python = ">= 3.11" analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.10 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -3098,7 +3098,7 @@ from typing import Union;foo: Union[int, str] = 1 analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.11 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -3478,7 +3478,7 @@ from typing import Union;foo: Union[int, str] = 1 analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.11 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -3806,7 +3806,7 @@ from typing import Union;foo: Union[int, str] = 1 analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.10 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -4134,7 +4134,7 @@ from typing import Union;foo: Union[int, str] = 1 analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.9 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -4419,7 +4419,7 @@ from typing import Union;foo: Union[int, str] = 1 analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.9 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} @@ -4757,7 +4757,7 @@ from typing import Union;foo: Union[int, str] = 1 analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.10 - analyze.detect_string_imports = false + analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} diff --git a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap index 37e8eae6bc..cb94c10332 100644 --- a/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap +++ b/crates/ruff/tests/snapshots/show_settings__display_default_settings.snap @@ -392,7 +392,7 @@ formatter.docstring_code_line_width = dynamic analyze.exclude = [] analyze.preview = disabled analyze.target_version = 3.7 -analyze.detect_string_imports = false +analyze.string_imports = disabled analyze.extension = ExtensionMapping({}) analyze.include_dependencies = {} diff --git a/crates/ruff_graph/Cargo.toml b/crates/ruff_graph/Cargo.toml index 917a2f4e05..7fb1470270 100644 --- a/crates/ruff_graph/Cargo.toml +++ b/crates/ruff_graph/Cargo.toml @@ -20,6 +20,7 @@ ty_python_semantic = { workspace = true } anyhow = { workspace = true } clap = { workspace = true, optional = true } +memchr = { workspace = true } salsa = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, optional = true } diff --git a/crates/ruff_graph/src/collector.rs b/crates/ruff_graph/src/collector.rs index 73b37991de..49e52a95fa 100644 --- a/crates/ruff_graph/src/collector.rs +++ b/crates/ruff_graph/src/collector.rs @@ -1,3 +1,4 @@ +use crate::StringImports; use ruff_python_ast::visitor::source_order::{ SourceOrderVisitor, walk_expr, walk_module, walk_stmt, }; @@ -10,13 +11,13 @@ pub(crate) struct Collector<'a> { /// The path to the current module. module_path: Option<&'a [String]>, /// Whether to detect imports from string literals. - string_imports: bool, + string_imports: StringImports, /// The collected imports from the Python AST. imports: Vec, } impl<'a> Collector<'a> { - pub(crate) fn new(module_path: Option<&'a [String]>, string_imports: bool) -> Self { + pub(crate) fn new(module_path: Option<&'a [String]>, string_imports: StringImports) -> Self { Self { module_path, string_imports, @@ -118,7 +119,7 @@ impl<'ast> SourceOrderVisitor<'ast> for Collector<'_> { | Stmt::Continue(_) | Stmt::IpyEscapeCommand(_) => { // Only traverse simple statements when string imports is enabled. - if self.string_imports { + if self.string_imports.enabled { walk_stmt(self, stmt); } } @@ -126,20 +127,26 @@ impl<'ast> SourceOrderVisitor<'ast> for Collector<'_> { } fn visit_expr(&mut self, expr: &'ast Expr) { - if self.string_imports { + if self.string_imports.enabled { if let Expr::StringLiteral(ast::ExprStringLiteral { value, range: _, node_index: _, }) = expr { - // Determine whether the string literal "looks like" an import statement: contains - // a dot, and consists solely of valid Python identifiers. let value = value.to_str(); - if let Some(module_name) = ModuleName::new(value) { - self.imports.push(CollectedImport::Import(module_name)); + // Determine whether the string literal "looks like" an import statement: contains + // the requisite number of dots, and consists solely of valid Python identifiers. + if self.string_imports.min_dots == 0 + || memchr::memchr_iter(b'.', value.as_bytes()).count() + >= self.string_imports.min_dots + { + if let Some(module_name) = ModuleName::new(value) { + self.imports.push(CollectedImport::Import(module_name)); + } } } + walk_expr(self, expr); } } diff --git a/crates/ruff_graph/src/lib.rs b/crates/ruff_graph/src/lib.rs index 6d2435efcd..bbe4182f5b 100644 --- a/crates/ruff_graph/src/lib.rs +++ b/crates/ruff_graph/src/lib.rs @@ -9,7 +9,7 @@ use ruff_python_parser::{Mode, ParseOptions, parse}; use crate::collector::Collector; pub use crate::db::ModuleDb; use crate::resolver::Resolver; -pub use crate::settings::{AnalyzeSettings, Direction}; +pub use crate::settings::{AnalyzeSettings, Direction, StringImports}; mod collector; mod db; @@ -26,7 +26,7 @@ impl ModuleImports { db: &ModuleDb, path: &SystemPath, package: Option<&SystemPath>, - string_imports: bool, + string_imports: StringImports, ) -> Result { // Read and parse the source code. let source = std::fs::read_to_string(path)?; diff --git a/crates/ruff_graph/src/settings.rs b/crates/ruff_graph/src/settings.rs index 3e47b97ef3..23d4b07f96 100644 --- a/crates/ruff_graph/src/settings.rs +++ b/crates/ruff_graph/src/settings.rs @@ -11,7 +11,7 @@ pub struct AnalyzeSettings { pub exclude: FilePatternSet, pub preview: PreviewMode, pub target_version: PythonVersion, - pub detect_string_imports: bool, + pub string_imports: StringImports, pub include_dependencies: BTreeMap)>, pub extension: ExtensionMapping, } @@ -26,7 +26,7 @@ impl fmt::Display for AnalyzeSettings { self.exclude, self.preview, self.target_version, - self.detect_string_imports, + self.string_imports, self.extension | debug, self.include_dependencies | debug, ] @@ -35,6 +35,31 @@ impl fmt::Display for AnalyzeSettings { } } +#[derive(Debug, Copy, Clone, CacheKey)] +pub struct StringImports { + pub enabled: bool, + pub min_dots: usize, +} + +impl Default for StringImports { + fn default() -> Self { + Self { + enabled: false, + min_dots: 2, + } + } +} + +impl fmt::Display for StringImports { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.enabled { + write!(f, "enabled (min_dots: {})", self.min_dots) + } else { + write!(f, "disabled") + } + } +} + #[derive(Default, Debug, Copy, Clone, PartialEq, Eq, CacheKey)] #[cfg_attr( feature = "serde", diff --git a/crates/ruff_workspace/src/configuration.rs b/crates/ruff_workspace/src/configuration.rs index 3e44b31596..2f8ef7b868 100644 --- a/crates/ruff_workspace/src/configuration.rs +++ b/crates/ruff_workspace/src/configuration.rs @@ -20,7 +20,7 @@ use strum::IntoEnumIterator; use ruff_cache::cache_dir; use ruff_formatter::IndentStyle; -use ruff_graph::{AnalyzeSettings, Direction}; +use ruff_graph::{AnalyzeSettings, Direction, StringImports}; use ruff_linter::line_width::{IndentWidth, LineLength}; use ruff_linter::registry::{INCOMPATIBLE_CODES, Rule, RuleNamespace, RuleSet}; use ruff_linter::rule_selector::{PreviewOptions, Specificity}; @@ -222,9 +222,14 @@ impl Configuration { preview: analyze_preview, target_version, extension: self.extension.clone().unwrap_or_default(), - detect_string_imports: analyze - .detect_string_imports - .unwrap_or(analyze_defaults.detect_string_imports), + string_imports: StringImports { + enabled: analyze + .detect_string_imports + .unwrap_or(analyze_defaults.string_imports.enabled), + min_dots: analyze + .string_imports_min_dots + .unwrap_or(analyze_defaults.string_imports.min_dots), + }, include_dependencies: analyze .include_dependencies .unwrap_or(analyze_defaults.include_dependencies), @@ -1271,6 +1276,7 @@ pub struct AnalyzeConfiguration { pub direction: Option, pub detect_string_imports: Option, + pub string_imports_min_dots: Option, pub include_dependencies: Option)>>, } @@ -1289,6 +1295,7 @@ impl AnalyzeConfiguration { preview: options.preview.map(PreviewMode::from), direction: options.direction, detect_string_imports: options.detect_string_imports, + string_imports_min_dots: options.string_imports_min_dots, include_dependencies: options.include_dependencies.map(|dependencies| { dependencies .into_iter() @@ -1307,6 +1314,9 @@ impl AnalyzeConfiguration { preview: self.preview.or(config.preview), direction: self.direction.or(config.direction), detect_string_imports: self.detect_string_imports.or(config.detect_string_imports), + string_imports_min_dots: self + .string_imports_min_dots + .or(config.string_imports_min_dots), include_dependencies: self.include_dependencies.or(config.include_dependencies), } } diff --git a/crates/ruff_workspace/src/options.rs b/crates/ruff_workspace/src/options.rs index a296adc773..44716e5b12 100644 --- a/crates/ruff_workspace/src/options.rs +++ b/crates/ruff_workspace/src/options.rs @@ -3843,6 +3843,12 @@ pub struct AnalyzeOptions { "# )] pub detect_string_imports: Option, + /// The minimum number of dots in a string to consider it a valid import. + /// + /// This setting is only relevant when [`detect-string-imports`](#detect-string-imports) is enabled. + /// For example, if this is set to `2`, then only strings with at least two dots (e.g., `"path.to.module"`) + /// would be considered valid imports. + pub string_imports_min_dots: Option, /// A map from file path to the list of Python or non-Python file paths or globs that should be /// considered dependencies of that file, regardless of whether relevant imports are detected. #[option( diff --git a/ruff.schema.json b/ruff.schema.json index 999daebd2f..0866fc0426 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -809,6 +809,15 @@ "boolean", "null" ] + }, + "string-imports-min-dots": { + "description": "The minimum number of dots in a string to consider it a valid import.\n\nThis setting is only relevant when [`detect-string-imports`](#detect-string-imports) is enabled. For example, if this is set to `2`, then only strings with at least two dots (e.g., `\"path.to.module\"`) would be considered valid imports.", + "type": [ + "integer", + "null" + ], + "format": "uint", + "minimum": 0.0 } }, "additionalProperties": false