mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-28 12:55:05 +00:00
Remove F401
fix for __init__
imports by default and allow opt-in to unsafe fix (#10365)
Re-implementation of https://github.com/astral-sh/ruff/pull/5845 but instead of deprecating the option I toggle the default. Now users can _opt-in_ via the setting which will give them an unsafe fix to remove the import. Otherwise, we raise violations but do not offer a fix. The setting is a bit of a misnomer in either case, maybe we'll want to remove it still someday. As discussed there, I think the safe fix should be to import it as an alias. I'm not sure. We need support for offering multiple fixes for ideal behavior though? I think we should remove the fix entirely and consider it separately. Closes https://github.com/astral-sh/ruff/issues/5697 Supersedes https://github.com/astral-sh/ruff/pull/5845 --------- Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This commit is contained in:
parent
c2e15f38ee
commit
7b3ee2daff
10 changed files with 81 additions and 25 deletions
|
@ -34,6 +34,11 @@ marking it as unused, as in:
|
||||||
from module import member as member
|
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
|
## Example
|
||||||
```python
|
```python
|
||||||
import numpy as np # unused import
|
import numpy as np # unused import
|
||||||
|
|
|
@ -201,7 +201,7 @@ linter.allowed_confusables = []
|
||||||
linter.builtins = []
|
linter.builtins = []
|
||||||
linter.dummy_variable_rgx = ^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$
|
linter.dummy_variable_rgx = ^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$
|
||||||
linter.external = []
|
linter.external = []
|
||||||
linter.ignore_init_module_imports = false
|
linter.ignore_init_module_imports = true
|
||||||
linter.logger_objects = []
|
linter.logger_objects = []
|
||||||
linter.namespace_packages = []
|
linter.namespace_packages = []
|
||||||
linter.src = [
|
linter.src = [
|
||||||
|
|
|
@ -223,6 +223,19 @@ mod tests {
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn init_unused_import_opt_in_to_fix() -> Result<()> {
|
||||||
|
let diagnostics = test_path(
|
||||||
|
Path::new("pyflakes/__init__.py"),
|
||||||
|
&LinterSettings {
|
||||||
|
ignore_init_module_imports: false,
|
||||||
|
..LinterSettings::for_rules(vec![Rule::UnusedImport])
|
||||||
|
},
|
||||||
|
)?;
|
||||||
|
assert_messages!(diagnostics);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn default_builtins() -> Result<()> {
|
fn default_builtins() -> Result<()> {
|
||||||
let diagnostics = test_path(
|
let diagnostics = test_path(
|
||||||
|
|
|
@ -3,7 +3,7 @@ use std::borrow::Cow;
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use rustc_hash::FxHashMap;
|
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_macros::{derive_message_formats, violation};
|
||||||
use ruff_python_semantic::{AnyImport, Exceptions, Imported, NodeId, Scope};
|
use ruff_python_semantic::{AnyImport, Exceptions, Imported, NodeId, Scope};
|
||||||
use ruff_text_size::{Ranged, TextRange};
|
use ruff_text_size::{Ranged, TextRange};
|
||||||
|
@ -37,6 +37,11 @@ enum UnusedImportContext {
|
||||||
/// from module import member as member
|
/// 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
|
/// ## Example
|
||||||
/// ```python
|
/// ```python
|
||||||
/// import numpy as np # unused import
|
/// import numpy as np # unused import
|
||||||
|
@ -90,7 +95,7 @@ impl Violation for UnusedImport {
|
||||||
}
|
}
|
||||||
Some(UnusedImportContext::Init) => {
|
Some(UnusedImportContext::Init) => {
|
||||||
format!(
|
format!(
|
||||||
"`{name}` imported but unused; consider adding to `__all__` or using a redundant alias"
|
"`{name}` imported but unused; consider removing, adding to `__all__`, or using a redundant alias"
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
None => format!("`{name}` imported but unused"),
|
None => format!("`{name}` imported but unused"),
|
||||||
|
@ -154,8 +159,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let in_init =
|
let in_init = checker.path().ends_with("__init__.py");
|
||||||
checker.settings.ignore_init_module_imports && checker.path().ends_with("__init__.py");
|
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
|
// Generate a diagnostic for every import, but share a fix across all imports within the same
|
||||||
// statement (excluding those that are ignored).
|
// statement (excluding those that are ignored).
|
||||||
|
@ -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);
|
exceptions.intersects(Exceptions::MODULE_NOT_FOUND_ERROR | Exceptions::IMPORT_ERROR);
|
||||||
let multiple = imports.len() > 1;
|
let multiple = imports.len() > 1;
|
||||||
|
|
||||||
let fix = if !in_init && !in_except_handler {
|
let fix = if (!in_init || fix_init) && !in_except_handler {
|
||||||
fix_imports(checker, node_id, &imports).ok()
|
fix_imports(checker, node_id, &imports, in_init).ok()
|
||||||
} else {
|
} else {
|
||||||
None
|
None
|
||||||
};
|
};
|
||||||
|
@ -243,7 +248,12 @@ impl Ranged for ImportBinding<'_> {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Generate a [`Fix`] to remove unused imports from a statement.
|
/// 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 statement = checker.semantic().statement(node_id);
|
||||||
let parent = checker.semantic().parent_statement(node_id);
|
let parent = checker.semantic().parent_statement(node_id);
|
||||||
|
|
||||||
|
@ -261,7 +271,15 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
|
||||||
checker.stylist(),
|
checker.stylist(),
|
||||||
checker.indexer(),
|
checker.indexer(),
|
||||||
)?;
|
)?;
|
||||||
Ok(Fix::safe_edit(edit).isolate(Checker::isolation(
|
// It's unsafe to remove things from `__init__.py` because it can break public interfaces
|
||||||
checker.semantic().parent_statement_id(node_id),
|
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),
|
||||||
|
)),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
---
|
---
|
||||||
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
|
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
|
||||||
---
|
---
|
||||||
__init__.py:1:8: F401 [*] `os` imported but unused
|
__init__.py:1:8: F401 `os` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
|
||||||
|
|
|
|
||||||
1 | import os
|
1 | import os
|
||||||
| ^^ F401
|
| ^^ F401
|
||||||
|
@ -9,9 +9,3 @@ __init__.py:1:8: F401 [*] `os` imported but unused
|
||||||
3 | print(__path__)
|
3 | print(__path__)
|
||||||
|
|
|
|
||||||
= help: Remove unused import: `os`
|
= help: Remove unused import: `os`
|
||||||
|
|
||||||
ℹ Safe fix
|
|
||||||
1 |-import os
|
|
||||||
2 1 |
|
|
||||||
3 2 | print(__path__)
|
|
||||||
4 3 |
|
|
||||||
|
|
|
@ -0,0 +1,17 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
|
||||||
|
---
|
||||||
|
__init__.py:1:8: F401 [*] `os` imported but unused; consider removing, 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 |
|
|
@ -383,7 +383,7 @@ impl LinterSettings {
|
||||||
dummy_variable_rgx: DUMMY_VARIABLE_RGX.clone(),
|
dummy_variable_rgx: DUMMY_VARIABLE_RGX.clone(),
|
||||||
|
|
||||||
external: vec![],
|
external: vec![],
|
||||||
ignore_init_module_imports: false,
|
ignore_init_module_imports: true,
|
||||||
logger_objects: vec![],
|
logger_objects: vec![],
|
||||||
namespace_packages: vec![],
|
namespace_packages: vec![],
|
||||||
|
|
||||||
|
|
|
@ -237,6 +237,7 @@ impl Configuration {
|
||||||
project_root: project_root.to_path_buf(),
|
project_root: project_root.to_path_buf(),
|
||||||
},
|
},
|
||||||
|
|
||||||
|
#[allow(deprecated)]
|
||||||
linter: LinterSettings {
|
linter: LinterSettings {
|
||||||
rules: lint.as_rule_table(lint_preview)?,
|
rules: lint.as_rule_table(lint_preview)?,
|
||||||
exclude: FilePatternSet::try_from_iter(lint.exclude.unwrap_or_default())?,
|
exclude: FilePatternSet::try_from_iter(lint.exclude.unwrap_or_default())?,
|
||||||
|
@ -253,7 +254,7 @@ impl Configuration {
|
||||||
.dummy_variable_rgx
|
.dummy_variable_rgx
|
||||||
.unwrap_or_else(|| DUMMY_VARIABLE_RGX.clone()),
|
.unwrap_or_else(|| DUMMY_VARIABLE_RGX.clone()),
|
||||||
external: lint.external.unwrap_or_default(),
|
external: lint.external.unwrap_or_default(),
|
||||||
ignore_init_module_imports: lint.ignore_init_module_imports.unwrap_or_default(),
|
ignore_init_module_imports: lint.ignore_init_module_imports.unwrap_or(true),
|
||||||
line_length,
|
line_length,
|
||||||
tab_size: self.indent_width.unwrap_or_default(),
|
tab_size: self.indent_width.unwrap_or_default(),
|
||||||
namespace_packages: self.namespace_packages.unwrap_or_default(),
|
namespace_packages: self.namespace_packages.unwrap_or_default(),
|
||||||
|
@ -650,6 +651,10 @@ impl LintConfiguration {
|
||||||
.flatten()
|
.flatten()
|
||||||
.chain(options.common.extend_unfixable.into_iter().flatten())
|
.chain(options.common.extend_unfixable.into_iter().flatten())
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
|
#[allow(deprecated)]
|
||||||
|
let ignore_init_module_imports = options.common.ignore_init_module_imports;
|
||||||
|
|
||||||
Ok(LintConfiguration {
|
Ok(LintConfiguration {
|
||||||
exclude: options.exclude.map(|paths| {
|
exclude: options.exclude.map(|paths| {
|
||||||
paths
|
paths
|
||||||
|
@ -692,7 +697,7 @@ impl LintConfiguration {
|
||||||
})
|
})
|
||||||
.unwrap_or_default(),
|
.unwrap_or_default(),
|
||||||
external: options.common.external,
|
external: options.common.external,
|
||||||
ignore_init_module_imports: options.common.ignore_init_module_imports,
|
ignore_init_module_imports,
|
||||||
explicit_preview_rules: options.common.explicit_preview_rules,
|
explicit_preview_rules: options.common.explicit_preview_rules,
|
||||||
per_file_ignores: options.common.per_file_ignores.map(|per_file_ignores| {
|
per_file_ignores: options.common.per_file_ignores.map(|per_file_ignores| {
|
||||||
per_file_ignores
|
per_file_ignores
|
||||||
|
@ -1316,6 +1321,7 @@ fn warn_about_deprecated_top_level_lint_options(
|
||||||
used_options.push("extend-unsafe-fixes");
|
used_options.push("extend-unsafe-fixes");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[allow(deprecated)]
|
||||||
if top_level_options.ignore_init_module_imports.is_some() {
|
if top_level_options.ignore_init_module_imports.is_some() {
|
||||||
used_options.push("ignore-init-module-imports");
|
used_options.push("ignore-init-module-imports");
|
||||||
}
|
}
|
||||||
|
|
|
@ -693,11 +693,14 @@ pub struct LintCommonOptions {
|
||||||
/// imports will still be flagged, but with a dedicated message suggesting
|
/// imports will still be flagged, but with a dedicated message suggesting
|
||||||
/// that the import is either added to the module's `__all__` symbol, or
|
/// that the import is either added to the module's `__all__` symbol, or
|
||||||
/// re-exported with a redundant alias (e.g., `import os as os`).
|
/// re-exported with a redundant alias (e.g., `import os as os`).
|
||||||
|
///
|
||||||
|
/// This option is enabled by default, but you can opt-in to removal of imports
|
||||||
|
/// via an unsafe fix.
|
||||||
#[option(
|
#[option(
|
||||||
default = "false",
|
default = "true",
|
||||||
value_type = "bool",
|
value_type = "bool",
|
||||||
example = r#"
|
example = r#"
|
||||||
ignore-init-module-imports = true
|
ignore-init-module-imports = false
|
||||||
"#
|
"#
|
||||||
)]
|
)]
|
||||||
pub ignore_init_module_imports: Option<bool>,
|
pub ignore_init_module_imports: Option<bool>,
|
||||||
|
|
4
ruff.schema.json
generated
4
ruff.schema.json
generated
|
@ -424,7 +424,7 @@
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"ignore-init-module-imports": {
|
"ignore-init-module-imports": {
|
||||||
"description": "Avoid automatically removing unused imports in `__init__.py` files. Such imports will still be flagged, but with a dedicated message suggesting that the import is either added to the module's `__all__` symbol, or re-exported with a redundant alias (e.g., `import os as os`).",
|
"description": "Avoid automatically removing unused imports in `__init__.py` files. Such imports will still be flagged, but with a dedicated message suggesting that the import is either added to the module's `__all__` symbol, or re-exported with a redundant alias (e.g., `import os as os`).\n\nThis option is enabled by default, but you can opt-in to removal of imports via an unsafe fix.",
|
||||||
"deprecated": true,
|
"deprecated": true,
|
||||||
"type": [
|
"type": [
|
||||||
"boolean",
|
"boolean",
|
||||||
|
@ -2079,7 +2079,7 @@
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"ignore-init-module-imports": {
|
"ignore-init-module-imports": {
|
||||||
"description": "Avoid automatically removing unused imports in `__init__.py` files. Such imports will still be flagged, but with a dedicated message suggesting that the import is either added to the module's `__all__` symbol, or re-exported with a redundant alias (e.g., `import os as os`).",
|
"description": "Avoid automatically removing unused imports in `__init__.py` files. Such imports will still be flagged, but with a dedicated message suggesting that the import is either added to the module's `__all__` symbol, or re-exported with a redundant alias (e.g., `import os as os`).\n\nThis option is enabled by default, but you can opt-in to removal of imports via an unsafe fix.",
|
||||||
"type": [
|
"type": [
|
||||||
"boolean",
|
"boolean",
|
||||||
"null"
|
"null"
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue