Skip to content

Commit

Permalink
Allow an unsafe fix
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Mar 12, 2024
1 parent a1b2a75 commit b8c4736
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 12 deletions.
5 changes: 5 additions & 0 deletions crates/ruff/tests/snapshots/integration_test__rule_f401.snap
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ marking it as unused, as in:
from module import member as member
```

## Fix safety

When `ignore_init_module_imports` is disabled, fixes can remove for unused imports in `__init__` files.
These fixes are considered unsafe because they can change the public interface.

## Example
```python
import numpy as np # unused import
Expand Down
34 changes: 26 additions & 8 deletions crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::borrow::Cow;
use anyhow::Result;
use rustc_hash::FxHashMap;

use ruff_diagnostics::{Diagnostic, Fix, FixAvailability, Violation};
use ruff_diagnostics::{Applicability, Diagnostic, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_semantic::{AnyImport, Exceptions, Imported, NodeId, Scope};
use ruff_text_size::{Ranged, TextRange};
Expand Down Expand Up @@ -37,6 +37,11 @@ enum UnusedImportContext {
/// from module import member as member
/// ```
///
/// ## Fix safety
///
/// When `ignore_init_module_imports` is disabled, fixes can remove for unused imports in `__init__` files.
/// These fixes are considered unsafe because they can change the public interface.
///
/// ## Example
/// ```python
/// import numpy as np # unused import
Expand Down Expand Up @@ -155,7 +160,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
}

let in_init = checker.path().ends_with("__init__.py");
let fix_init = in_init && !checker.settings.ignore_init_module_imports;
let fix_init = !checker.settings.ignore_init_module_imports;

// Generate a diagnostic for every import, but share a fix across all imports within the same
// statement (excluding those that are ignored).
Expand All @@ -164,8 +169,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
exceptions.intersects(Exceptions::MODULE_NOT_FOUND_ERROR | Exceptions::IMPORT_ERROR);
let multiple = imports.len() > 1;

let fix = if !fix_init && !in_except_handler {
fix_imports(checker, node_id, &imports).ok()
let fix = if (!in_init || fix_init) && !in_except_handler {
fix_imports(checker, node_id, &imports, in_init).ok()
} else {
None
};
Expand Down Expand Up @@ -243,7 +248,12 @@ impl Ranged for ImportBinding<'_> {
}

/// Generate a [`Fix`] to remove unused imports from a statement.
fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) -> Result<Fix> {
fn fix_imports(
checker: &Checker,
node_id: NodeId,
imports: &[ImportBinding],
in_init: bool,
) -> Result<Fix> {
let statement = checker.semantic().statement(node_id);
let parent = checker.semantic().parent_statement(node_id);

Expand All @@ -261,7 +271,15 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
checker.stylist(),
checker.indexer(),
)?;
Ok(Fix::safe_edit(edit).isolate(Checker::isolation(
checker.semantic().parent_statement_id(node_id),
)))
// It's unsafe to remove things from init because it can break public interfaces
let applicability = if in_init {
Applicability::Unsafe
} else {
Applicability::Safe
};
Ok(
Fix::applicable_edit(edit, applicability).isolate(Checker::isolation(
checker.semantic().parent_statement_id(node_id),
)),
)
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
__init__.py:1:8: F401 `os` imported but unused; consider adding to `__all__` or using a redundant alias
__init__.py:1:8: F401 [*] `os` imported but unused; consider adding to `__all__` or using a redundant alias
|
1 | import os
| ^^ F401
2 |
3 | print(__path__)
|
= help: Remove unused import: `os`

Unsafe fix
1 |-import os
2 1 |
3 2 | print(__path__)
4 3 |
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
__init__.py:1:8: F401 `os` imported but unused; consider adding to `__all__` or using a redundant alias
__init__.py:1:8: F401 [*] `os` imported but unused; consider adding to `__all__` or using a redundant alias
|
1 | import os
| ^^ F401
2 |
3 | print(__path__)
|
= help: Remove unused import: `os`

Unsafe fix
1 |-import os
2 1 |
3 2 | print(__path__)
4 3 |
4 changes: 2 additions & 2 deletions ruff.schema.json

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

0 comments on commit b8c4736

Please sign in to comment.