Add support for specifying minimum dots in detected string imports (#19538)

## Summary

Defaults to requiring two dots, which matches the Pants default.
This commit is contained in:
Charlie Marsh 2025-07-24 15:48:23 -04:00 committed by GitHub
parent d77b7312b0
commit d9cab4d242
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 129 additions and 42 deletions

1
Cargo.lock generated
View file

@ -2960,6 +2960,7 @@ version = "0.1.0"
dependencies = [ dependencies = [
"anyhow", "anyhow",
"clap", "clap",
"memchr",
"ruff_cache", "ruff_cache",
"ruff_db", "ruff_db",
"ruff_linter", "ruff_linter",

View file

@ -169,6 +169,9 @@ pub struct AnalyzeGraphCommand {
/// Attempt to detect imports from string literals. /// Attempt to detect imports from string literals.
#[clap(long)] #[clap(long)]
detect_string_imports: bool, detect_string_imports: bool,
/// The minimum number of dots in a string import to consider it a valid import.
#[clap(long)]
min_dots: Option<usize>,
/// Enable preview mode. Use `--no-preview` to disable. /// Enable preview mode. Use `--no-preview` to disable.
#[arg(long, overrides_with("no_preview"))] #[arg(long, overrides_with("no_preview"))]
preview: bool, preview: bool,
@ -808,6 +811,7 @@ impl AnalyzeGraphCommand {
} else { } else {
None None
}, },
string_imports_min_dots: self.min_dots,
preview: resolve_bool_arg(self.preview, self.no_preview).map(PreviewMode::from), preview: resolve_bool_arg(self.preview, self.no_preview).map(PreviewMode::from),
target_version: self.target_version.map(ast::PythonVersion::from), target_version: self.target_version.map(ast::PythonVersion::from),
..ExplicitConfigOverrides::default() ..ExplicitConfigOverrides::default()
@ -1305,6 +1309,7 @@ struct ExplicitConfigOverrides {
show_fixes: Option<bool>, show_fixes: Option<bool>,
extension: Option<Vec<ExtensionPair>>, extension: Option<Vec<ExtensionPair>>,
detect_string_imports: Option<bool>, detect_string_imports: Option<bool>,
string_imports_min_dots: Option<usize>,
} }
impl ConfigurationTransformer for ExplicitConfigOverrides { impl ConfigurationTransformer for ExplicitConfigOverrides {
@ -1392,6 +1397,9 @@ impl ConfigurationTransformer for ExplicitConfigOverrides {
if let Some(detect_string_imports) = &self.detect_string_imports { if let Some(detect_string_imports) = &self.detect_string_imports {
config.analyze.detect_string_imports = Some(*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 config
} }

View file

@ -102,7 +102,7 @@ pub(crate) fn analyze_graph(
// Resolve the per-file settings. // Resolve the per-file settings.
let settings = resolver.resolve(path); 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(); let include_dependencies = settings.analyze.include_dependencies.get(path).cloned();
// Skip excluded files. // Skip excluded files.

View file

@ -197,7 +197,27 @@ fn string_detection() -> Result<()> {
insta::with_settings!({ insta::with_settings!({
filters => INSTA_FILTERS.to_vec(), filters => INSTA_FILTERS.to_vec(),
}, { }, {
assert_cmd_snapshot!(command().arg("--detect-string-imports").current_dir(&root), @r###" 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 -----
"#);
});
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 success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----
@ -213,7 +233,7 @@ fn string_detection() -> Result<()> {
} }
----- stderr ----- ----- stderr -----
"###); "#);
}); });
Ok(()) Ok(())

View file

@ -2422,7 +2422,7 @@ requires-python = ">= 3.11"
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.11 analyze.target_version = 3.11
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -2734,7 +2734,7 @@ requires-python = ">= 3.11"
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.10 analyze.target_version = 3.10
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -3098,7 +3098,7 @@ from typing import Union;foo: Union[int, str] = 1
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.11 analyze.target_version = 3.11
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -3478,7 +3478,7 @@ from typing import Union;foo: Union[int, str] = 1
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.11 analyze.target_version = 3.11
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -3806,7 +3806,7 @@ from typing import Union;foo: Union[int, str] = 1
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.10 analyze.target_version = 3.10
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -4134,7 +4134,7 @@ from typing import Union;foo: Union[int, str] = 1
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.9 analyze.target_version = 3.9
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -4419,7 +4419,7 @@ from typing import Union;foo: Union[int, str] = 1
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.9 analyze.target_version = 3.9
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}
@ -4757,7 +4757,7 @@ from typing import Union;foo: Union[int, str] = 1
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.10 analyze.target_version = 3.10
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}

View file

@ -392,7 +392,7 @@ formatter.docstring_code_line_width = dynamic
analyze.exclude = [] analyze.exclude = []
analyze.preview = disabled analyze.preview = disabled
analyze.target_version = 3.7 analyze.target_version = 3.7
analyze.detect_string_imports = false analyze.string_imports = disabled
analyze.extension = ExtensionMapping({}) analyze.extension = ExtensionMapping({})
analyze.include_dependencies = {} analyze.include_dependencies = {}

View file

@ -20,6 +20,7 @@ ty_python_semantic = { workspace = true }
anyhow = { workspace = true } anyhow = { workspace = true }
clap = { workspace = true, optional = true } clap = { workspace = true, optional = true }
memchr = { workspace = true }
salsa = { workspace = true } salsa = { workspace = true }
schemars = { workspace = true, optional = true } schemars = { workspace = true, optional = true }
serde = { workspace = true, optional = true } serde = { workspace = true, optional = true }

View file

@ -1,3 +1,4 @@
use crate::StringImports;
use ruff_python_ast::visitor::source_order::{ use ruff_python_ast::visitor::source_order::{
SourceOrderVisitor, walk_expr, walk_module, walk_stmt, SourceOrderVisitor, walk_expr, walk_module, walk_stmt,
}; };
@ -10,13 +11,13 @@ pub(crate) struct Collector<'a> {
/// The path to the current module. /// The path to the current module.
module_path: Option<&'a [String]>, module_path: Option<&'a [String]>,
/// Whether to detect imports from string literals. /// Whether to detect imports from string literals.
string_imports: bool, string_imports: StringImports,
/// The collected imports from the Python AST. /// The collected imports from the Python AST.
imports: Vec<CollectedImport>, imports: Vec<CollectedImport>,
} }
impl<'a> Collector<'a> { 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 { Self {
module_path, module_path,
string_imports, string_imports,
@ -118,7 +119,7 @@ impl<'ast> SourceOrderVisitor<'ast> for Collector<'_> {
| Stmt::Continue(_) | Stmt::Continue(_)
| Stmt::IpyEscapeCommand(_) => { | Stmt::IpyEscapeCommand(_) => {
// Only traverse simple statements when string imports is enabled. // Only traverse simple statements when string imports is enabled.
if self.string_imports { if self.string_imports.enabled {
walk_stmt(self, stmt); walk_stmt(self, stmt);
} }
} }
@ -126,20 +127,26 @@ impl<'ast> SourceOrderVisitor<'ast> for Collector<'_> {
} }
fn visit_expr(&mut self, expr: &'ast Expr) { fn visit_expr(&mut self, expr: &'ast Expr) {
if self.string_imports { if self.string_imports.enabled {
if let Expr::StringLiteral(ast::ExprStringLiteral { if let Expr::StringLiteral(ast::ExprStringLiteral {
value, value,
range: _, range: _,
node_index: _, node_index: _,
}) = expr }) = 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(); let value = value.to_str();
// 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) { if let Some(module_name) = ModuleName::new(value) {
self.imports.push(CollectedImport::Import(module_name)); self.imports.push(CollectedImport::Import(module_name));
} }
} }
}
walk_expr(self, expr); walk_expr(self, expr);
} }
} }

View file

@ -9,7 +9,7 @@ use ruff_python_parser::{Mode, ParseOptions, parse};
use crate::collector::Collector; use crate::collector::Collector;
pub use crate::db::ModuleDb; pub use crate::db::ModuleDb;
use crate::resolver::Resolver; use crate::resolver::Resolver;
pub use crate::settings::{AnalyzeSettings, Direction}; pub use crate::settings::{AnalyzeSettings, Direction, StringImports};
mod collector; mod collector;
mod db; mod db;
@ -26,7 +26,7 @@ impl ModuleImports {
db: &ModuleDb, db: &ModuleDb,
path: &SystemPath, path: &SystemPath,
package: Option<&SystemPath>, package: Option<&SystemPath>,
string_imports: bool, string_imports: StringImports,
) -> Result<Self> { ) -> Result<Self> {
// Read and parse the source code. // Read and parse the source code.
let source = std::fs::read_to_string(path)?; let source = std::fs::read_to_string(path)?;

View file

@ -11,7 +11,7 @@ pub struct AnalyzeSettings {
pub exclude: FilePatternSet, pub exclude: FilePatternSet,
pub preview: PreviewMode, pub preview: PreviewMode,
pub target_version: PythonVersion, pub target_version: PythonVersion,
pub detect_string_imports: bool, pub string_imports: StringImports,
pub include_dependencies: BTreeMap<PathBuf, (PathBuf, Vec<String>)>, pub include_dependencies: BTreeMap<PathBuf, (PathBuf, Vec<String>)>,
pub extension: ExtensionMapping, pub extension: ExtensionMapping,
} }
@ -26,7 +26,7 @@ impl fmt::Display for AnalyzeSettings {
self.exclude, self.exclude,
self.preview, self.preview,
self.target_version, self.target_version,
self.detect_string_imports, self.string_imports,
self.extension | debug, self.extension | debug,
self.include_dependencies | 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)] #[derive(Default, Debug, Copy, Clone, PartialEq, Eq, CacheKey)]
#[cfg_attr( #[cfg_attr(
feature = "serde", feature = "serde",

View file

@ -20,7 +20,7 @@ use strum::IntoEnumIterator;
use ruff_cache::cache_dir; use ruff_cache::cache_dir;
use ruff_formatter::IndentStyle; 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::line_width::{IndentWidth, LineLength};
use ruff_linter::registry::{INCOMPATIBLE_CODES, Rule, RuleNamespace, RuleSet}; use ruff_linter::registry::{INCOMPATIBLE_CODES, Rule, RuleNamespace, RuleSet};
use ruff_linter::rule_selector::{PreviewOptions, Specificity}; use ruff_linter::rule_selector::{PreviewOptions, Specificity};
@ -222,9 +222,14 @@ impl Configuration {
preview: analyze_preview, preview: analyze_preview,
target_version, target_version,
extension: self.extension.clone().unwrap_or_default(), extension: self.extension.clone().unwrap_or_default(),
detect_string_imports: analyze string_imports: StringImports {
enabled: analyze
.detect_string_imports .detect_string_imports
.unwrap_or(analyze_defaults.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: analyze
.include_dependencies .include_dependencies
.unwrap_or(analyze_defaults.include_dependencies), .unwrap_or(analyze_defaults.include_dependencies),
@ -1271,6 +1276,7 @@ pub struct AnalyzeConfiguration {
pub direction: Option<Direction>, pub direction: Option<Direction>,
pub detect_string_imports: Option<bool>, pub detect_string_imports: Option<bool>,
pub string_imports_min_dots: Option<usize>,
pub include_dependencies: Option<BTreeMap<PathBuf, (PathBuf, Vec<String>)>>, pub include_dependencies: Option<BTreeMap<PathBuf, (PathBuf, Vec<String>)>>,
} }
@ -1289,6 +1295,7 @@ impl AnalyzeConfiguration {
preview: options.preview.map(PreviewMode::from), preview: options.preview.map(PreviewMode::from),
direction: options.direction, direction: options.direction,
detect_string_imports: options.detect_string_imports, detect_string_imports: options.detect_string_imports,
string_imports_min_dots: options.string_imports_min_dots,
include_dependencies: options.include_dependencies.map(|dependencies| { include_dependencies: options.include_dependencies.map(|dependencies| {
dependencies dependencies
.into_iter() .into_iter()
@ -1307,6 +1314,9 @@ impl AnalyzeConfiguration {
preview: self.preview.or(config.preview), preview: self.preview.or(config.preview),
direction: self.direction.or(config.direction), direction: self.direction.or(config.direction),
detect_string_imports: self.detect_string_imports.or(config.detect_string_imports), 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), include_dependencies: self.include_dependencies.or(config.include_dependencies),
} }
} }

View file

@ -3843,6 +3843,12 @@ pub struct AnalyzeOptions {
"# "#
)] )]
pub detect_string_imports: Option<bool>, pub detect_string_imports: Option<bool>,
/// 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<usize>,
/// A map from file path to the list of Python or non-Python file paths or globs that should be /// 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. /// considered dependencies of that file, regardless of whether relevant imports are detected.
#[option( #[option(

9
ruff.schema.json generated
View file

@ -809,6 +809,15 @@
"boolean", "boolean",
"null" "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 "additionalProperties": false