Skip to content

Commit

Permalink
Merge branch 'main' into reuse-of-groupby-generator
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 24, 2024
2 parents 4310853 + 021f0bd commit 4f688c2
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 37 deletions.
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
31 changes: 31 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pycodestyle/E402.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,37 @@
"metadata": {},
"outputs": [],
"source": []
},
{
"cell_type": "code",
"execution_count": null,
"id": "a51463ee-091c-44b4-9069-c03bf7e3bf83",
"metadata": {},
"outputs": [],
"source": [
"%%time\n",
"import pathlib"
]
},
{
"cell_type": "code",
"execution_count": null,
"id": "0ddc937e-6c19-475f-b108-9405aa1af4f1",
"metadata": {},
"outputs": [],
"source": []
},
{
"cell_type": "code",
"execution_count": null,
"id": "285041d2-a76c-4ff3-8ff2-0131bbf66016",
"metadata": {},
"outputs": [],
"source": [
"%%time\n",
"%%time\n",
"import pathlib"
]
}
],
"metadata": {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.semantic.flags |= SemanticModelFlags::MODULE_DOCSTRING_BOUNDARY;
self.semantic.flags |= SemanticModelFlags::FUTURES_BOUNDARY;
if !(self.semantic.seen_import_boundary()
|| stmt.is_ipy_escape_command_stmt()
|| helpers::is_assignment_to_a_dunder(stmt)
|| helpers::in_nested_block(self.semantic.current_statements())
|| imports::is_matplotlib_activation(stmt, self.semantic())
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ E402.ipynb:30:1: E402 Module level import not at top of cell
|
30 | import no_ok
| ^^^^^^^^^^^^ E402
31 |
32 | %%time
|


23 changes: 8 additions & 15 deletions crates/ruff_workspace/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,14 @@ impl<'a> Resolver<'a> {
let mut package_roots: FxHashMap<&Path, Option<&Path>> = FxHashMap::default();
for file in files {
if let Some(package) = file.parent() {
match package_roots.entry(package) {
std::collections::hash_map::Entry::Occupied(_) => continue,
std::collections::hash_map::Entry::Vacant(entry) => {
let namespace_packages = if has_namespace_packages {
self.resolve(file).linter.namespace_packages.as_slice()
} else {
&[]
};
entry.insert(detect_package_root_with_cache(
package,
namespace_packages,
&mut package_cache,
));
}
}
package_roots.entry(package).or_insert_with(|| {
let namespace_packages = if has_namespace_packages {
self.resolve(file).linter.namespace_packages.as_slice()
} else {
&[]
};
detect_package_root_with_cache(package, namespace_packages, &mut package_cache)
});
}
}

Expand Down

0 comments on commit 4f688c2

Please sign in to comment.