mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-04 18:58:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[flake8-builtins] Make strict module name comparison optional (A005) (#15951)

Browse source 
## Summary

This PR adds the configuration option
`lint.flake8-builtins.builtins-strict-checking`, which is used in A005
to determine whether the fully-qualified module name (relative to the
project root or source directories) should be checked instead of just
the final component as is currently the case.

As discussed in
https://github.com/astral-sh/ruff/issues/15399#issuecomment-2587017147,
the default value of the new option is `false` on preview, so modules
like `utils.logging` from the initial report are no longer flagged by
default. For non-preview the default is still strict checking.

## Test Plan

New A005 test module with the structure reported in #15399.

Fixes #15399
This commit is contained in:
Brent Westbrook 2025-02-09 19:33:03 -05:00 • committed by GitHub
parent f367aa8367
commit 88b543d73a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 175 additions and 76 deletions
Download patch file Download diff file Expand all files Collapse all files

166
crates/ruff/tests/lint.rs
View file

@ -2272,38 +2272,36 @@ class Foo[_T, __T]:
);
}
#[test]
fn a005_module_shadowing_strict() -> Result<()> {
/// construct a directory tree with this structure:
/// .
/// ├── abc
/// │   └── __init__.py
/// ├── collections
/// │   ├── __init__.py
/// │   ├── abc
/// │   │   └── __init__.py
/// │   └── foobar
/// │   └── __init__.py
/// ├── foobar
/// │   ├── __init__.py
/// │   ├── abc
/// │   │   └── __init__.py
/// │   └── collections
/// │   ├── __init__.py
/// │   ├── abc
/// │   │   └── __init__.py
/// │   └── foobar
/// │   └── __init__.py
/// ├── ruff.toml
/// └── urlparse
/// └── __init__.py
fn create_a005_module_structure(tempdir: &TempDir) -> Result<()> {
fn create_module(path: &Path) -> Result<()> {
fs::create_dir(path)?;
fs::File::create(path.join("__init__.py"))?;
Ok(())
}
// construct a directory tree with this structure:
// .
// ├── abc
// │   └── __init__.py
// ├── collections
// │   ├── __init__.py
// │   ├── abc
// │   │   └── __init__.py
// │   └── foobar
// │   └── __init__.py
// ├── foobar
// │   ├── __init__.py
// │   ├── abc
// │   │   └── __init__.py
// │   └── collections
// │   ├── __init__.py
// │   ├── abc
// │   │   └── __init__.py
// │   └── foobar
// │   └── __init__.py
// ├── ruff.toml
// └── urlparse
// └── __init__.py
let tempdir = TempDir::new()?;
let foobar = tempdir.path().join("foobar");
create_module(&foobar)?;
for base in [&tempdir.path().into(), &foobar] {
@ -2317,6 +2315,82 @@ fn a005_module_shadowing_strict() -> Result<()> {
// also create a ruff.toml to mark the project root
fs::File::create(tempdir.path().join("ruff.toml"))?;
Ok(())
}
/// Test A005 with `builtins-strict-checking = true`
#[test]
fn a005_module_shadowing_strict() -> Result<()> {
let tempdir = TempDir::new()?;
create_a005_module_structure(&tempdir)?;
insta::with_settings!({
filters => vec![(r"\\", "/")]
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.arg("--config")
.arg(r#"lint.flake8-builtins.builtins-strict-checking = true"#)
.args(["--select", "A005"])
.current_dir(tempdir.path()),
@r"
success: false
exit_code: 1
----- stdout -----
abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
collections/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
foobar/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
foobar/collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
foobar/collections/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
Found 6 errors.
----- stderr -----
");
});
Ok(())
}
/// Test A005 with `builtins-strict-checking = false`
#[test]
fn a005_module_shadowing_non_strict() -> Result<()> {
let tempdir = TempDir::new()?;
create_a005_module_structure(&tempdir)?;
insta::with_settings!({
filters => vec![(r"\\", "/")]
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.arg("--config")
.arg(r#"lint.flake8-builtins.builtins-strict-checking = false"#)
.args(["--select", "A005"])
.current_dir(tempdir.path()),
@r"
success: false
exit_code: 1
----- stdout -----
abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
Found 2 errors.
----- stderr -----
");
});
Ok(())
}
/// Test A005 with `builtins-strict-checking` unset
/// TODO(brent) This should currently match the strict version, but after the next minor
/// release it will match the non-strict version directly above
#[test]
fn a005_module_shadowing_strict_default() -> Result<()> {
let tempdir = TempDir::new()?;
create_a005_module_structure(&tempdir)?;
insta::with_settings!({
filters => vec![(r"\\", "/")]
}, {
@ -2338,46 +2412,6 @@ fn a005_module_shadowing_strict() -> Result<()> {
----- stderr -----
");
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.args(["--select", "A005"])
.current_dir(tempdir.path()),
@r"
success: false
exit_code: 1
----- stdout -----
abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
collections/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
foobar/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
foobar/collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
foobar/collections/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
Found 6 errors.
----- stderr -----
");
// TODO(brent) Default should currently match the strict version, but after the next minor
// release it will match the non-strict version directly above
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.args(["--select", "A005"])
.current_dir(tempdir.path()),
@r"
success: false
exit_code: 1
----- stdout -----
abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
collections/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
foobar/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
foobar/collections/__init__.py:1:1: A005 Module `collections` shadows a Python standard-library module
foobar/collections/abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
Found 6 errors.
----- stderr -----
");
});
Ok(())

1
crates/ruff/tests/snapshots/show_settings__display_default_settings.snap
View file

@ -228,6 +228,7 @@ linter.flake8_bandit.check_typed_exception = false
linter.flake8_bugbear.extend_immutable_calls = []
linter.flake8_builtins.builtins_allowed_modules = []
linter.flake8_builtins.builtins_ignorelist = []
linter.flake8_builtins.builtins_strict_checking = true
linter.flake8_comprehensions.allow_dict_calls_with_keyword_arguments = false
linter.flake8_copyright.notice_rgx = (?i)Copyright\s+((?:\(C\)|©)\s+)?\d{4}((-|,\s)\d{4})*
linter.flake8_copyright.author = none

26
crates/ruff_linter/src/rules/flake8_builtins/mod.rs
View file

@ -12,6 +12,7 @@ mod tests {
use crate::assert_messages;
use crate::registry::Rule;
use crate::rules::flake8_builtins;
use crate::settings::types::PythonVersion;
use crate::settings::LinterSettings;
use crate::test::{test_path, test_resource_path};
@ -50,7 +51,13 @@ mod tests {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("flake8_builtins").join(path).as_path(),
&LinterSettings::for_rule(rule_code),
&LinterSettings {
flake8_builtins: flake8_builtins::settings::Settings {
builtins_strict_checking: true,
..Default::default()
},
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
@ -74,7 +81,13 @@ mod tests {
);
let diagnostics = test_path(
Path::new("flake8_builtins").join(path).as_path(),
&LinterSettings::for_rule(rule_code),
&LinterSettings {
flake8_builtins: flake8_builtins::settings::Settings {
builtins_strict_checking: strict,
..Default::default()
},
..LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
@ -92,6 +105,10 @@ mod tests {
Path::new("flake8_builtins").join(path).as_path(),
&LinterSettings {
src: vec![test_resource_path(src.join(path.parent().unwrap()))],
flake8_builtins: flake8_builtins::settings::Settings {
builtins_strict_checking: false,
..Default::default()
},
..LinterSettings::for_rule(rule_code)
},
)?;
@ -112,6 +129,10 @@ mod tests {
Path::new("flake8_builtins").join(path).as_path(),
&LinterSettings {
project_root: test_resource_path(src.join(path.parent().unwrap())),
flake8_builtins: flake8_builtins::settings::Settings {
builtins_strict_checking: false,
..Default::default()
},
..LinterSettings::for_rule(rule_code)
},
)?;
@ -179,6 +200,7 @@ mod tests {
&LinterSettings {
flake8_builtins: super::settings::Settings {
builtins_allowed_modules: vec!["xml".to_string(), "logging".to_string()],
builtins_strict_checking: true,
..Default::default()
},
..LinterSettings::for_rules(vec![rule_code])

5
crates/ruff_linter/src/rules/flake8_builtins/rules/stdlib_module_shadowing.rs
View file

@ -93,6 +93,11 @@ pub(crate) fn stdlib_module_shadowing(
return None;
}
// not allowed generally, but check for a parent in non-strict mode
if !settings.flake8_builtins.builtins_strict_checking && components.next().is_some() {
return None;
}
Some(Diagnostic::new(
StdlibModuleShadowing {
name: module_name.to_string(),

14
crates/ruff_linter/src/rules/flake8_builtins/settings.rs
View file

@ -1,6 +1,6 @@
//! Settings for the `flake8-builtins` plugin.
use crate::display_settings;
use crate::{display_settings, settings::types::PreviewMode};
use ruff_macros::CacheKey;
use std::fmt::{Display, Formatter};
@ -8,6 +8,17 @@ use std::fmt::{Display, Formatter};
pub struct Settings {
pub builtins_ignorelist: Vec<String>,
pub builtins_allowed_modules: Vec<String>,
pub builtins_strict_checking: bool,
}
impl Settings {
pub fn new(preview: PreviewMode) -> Self {
Self {
builtins_ignorelist: Vec::new(),
builtins_allowed_modules: Vec::new(),
builtins_strict_checking: preview.is_disabled(),
}
}
}
impl Display for Settings {
@ -18,6 +29,7 @@ impl Display for Settings {
fields = [
self.builtins_allowed_modules | array,
self.builtins_ignorelist | array,
self.builtins_strict_checking,
]
}
Ok(())

2
crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules__utils__logging.py_false.snap
View file

@ -1,4 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
---
logging.py:1:1: A005 Module `logging` shadows a Python standard-library module

12
crates/ruff_workspace/src/configuration.rs
View file

@ -234,6 +234,13 @@ impl Configuration {
let rules = lint.as_rule_table(lint_preview)?;
let flake8_builtins = lint
.flake8_builtins
.map(|builtins| builtins.into_settings(lint_preview))
.unwrap_or_else(|| {
ruff_linter::rules::flake8_builtins::settings::Settings::new(lint_preview)
});
// LinterSettings validation
let isort = lint
.isort
@ -335,10 +342,7 @@ impl Configuration {
.flake8_bugbear
.map(Flake8BugbearOptions::into_settings)
.unwrap_or_default(),
flake8_builtins: lint
.flake8_builtins
.map(Flake8BuiltinsOptions::into_settings)
.unwrap_or_default(),
flake8_builtins,
flake8_comprehensions: lint
.flake8_comprehensions
.map(Flake8ComprehensionsOptions::into_settings)

18
crates/ruff_workspace/src/options.rs
View file

@ -28,7 +28,7 @@ use ruff_linter::rules::{
pycodestyle, pydoclint, pydocstyle, pyflakes, pylint, pyupgrade, ruff,
};
use ruff_linter::settings::types::{
IdentifierPattern, OutputFormat, PythonVersion, RequiredVersion,
IdentifierPattern, OutputFormat, PreviewMode, PythonVersion, RequiredVersion,
};
use ruff_linter::{warn_user_once, RuleSelector};
use ruff_macros::{CombineOptions, OptionsMetadata};
@ -1143,13 +1143,27 @@ pub struct Flake8BuiltinsOptions {
)]
/// List of builtin module names to allow.
pub builtins_allowed_modules: Option<Vec<String>>,
#[option(
default = r#"true"#,
value_type = "bool",
example = "builtins-strict-checking = false"
)]
/// Compare module names instead of full module paths.
pub builtins_strict_checking: Option<bool>,
}
impl Flake8BuiltinsOptions {
pub fn into_settings(self) -> ruff_linter::rules::flake8_builtins::settings::Settings {
pub fn into_settings(
self,
preview: PreviewMode,
) -> ruff_linter::rules::flake8_builtins::settings::Settings {
ruff_linter::rules::flake8_builtins::settings::Settings {
builtins_ignorelist: self.builtins_ignorelist.unwrap_or_default(),
builtins_allowed_modules: self.builtins_allowed_modules.unwrap_or_default(),
builtins_strict_checking: self
.builtins_strict_checking
// use the old default of true on non-preview
.unwrap_or(preview.is_disabled()),
}
}
}

7
ruff.schema.json generated
View file

@ -1024,6 +1024,13 @@
"items": {
"type": "string"
}
},
"builtins-strict-checking": {
"description": "Compare module names instead of full module paths.",
"type": [
"boolean",
"null"
]
}
},
"additionalProperties": false