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

Mark PYI025 fix as safe in more cases for stub files #10547

Merged
merged 1 commit into from
Mar 24, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
Tests for PYI025 where the import is marked as re-exported
through usage of a "redundant" `import Set as Set` alias
"""

from collections.abc import Set as Set # PYI025 triggered but fix is not marked as safe
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
Tests for PYI025 where the import is marked as re-exported
through usage of a "redundant" `import Set as Set` alias
"""

from collections.abc import Set as Set # PYI025 triggered but fix is not marked as safe
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_pyi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ mod tests {
#[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025_1.pyi"))]
#[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025_2.py"))]
#[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025_2.pyi"))]
#[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025_3.py"))]
#[test_case(Rule::UnaliasedCollectionsAbcSetImport, Path::new("PYI025_3.pyi"))]
#[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.py"))]
#[test_case(Rule::UnannotatedAssignmentInStub, Path::new("PYI052.pyi"))]
#[test_case(Rule::UnassignedSpecialVariableInStub, Path::new("PYI035.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::Imported;
use ruff_python_semantic::{Binding, BindingKind};
use ruff_python_semantic::{Binding, BindingKind, Scope};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -32,9 +32,14 @@ use crate::renamer::Renamer;
///
/// ## Fix safety
/// This rule's fix is marked as unsafe for `Set` imports defined at the
/// top-level of a module. Top-level symbols are implicitly exported by the
/// top-level of a `.py` module. Top-level symbols are implicitly exported by the
/// module, and so renaming a top-level symbol may break downstream modules
/// that import it.
///
/// The same is not true for `.pyi` files, where imported symbols are only
/// re-exported if they are included in `__all__`, use a "redundant"
/// `import foo as foo` alias, or are imported via a `*` import. As such, the
/// fix is marked as safe in more cases for `.pyi` files.
#[violation]
pub struct UnaliasedCollectionsAbcSetImport;

Expand Down Expand Up @@ -76,24 +81,41 @@ pub(crate) fn unaliased_collections_abc_set_import(
let mut diagnostic = Diagnostic::new(UnaliasedCollectionsAbcSetImport, binding.range());
if checker.semantic().is_available("AbstractSet") {
diagnostic.try_set_fix(|| {
let scope = &checker.semantic().scopes[binding.scope];
let (edit, rest) = Renamer::rename(
name,
"AbstractSet",
scope,
checker.semantic(),
checker.stylist(),
)?;
Ok(Fix::applicable_edits(
edit,
rest,
if scope.kind.is_module() {
Applicability::Unsafe
} else {
Applicability::Safe
},
))
let semantic = checker.semantic();
let scope = &semantic.scopes[binding.scope];
let (edit, rest) =
Renamer::rename(name, "AbstractSet", scope, semantic, checker.stylist())?;
let applicability = determine_applicability(binding, scope, checker);
Ok(Fix::applicable_edits(edit, rest, applicability))
});
}
Some(diagnostic)
}

fn determine_applicability(binding: &Binding, scope: &Scope, checker: &Checker) -> Applicability {
// If it's not in a module scope, the import can't be implicitly re-exported,
// so always mark it as safe
if !scope.kind.is_module() {
return Applicability::Safe;
}
// If it's not a stub and it's in the module scope, always mark the fix as unsafe
if !checker.source_type.is_stub() {
return Applicability::Unsafe;
}
// If the import was `from collections.abc import Set as Set`,
// it's being explicitly re-exported: mark the fix as unsafe
if binding.is_explicit_export() {
return Applicability::Unsafe;
}
// If it's included in `__all__`, mark the fix as unsafe
if binding.references().any(|reference| {
checker
.semantic()
.reference(reference)
.in_dunder_all_definition()
}) {
return Applicability::Unsafe;
}
// Anything else in a stub, and it's a safe fix:
Applicability::Safe
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ PYI025_1.pyi:33:29: PYI025 [*] Use `from collections.abc import Set as AbstractS
|
= help: Alias `Set` to `AbstractSet`

Unsafe fix
Safe fix
17 17 | else:
18 18 | Set = 1
19 19 |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
---
PYI025_3.py:6:36: PYI025 [*] Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin
|
4 | """
5 |
6 | from collections.abc import Set as Set # PYI025 triggered but fix is not marked as safe
| ^^^ PYI025
|
= help: Alias `Set` to `AbstractSet`

ℹ Unsafe fix
3 3 | through usage of a "redundant" `import Set as Set` alias
4 4 | """
5 5 |
6 |-from collections.abc import Set as Set # PYI025 triggered but fix is not marked as safe
6 |+from collections.abc import Set as AbstractSet # PYI025 triggered but fix is not marked as safe
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
source: crates/ruff_linter/src/rules/flake8_pyi/mod.rs
---
PYI025_3.pyi:6:36: PYI025 [*] Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin
|
4 | """
5 |
6 | from collections.abc import Set as Set # PYI025 triggered but fix is not marked as safe
| ^^^ PYI025
|
= help: Alias `Set` to `AbstractSet`

ℹ Unsafe fix
3 3 | through usage of a "redundant" `import Set as Set` alias
4 4 | """
5 5 |
6 |-from collections.abc import Set as Set # PYI025 triggered but fix is not marked as safe
6 |+from collections.abc import Set as AbstractSet # PYI025 triggered but fix is not marked as safe
Loading