From 545e9deba3dfc0855839875710d82ef24874bad7 Mon Sep 17 00:00:00 2001 From: InSync Date: Sun, 24 Nov 2024 09:39:04 +0700 Subject: [PATCH] [`flake8-builtins`] Exempt private built-in modules (`A005`) (#14505) ## Summary Resolves #12949. ## Test Plan `cargo nextest run` and `cargo insta test`. --- .../A005/modules/_abc/__init__.py | 0 .../src/rules/flake8_builtins/mod.rs | 8 +++ .../rules/builtin_module_shadowing.rs | 53 +++++++++++-------- ...A005_A005__modules___abc____init__.py.snap | 5 ++ ...___init__.py_builtins_allowed_modules.snap | 5 ++ 5 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_builtins/A005/modules/_abc/__init__.py create mode 100644 crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py.snap create mode 100644 crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py_builtins_allowed_modules.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_builtins/A005/modules/_abc/__init__.py b/crates/ruff_linter/resources/test/fixtures/flake8_builtins/A005/modules/_abc/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/crates/ruff_linter/src/rules/flake8_builtins/mod.rs b/crates/ruff_linter/src/rules/flake8_builtins/mod.rs index 3bc4d8f3f9..3c32ef0185 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/mod.rs @@ -36,6 +36,10 @@ mod tests { Rule::BuiltinModuleShadowing, Path::new("A005/modules/package/bisect.py") )] + #[test_case( + Rule::BuiltinModuleShadowing, + Path::new("A005/modules/_abc/__init__.py") + )] #[test_case(Rule::BuiltinModuleShadowing, Path::new("A005/modules/package/xml.py"))] #[test_case(Rule::BuiltinLambdaArgumentShadowing, Path::new("A006.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { @@ -91,6 +95,10 @@ mod tests { Rule::BuiltinModuleShadowing, Path::new("A005/modules/package/bisect.py") )] + #[test_case( + Rule::BuiltinModuleShadowing, + Path::new("A005/modules/_abc/__init__.py") + )] #[test_case(Rule::BuiltinModuleShadowing, Path::new("A005/modules/package/xml.py"))] fn builtins_allowed_modules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs index 254b390561..292cfc9520 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs @@ -1,7 +1,5 @@ use std::path::Path; -use crate::package::PackageRoot; -use crate::settings::types::PythonVersion; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::PySourceType; @@ -9,6 +7,9 @@ use ruff_python_stdlib::path::is_module_file; use ruff_python_stdlib::sys::is_known_standard_library; use ruff_text_size::TextRange; +use crate::package::PackageRoot; +use crate::settings::types::PythonVersion; + /// ## What it does /// Checks for modules that use the same names as Python builtin modules. /// @@ -47,25 +48,35 @@ pub(crate) fn builtin_module_shadowing( return None; } - if let Some(package) = package { - let module_name = if is_module_file(path) { - package.path().file_name().unwrap().to_string_lossy() - } else { - path.file_stem().unwrap().to_string_lossy() - }; + let package = package?; - if is_known_standard_library(target_version.minor(), &module_name) - && allowed_modules - .iter() - .all(|allowed_module| allowed_module != &module_name) - { - return Some(Diagnostic::new( - BuiltinModuleShadowing { - name: module_name.to_string(), - }, - TextRange::default(), - )); - } + let module_name = if is_module_file(path) { + package.path().file_name().unwrap().to_string_lossy() + } else { + path.file_stem().unwrap().to_string_lossy() + }; + + if !is_known_standard_library(target_version.minor(), &module_name) { + return None; } - None + + // Shadowing private stdlib modules is okay. + // https://github.com/astral-sh/ruff/issues/12949 + if module_name.starts_with('_') && !module_name.starts_with("__") { + return None; + } + + if allowed_modules + .iter() + .any(|allowed_module| allowed_module == &module_name) + { + return None; + } + + Some(Diagnostic::new( + BuiltinModuleShadowing { + name: module_name.to_string(), + }, + TextRange::default(), + )) } diff --git a/crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py.snap b/crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py.snap new file mode 100644 index 0000000000..d9ec358dcc --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py.snap @@ -0,0 +1,5 @@ +--- +source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs +snapshot_kind: text +--- + diff --git a/crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py_builtins_allowed_modules.snap b/crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py_builtins_allowed_modules.snap new file mode 100644 index 0000000000..d9ec358dcc --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_builtins/snapshots/ruff_linter__rules__flake8_builtins__tests__A005_A005__modules___abc____init__.py_builtins_allowed_modules.snap @@ -0,0 +1,5 @@ +--- +source: crates/ruff_linter/src/rules/flake8_builtins/mod.rs +snapshot_kind: text +--- +