Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-pyi] Implement PYI025 #4791

Merged
merged 6 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.py
Original file line number Diff line number Diff line change
@@ -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
)
19 changes: 19 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.pyi
Original file line number Diff line number Diff line change
@@ -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
)
19 changes: 13 additions & 6 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,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,
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/flake8_pyi/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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;
Original file line number Diff line number Diff line change
@@ -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<String> {
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,
));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff/src/rules/flake8_pyi/mod.rs
---

Original file line number Diff line number Diff line change
@@ -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`


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.