mirror of https://github.com/astral-sh/ruff
[`flake8-builtins`] Default to non-strict checking (`A005`) (#16125)
## Summary This PR changes the default value of `lint.flake8-builtins.builtins-strict-checking` added in https://github.com/astral-sh/ruff/pull/15951 from `true` to `false`. This also allows simplifying the default option logic and removes the dependence on preview mode. https://github.com/astral-sh/ruff/issues/15399 was already closed by #15951, but this change will finalize the behavior mentioned in https://github.com/astral-sh/ruff/issues/15399#issuecomment-2587017147. As an example, strict checking flags modules based on their last component, so `utils/logging.py` triggers A005. Non-strict checking checks the path to the module, so `utils/logging.py` is allowed (this is the example and desired behavior from #15399 exactly) but a top-level `logging.py` or `logging/__init__.py` is still disallowed. ## Test Plan Existing tests from #15951 and #16006, with the snapshot updated in `a005_module_shadowing_strict_default` to reflect the new default.
This commit is contained in:
parent
958e1177ce
commit
a04347b7a3
|
|
@ -2556,11 +2556,7 @@ fn a005_module_shadowing_strict_default() -> Result<()> {
|
||||||
----- stdout -----
|
----- stdout -----
|
||||||
abc/__init__.py:1:1: A005 Module `abc` shadows a Python standard-library module
|
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/__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
|
Found 2 errors.
|
||||||
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 -----
|
----- stderr -----
|
||||||
");
|
");
|
||||||
|
|
|
||||||
|
|
@ -231,7 +231,7 @@ linter.flake8_bandit.allowed_markup_calls = []
|
||||||
linter.flake8_bugbear.extend_immutable_calls = []
|
linter.flake8_bugbear.extend_immutable_calls = []
|
||||||
linter.flake8_builtins.allowed_modules = []
|
linter.flake8_builtins.allowed_modules = []
|
||||||
linter.flake8_builtins.ignorelist = []
|
linter.flake8_builtins.ignorelist = []
|
||||||
linter.flake8_builtins.strict_checking = true
|
linter.flake8_builtins.strict_checking = false
|
||||||
linter.flake8_comprehensions.allow_dict_calls_with_keyword_arguments = false
|
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.notice_rgx = (?i)Copyright\s+((?:\(C\)|©)\s+)?\d{4}((-|,\s)\d{4})*
|
||||||
linter.flake8_copyright.author = none
|
linter.flake8_copyright.author = none
|
||||||
|
|
|
||||||
|
|
@ -23,12 +23,12 @@ use crate::settings::LinterSettings;
|
||||||
/// Standard-library modules can be marked as exceptions to this rule via the
|
/// Standard-library modules can be marked as exceptions to this rule via the
|
||||||
/// [`lint.flake8-builtins.allowed-modules`] configuration option.
|
/// [`lint.flake8-builtins.allowed-modules`] configuration option.
|
||||||
///
|
///
|
||||||
/// By default, only the last component of the module name is considered, so `logging.py`,
|
/// By default, the module path relative to the project root or [`src`] directories is considered,
|
||||||
/// `utils/logging.py`, and `utils/logging/__init__.py` would all clash with the builtin `logging`
|
/// so a top-level `logging.py` or `logging/__init__.py` will clash with the builtin `logging`
|
||||||
/// module. With the [`lint.flake8-builtins.strict-checking`] option set to `false`, the module
|
/// module, but `utils/logging.py`, for example, will not. With the
|
||||||
/// path is considered, so only a top-level `logging.py` or `logging/__init__.py` will trigger the
|
/// [`lint.flake8-builtins.builtins-strict-checking`] option set to `true`, only the last component
|
||||||
/// rule and `utils/logging.py`, for example, would not. In preview mode, the default value of
|
/// of the module name is considered, so `logging.py`, `utils/logging.py`, and
|
||||||
/// [`lint.flake8-builtins.strict-checking`] is `false` rather than `true` in stable mode.
|
/// `utils/logging/__init__.py` will all trigger the rule.
|
||||||
///
|
///
|
||||||
/// This rule is not applied to stub files, as the name of a stub module is out
|
/// This rule is not applied to stub files, as the name of a stub module is out
|
||||||
/// of the control of the author of the stub file. Instead, a stub should aim to
|
/// of the control of the author of the stub file. Instead, a stub should aim to
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
//! Settings for the `flake8-builtins` plugin.
|
//! Settings for the `flake8-builtins` plugin.
|
||||||
|
|
||||||
use crate::{display_settings, settings::types::PreviewMode};
|
use crate::display_settings;
|
||||||
use ruff_macros::CacheKey;
|
use ruff_macros::CacheKey;
|
||||||
use std::fmt::{Display, Formatter};
|
use std::fmt::{Display, Formatter};
|
||||||
|
|
||||||
|
|
@ -11,16 +11,6 @@ pub struct Settings {
|
||||||
pub strict_checking: bool,
|
pub strict_checking: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Settings {
|
|
||||||
pub fn new(preview: PreviewMode) -> Self {
|
|
||||||
Self {
|
|
||||||
ignorelist: Vec::new(),
|
|
||||||
allowed_modules: Vec::new(),
|
|
||||||
strict_checking: preview.is_disabled(),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
impl Display for Settings {
|
impl Display for Settings {
|
||||||
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
|
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
|
||||||
display_settings! {
|
display_settings! {
|
||||||
|
|
|
||||||
|
|
@ -235,13 +235,6 @@ impl Configuration {
|
||||||
|
|
||||||
let rules = lint.as_rule_table(lint_preview)?;
|
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
|
// LinterSettings validation
|
||||||
let isort = lint
|
let isort = lint
|
||||||
.isort
|
.isort
|
||||||
|
|
@ -344,7 +337,10 @@ impl Configuration {
|
||||||
.flake8_bugbear
|
.flake8_bugbear
|
||||||
.map(Flake8BugbearOptions::into_settings)
|
.map(Flake8BugbearOptions::into_settings)
|
||||||
.unwrap_or_default(),
|
.unwrap_or_default(),
|
||||||
flake8_builtins,
|
flake8_builtins: lint
|
||||||
|
.flake8_builtins
|
||||||
|
.map(Flake8BuiltinsOptions::into_settings)
|
||||||
|
.unwrap_or_default(),
|
||||||
flake8_comprehensions: lint
|
flake8_comprehensions: lint
|
||||||
.flake8_comprehensions
|
.flake8_comprehensions
|
||||||
.map(Flake8ComprehensionsOptions::into_settings)
|
.map(Flake8ComprehensionsOptions::into_settings)
|
||||||
|
|
|
||||||
|
|
@ -28,7 +28,7 @@ use ruff_linter::rules::{
|
||||||
pycodestyle, pydoclint, pydocstyle, pyflakes, pylint, pyupgrade, ruff,
|
pycodestyle, pydoclint, pydocstyle, pyflakes, pylint, pyupgrade, ruff,
|
||||||
};
|
};
|
||||||
use ruff_linter::settings::types::{
|
use ruff_linter::settings::types::{
|
||||||
IdentifierPattern, OutputFormat, PreviewMode, PythonVersion, RequiredVersion,
|
IdentifierPattern, OutputFormat, PythonVersion, RequiredVersion,
|
||||||
};
|
};
|
||||||
use ruff_linter::{warn_user_once, RuleSelector};
|
use ruff_linter::{warn_user_once, RuleSelector};
|
||||||
use ruff_macros::{CombineOptions, OptionsMetadata};
|
use ruff_macros::{CombineOptions, OptionsMetadata};
|
||||||
|
|
@ -1267,9 +1267,9 @@ pub struct Flake8BuiltinsOptions {
|
||||||
///
|
///
|
||||||
/// This option is ignored if both `strict-checking` and `builtins-strict-checking` are set.
|
/// This option is ignored if both `strict-checking` and `builtins-strict-checking` are set.
|
||||||
#[option(
|
#[option(
|
||||||
default = r#"true"#,
|
default = r#"false"#,
|
||||||
value_type = "bool",
|
value_type = "bool",
|
||||||
example = "builtins-strict-checking = false"
|
example = "builtins-strict-checking = true"
|
||||||
)]
|
)]
|
||||||
#[deprecated(
|
#[deprecated(
|
||||||
since = "0.10.0",
|
since = "0.10.0",
|
||||||
|
|
@ -1280,21 +1280,16 @@ pub struct Flake8BuiltinsOptions {
|
||||||
/// Compare module names instead of full module paths.
|
/// Compare module names instead of full module paths.
|
||||||
///
|
///
|
||||||
/// Used by [`A005` - `stdlib-module-shadowing`](https://docs.astral.sh/ruff/rules/stdlib-module-shadowing/).
|
/// Used by [`A005` - `stdlib-module-shadowing`](https://docs.astral.sh/ruff/rules/stdlib-module-shadowing/).
|
||||||
///
|
|
||||||
/// In preview mode the default value is `false` rather than `true`.
|
|
||||||
#[option(
|
#[option(
|
||||||
default = r#"true"#,
|
default = r#"false"#,
|
||||||
value_type = "bool",
|
value_type = "bool",
|
||||||
example = "strict-checking = false"
|
example = "strict-checking = true"
|
||||||
)]
|
)]
|
||||||
pub strict_checking: Option<bool>,
|
pub strict_checking: Option<bool>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Flake8BuiltinsOptions {
|
impl Flake8BuiltinsOptions {
|
||||||
pub fn into_settings(
|
pub fn into_settings(self) -> ruff_linter::rules::flake8_builtins::settings::Settings {
|
||||||
self,
|
|
||||||
preview: PreviewMode,
|
|
||||||
) -> ruff_linter::rules::flake8_builtins::settings::Settings {
|
|
||||||
#[allow(deprecated)]
|
#[allow(deprecated)]
|
||||||
ruff_linter::rules::flake8_builtins::settings::Settings {
|
ruff_linter::rules::flake8_builtins::settings::Settings {
|
||||||
ignorelist: self
|
ignorelist: self
|
||||||
|
|
@ -1309,7 +1304,7 @@ impl Flake8BuiltinsOptions {
|
||||||
.strict_checking
|
.strict_checking
|
||||||
.or(self.builtins_strict_checking)
|
.or(self.builtins_strict_checking)
|
||||||
// use the old default of true on non-preview
|
// use the old default of true on non-preview
|
||||||
.unwrap_or(preview.is_disabled()),
|
.unwrap_or_default(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -210,7 +210,7 @@ mod tests {
|
||||||
|
|
||||||
use ruff_linter::codes;
|
use ruff_linter::codes;
|
||||||
use ruff_linter::line_width::LineLength;
|
use ruff_linter::line_width::LineLength;
|
||||||
use ruff_linter::settings::types::{PatternPrefixPair, PreviewMode};
|
use ruff_linter::settings::types::PatternPrefixPair;
|
||||||
|
|
||||||
use crate::options::{Flake8BuiltinsOptions, LintCommonOptions, LintOptions, Options};
|
use crate::options::{Flake8BuiltinsOptions, LintCommonOptions, LintOptions, Options};
|
||||||
use crate::pyproject::{find_settings_toml, parse_pyproject_toml, Pyproject, Tools};
|
use crate::pyproject::{find_settings_toml, parse_pyproject_toml, Pyproject, Tools};
|
||||||
|
|
@ -363,7 +363,7 @@ strict-checking = false
|
||||||
})
|
})
|
||||||
);
|
);
|
||||||
|
|
||||||
let settings = expected.into_settings(PreviewMode::Enabled);
|
let settings = expected.into_settings();
|
||||||
|
|
||||||
assert_eq!(settings.allowed_modules, vec!["sys".to_string()]);
|
assert_eq!(settings.allowed_modules, vec!["sys".to_string()]);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|
|
||||||
|
|
@ -1086,7 +1086,7 @@
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"strict-checking": {
|
"strict-checking": {
|
||||||
"description": "Compare module names instead of full module paths.\n\nUsed by [`A005` - `stdlib-module-shadowing`](https://docs.astral.sh/ruff/rules/stdlib-module-shadowing/).\n\nIn preview mode the default value is `false` rather than `true`.",
|
"description": "Compare module names instead of full module paths.\n\nUsed by [`A005` - `stdlib-module-shadowing`](https://docs.astral.sh/ruff/rules/stdlib-module-shadowing/).",
|
||||||
"type": [
|
"type": [
|
||||||
"boolean",
|
"boolean",
|
||||||
"null"
|
"null"
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue