Skip to content

Commit

Permalink
[flake8-builtins] Rename A005 and improve its error message (#15348)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexWaygood authored Jan 8, 2025
1 parent 487f2f5 commit 9a27b37
Show file tree
Hide file tree
Showing 12 changed files with 38 additions and 43 deletions.
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/checkers/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_python_trivia::CommentRanges;

use crate::package::PackageRoot;
use crate::registry::Rule;
use crate::rules::flake8_builtins::rules::builtin_module_shadowing;
use crate::rules::flake8_builtins::rules::stdlib_module_shadowing;
use crate::rules::flake8_no_pep420::rules::implicit_namespace_package;
use crate::rules::pep8_naming::rules::invalid_module_name;
use crate::settings::LinterSettings;
Expand Down Expand Up @@ -45,8 +45,8 @@ pub(crate) fn check_file_path(
}

// flake8-builtins
if settings.rules.enabled(Rule::BuiltinModuleShadowing) {
if let Some(diagnostic) = builtin_module_shadowing(
if settings.rules.enabled(Rule::StdlibModuleShadowing) {
if let Some(diagnostic) = stdlib_module_shadowing(
path,
package,
&settings.flake8_builtins.builtins_allowed_modules,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Builtins, "002") => (RuleGroup::Stable, rules::flake8_builtins::rules::BuiltinArgumentShadowing),
(Flake8Builtins, "003") => (RuleGroup::Stable, rules::flake8_builtins::rules::BuiltinAttributeShadowing),
(Flake8Builtins, "004") => (RuleGroup::Stable, rules::flake8_builtins::rules::BuiltinImportShadowing),
(Flake8Builtins, "005") => (RuleGroup::Preview, rules::flake8_builtins::rules::BuiltinModuleShadowing),
(Flake8Builtins, "005") => (RuleGroup::Preview, rules::flake8_builtins::rules::StdlibModuleShadowing),
(Flake8Builtins, "006") => (RuleGroup::Preview, rules::flake8_builtins::rules::BuiltinLambdaArgumentShadowing),

// flake8-bugbear
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ impl Rule {
Rule::UnsortedImports | Rule::MissingRequiredImport => LintSource::Imports,
Rule::ImplicitNamespacePackage
| Rule::InvalidModuleName
| Rule::BuiltinModuleShadowing => LintSource::Filesystem,
| Rule::StdlibModuleShadowing => LintSource::Filesystem,
Rule::IndentationWithInvalidMultiple
| Rule::IndentationWithInvalidMultipleComment
| Rule::MissingWhitespace
Expand Down
24 changes: 12 additions & 12 deletions crates/ruff_linter/src/rules/flake8_builtins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,26 @@ mod tests {
#[test_case(Rule::BuiltinAttributeShadowing, Path::new("A003.py"))]
#[test_case(Rule::BuiltinImportShadowing, Path::new("A004.py"))]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/non_builtin/__init__.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/logging/__init__.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/string/__init__.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/package/bisect.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/_abc/__init__.py")
)]
#[test_case(Rule::BuiltinModuleShadowing, Path::new("A005/modules/package/xml.py"))]
#[test_case(Rule::StdlibModuleShadowing, Path::new("A005/modules/package/xml.py"))]
#[test_case(Rule::BuiltinLambdaArgumentShadowing, Path::new("A006.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand Down Expand Up @@ -80,26 +80,26 @@ mod tests {
}

#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/non_builtin/__init__.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/logging/__init__.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/string/__init__.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/package/bisect.py")
)]
#[test_case(
Rule::BuiltinModuleShadowing,
Rule::StdlibModuleShadowing,
Path::new("A005/modules/_abc/__init__.py")
)]
#[test_case(Rule::BuiltinModuleShadowing, Path::new("A005/modules/package/xml.py"))]
#[test_case(Rule::StdlibModuleShadowing, Path::new("A005/modules/package/xml.py"))]
fn builtins_allowed_modules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"{}_{}_builtins_allowed_modules",
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/flake8_builtins/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ pub(crate) use builtin_argument_shadowing::*;
pub(crate) use builtin_attribute_shadowing::*;
pub(crate) use builtin_import_shadowing::*;
pub(crate) use builtin_lambda_argument_shadowing::*;
pub(crate) use builtin_module_shadowing::*;
pub(crate) use builtin_variable_shadowing::*;
pub(crate) use stdlib_module_shadowing::*;

mod builtin_argument_shadowing;
mod builtin_attribute_shadowing;
mod builtin_import_shadowing;
mod builtin_lambda_argument_shadowing;
mod builtin_module_shadowing;
mod builtin_variable_shadowing;
mod stdlib_module_shadowing;
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,35 @@ use crate::package::PackageRoot;
use crate::settings::types::PythonVersion;

/// ## What it does
/// Checks for modules that use the same names as Python builtin modules.
/// Checks for modules that use the same names as Python standard-library
/// modules.
///
/// ## Why is this bad?
/// Reusing a builtin module name for the name of a module increases the
/// difficulty of reading and maintaining the code, and can cause
/// non-obvious errors, as readers may mistake the variable for the
/// builtin and vice versa.
/// Reusing a standard-library module name for the name of a module increases
/// the difficulty of reading and maintaining the code, and can cause
/// non-obvious errors. Readers may mistake the first-party module for the
/// standard-library module and vice versa.
///
/// Builtin 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.builtins-allowed-modules`] configuration option.
///
/// ## Options
/// - `lint.flake8-builtins.builtins-allowed-modules`
#[derive(ViolationMetadata)]
pub(crate) struct BuiltinModuleShadowing {
pub(crate) struct StdlibModuleShadowing {
name: String,
}

impl Violation for BuiltinModuleShadowing {
impl Violation for StdlibModuleShadowing {
#[derive_message_formats]
fn message(&self) -> String {
let BuiltinModuleShadowing { name } = self;
format!("Module `{name}` is shadowing a Python builtin module")
let StdlibModuleShadowing { name } = self;
format!("Module `{name}` shadows a Python standard-library module")
}
}

/// A005
pub(crate) fn builtin_module_shadowing(
pub(crate) fn stdlib_module_shadowing(
path: &Path,
package: Option<PackageRoot<'_>>,
allowed_modules: &[String],
Expand Down Expand Up @@ -74,7 +75,7 @@ pub(crate) fn builtin_module_shadowing(
}

Some(Diagnostic::new(
BuiltinModuleShadowing {
StdlibModuleShadowing {
name: module_name.to_string(),
},
TextRange::default(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
snapshot_kind: text
---
__init__.py:1:1: A005 Module `logging` is shadowing a Python builtin module
__init__.py:1:1: A005 Module `logging` shadows a Python standard-library module
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
snapshot_kind: text
---
bisect.py:1:1: A005 Module `bisect` is shadowing a Python builtin module
bisect.py:1:1: A005 Module `bisect` shadows a Python standard-library module
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
snapshot_kind: text
---
bisect.py:1:1: A005 Module `bisect` is shadowing a Python builtin module
bisect.py:1:1: A005 Module `bisect` shadows a Python standard-library module
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
snapshot_kind: text
---
xml.py:1:1: A005 Module `xml` is shadowing a Python builtin module
xml.py:1:1: A005 Module `xml` shadows a Python standard-library module
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
snapshot_kind: text
---
__init__.py:1:1: A005 Module `string` is shadowing a Python builtin module
__init__.py:1:1: A005 Module `string` shadows a Python standard-library module
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs
snapshot_kind: text
---
__init__.py:1:1: A005 Module `string` is shadowing a Python builtin module
__init__.py:1:1: A005 Module `string` shadows a Python standard-library module

0 comments on commit 9a27b37

Please sign in to comment.