From 6d94aa89e3cef67ef24d2997f7cb458f81cf794d Mon Sep 17 00:00:00 2001 From: qdegraaf <34540841+qdegraaf@users.noreply.github.com> Date: Fri, 2 Jun 2023 00:45:31 +0200 Subject: [PATCH] [`flake8-pyi`] Implement `PYI025` (#4791) --- .../test/fixtures/flake8_pyi/PYI025.py | 19 ++++++ .../test/fixtures/flake8_pyi/PYI025.pyi | 19 ++++++ crates/ruff/src/checkers/ast/mod.rs | 19 ++++-- crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/flake8_pyi/mod.rs | 2 + crates/ruff/src/rules/flake8_pyi/rules/mod.rs | 4 ++ .../unaliased_collections_abc_set_import.rs | 62 +++++++++++++++++++ ...__flake8_pyi__tests__PYI025_PYI025.py.snap | 4 ++ ..._flake8_pyi__tests__PYI025_PYI025.pyi.snap | 22 +++++++ ruff.schema.json | 1 + 11 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.py create mode 100644 crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.pyi create mode 100644 crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.py.snap create mode 100644 crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.pyi.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.py b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.py new file mode 100644 index 0000000000..5c8b8fa26b --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.py @@ -0,0 +1,19 @@ +from collections.abc import Set as AbstractSet # Ok + + +from collections.abc import Set # Ok + + +from collections.abc import ( + Container, + Sized, + Set, # Ok + ValuesView +) + +from collections.abc import ( + Container, + Sized, + Set as AbstractSet, # Ok + ValuesView +) diff --git a/crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.pyi b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.pyi new file mode 100644 index 0000000000..c12ccdffb5 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.pyi @@ -0,0 +1,19 @@ +from collections.abc import Set as AbstractSet # Ok + + +from collections.abc import Set # PYI025 + + +from collections.abc import ( + Container, + Sized, + Set, # PYI025 + ValuesView +) + +from collections.abc import ( + Container, + Sized, + Set as AbstractSet, + ValuesView # Ok +) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 5df71e15aa..97c328059d 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -1021,12 +1021,14 @@ where } } } - Stmt::ImportFrom(ast::StmtImportFrom { - names, - module, - level, - range: _, - }) => { + Stmt::ImportFrom( + import_from @ ast::StmtImportFrom { + names, + module, + level, + range: _, + }, + ) => { let module = module.as_deref(); let level = level.map(|level| level.to_u32()); if self.enabled(Rule::ModuleImportNotAtTopOfFile) { @@ -1091,6 +1093,11 @@ where } } + if self.is_stub { + if self.enabled(Rule::UnaliasedCollectionsAbcSetImport) { + flake8_pyi::rules::unaliased_collections_abc_set_import(self, import_from); + } + } for alias in names { if let Some("__future__") = module { let name = alias.asname.as_ref().unwrap_or(&alias.name); diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 582a10f0ea..75c56b5cfe 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -593,6 +593,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pyi, "020") => (RuleGroup::Unspecified, Rule::QuotedAnnotationInStub), (Flake8Pyi, "021") => (RuleGroup::Unspecified, Rule::DocstringInStub), (Flake8Pyi, "024") => (RuleGroup::Unspecified, Rule::CollectionsNamedTuple), + (Flake8Pyi, "025") => (RuleGroup::Unspecified, Rule::UnaliasedCollectionsAbcSetImport), (Flake8Pyi, "032") => (RuleGroup::Unspecified, Rule::AnyEqNeAnnotation), (Flake8Pyi, "033") => (RuleGroup::Unspecified, Rule::TypeCommentInStub), (Flake8Pyi, "042") => (RuleGroup::Unspecified, Rule::SnakeCaseTypeAlias), diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 2f4c024abd..92a21b475d 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -529,6 +529,7 @@ ruff_macros::register_rules!( rules::flake8_pyi::rules::TSuffixedTypeAlias, rules::flake8_pyi::rules::TypeCommentInStub, rules::flake8_pyi::rules::TypedArgumentDefaultInStub, + rules::flake8_pyi::rules::UnaliasedCollectionsAbcSetImport, rules::flake8_pyi::rules::UnannotatedAssignmentInStub, rules::flake8_pyi::rules::UnprefixedTypeParam, rules::flake8_pyi::rules::UnrecognizedPlatformCheck, diff --git a/crates/ruff/src/rules/flake8_pyi/mod.rs b/crates/ruff/src/rules/flake8_pyi/mod.rs index 5d242bb333..c724ff4df1 100644 --- a/crates/ruff/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/mod.rs @@ -48,6 +48,8 @@ mod tests { #[test_case(Rule::TypeCommentInStub, Path::new("PYI033.pyi"))] #[test_case(Rule::TypedArgumentDefaultInStub, Path::new("PYI011.py"))] #[test_case(Rule::TypedArgumentDefaultInStub, Path::new("PYI011.pyi"))] + #[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025.py"))] + #[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025.pyi"))] #[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.py"))] #[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.pyi"))] #[test_case(Rule::UnprefixedTypeParam, Path::new("PYI001.py"))] diff --git a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs index 2fe699df83..7978118cd8 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/mod.rs @@ -28,6 +28,9 @@ pub(crate) use type_alias_naming::{ snake_case_type_alias, t_suffixed_type_alias, SnakeCaseTypeAlias, TSuffixedTypeAlias, }; pub(crate) use type_comment_in_stub::{type_comment_in_stub, TypeCommentInStub}; +pub(crate) use unaliased_collections_abc_set_import::{ + unaliased_collections_abc_set_import, UnaliasedCollectionsAbcSetImport, +}; pub(crate) use unrecognized_platform::{ unrecognized_platform, UnrecognizedPlatformCheck, UnrecognizedPlatformName, }; @@ -48,4 +51,5 @@ mod simple_defaults; mod stub_body_multiple_statements; mod type_alias_naming; mod type_comment_in_stub; +mod unaliased_collections_abc_set_import; mod unrecognized_platform; diff --git a/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs b/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs new file mode 100644 index 0000000000..db17d9c272 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs @@ -0,0 +1,62 @@ +use rustpython_parser::ast::StmtImportFrom; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `from collections.abc import Set` imports that do not alias +/// `Set` to `AbstractSet`. +/// +/// ## Why is this bad? +/// The `Set` type in `collections.abc` is an abstract base class for set-like types. +/// It is easily confused with, and not equivalent to, the `set` builtin. +/// +/// To avoid confusion, `Set` should be aliased to `AbstractSet` when imported. This +/// makes it clear that the imported type is an abstract base class, and not the +/// `set` builtin. +/// +/// ## Example +/// ```python +/// from collections.abc import Set +/// ``` +/// +/// Use instead: +/// ```python +/// from collections.abc import Set as AbstractSet +/// ``` +#[violation] +pub struct UnaliasedCollectionsAbcSetImport; + +impl Violation for UnaliasedCollectionsAbcSetImport { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin" + ) + } + + fn autofix_title(&self) -> Option { + Some(format!("Alias `Set` to `AbstractSet`")) + } +} + +/// PYI025 +pub(crate) fn unaliased_collections_abc_set_import(checker: &mut Checker, stmt: &StmtImportFrom) { + let Some(module_id) = &stmt.module else { + return; + }; + if module_id.as_str() != "collections.abc" { + return; + } + + for name in &stmt.names { + if name.name.as_str() == "Set" && name.asname.is_none() { + checker.diagnostics.push(Diagnostic::new( + UnaliasedCollectionsAbcSetImport, + name.range, + )); + } + } +} diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.py.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.py.snap new file mode 100644 index 0000000000..d1aa2e9116 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.py.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- + diff --git a/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.pyi.snap b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.pyi.snap new file mode 100644 index 0000000000..bcba8d1bf5 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.pyi.snap @@ -0,0 +1,22 @@ +--- +source: crates/ruff/src/rules/flake8_pyi/mod.rs +--- +PYI025.pyi:4:29: PYI025 Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin + | +4 | from collections.abc import Set # PYI025 + | ^^^ PYI025 + | + = help: Alias `Set` to `AbstractSet` + +PYI025.pyi:10:5: PYI025 Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin + | +10 | Container, +11 | Sized, +12 | Set, # PYI025 + | ^^^ PYI025 +13 | ValuesView +14 | ) + | + = help: Alias `Set` to `AbstractSet` + + diff --git a/ruff.schema.json b/ruff.schema.json index 9922b71da5..e07a609d39 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2240,6 +2240,7 @@ "PYI020", "PYI021", "PYI024", + "PYI025", "PYI03", "PYI032", "PYI033",