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

F401 - update documentation and deprecate ignore_init_module_imports #11436

Merged
merged 20 commits into from
May 21, 2024
Merged
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e78fee6
docs for updated F401 behavior #11168,#11314
plredmond May 15, 2024
c9ac09b
cargo insta review -- accept doc changes
plredmond May 15, 2024
fb1ac97
wording improvements in after zanie's comments
plredmond May 15, 2024
7706561
cargo insta review -- accept documentation change
plredmond May 15, 2024
36b4331
add deprecation warning for option lint.ignore_init_module_imports in…
plredmond May 20, 2024
63a9676
add module member name field to UnusedImport violation; redundant-ali…
plredmond May 20, 2024
f656ca4
cargo insta review -- accept change in diff for incorrect fix title
plredmond May 20, 2024
1bfa7b1
restore deprecated behavior for f401 when linter.ignore_init_module_i…
plredmond May 20, 2024
4f0b79e
tests for f401 in stable w/o deprecated option
plredmond May 20, 2024
3202de8
tests for f401 in stable w/ deprecated option: emit unsafe fixes to r…
plredmond May 20, 2024
97d1526
add deprecation message for `ignore-init-module-imports` in settings …
plredmond May 20, 2024
8d3ffbd
cargo insta review -- accept changed documentation
plredmond May 20, 2024
249d848
cargo dev generate-all
plredmond May 20, 2024
b6871c9
tweak boolean condition
plredmond May 20, 2024
715da2e
correct documentation formatting
plredmond May 20, 2024
21fe25e
cargo insta review -- accept documentation change
plredmond May 20, 2024
26f80c1
emit the deprecation warning if the option is set, regardless of valu…
plredmond May 21, 2024
2be489f
improve wording of deprecation warning by using zanie's suggestion
plredmond May 21, 2024
943ee25
list only the option, not information about it, in the docstring of a…
plredmond May 21, 2024
6028415
cargo insta review -- accept change in documentation
plredmond May 21, 2024
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
Prev Previous commit
Next Next commit
restore deprecated behavior for f401 when linter.ignore_init_module_i…
…mports=false: emit unsafe fixes to remove unused imports even in __init__.py
  • Loading branch information
plredmond committed May 20, 2024
commit 1bfa7b1ba18214b22dd9fd4f60f6b9e09b296cbc
13 changes: 11 additions & 2 deletions crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ enum UnusedImportContext {
Init {
first_party: bool,
dunder_all_count: usize,
ignore_init_module_imports: bool,
Copy link
Contributor Author

@plredmond plredmond May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is required to make fix titles follow the "old" behavior in __init__.py files. When we finally remove the option ignore_init_module_imports then this field ignore_init_module_imports can be deleted too.

},
}

Expand Down Expand Up @@ -91,6 +92,10 @@ enum UnusedImportContext {
/// print("numpy is not installed")
/// ```
///
/// ## Options
/// - Deprecated `lint.ignore-init-module-imports` to `true`. When set to `false`, unused imports
/// in `__init__.py` files are removed (unsafe).
///
/// ## References
/// - [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)
/// - [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)
Expand Down Expand Up @@ -140,11 +145,13 @@ impl Violation for UnusedImport {
Some(UnusedImportContext::Init {
first_party: true,
dunder_all_count: 1,
ignore_init_module_imports: true,
}) => Some(format!("Add unused import `{binding}` to __all__")),

Some(UnusedImportContext::Init {
first_party: true,
dunder_all_count: 0,
ignore_init_module_imports: true,
}) => Some(format!("Use an explicit re-export: `{module} as {module}`")),

_ => Some(if *multiple {
Expand Down Expand Up @@ -257,7 +264,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
}

let in_init = checker.path().ends_with("__init__.py");
let fix_init = checker.settings.preview.is_enabled();
let fix_init = !checker.settings.ignore_init_module_imports;
let preview_mode = checker.settings.preview.is_enabled();
Copy link
Contributor Author

@plredmond plredmond May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three lines look fishy because I'd originally replaced this

    let fix_init = !checker.settings.ignore_init_module_imports;

with

    let fix_init = checker.settings.preview.is_enabled();

which was perhaps lazy. In this PR I'm restoring the old behavior, and so gave checker.settings.preview.is_enabled(); its own name.

let dunder_all_exprs = find_dunder_all_exprs(checker.semantic());

// Generate a diagnostic for every import, but share fixes across all imports within the same
Expand Down Expand Up @@ -288,6 +296,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
checker,
),
dunder_all_count: dunder_all_exprs.len(),
ignore_init_module_imports: !fix_init,
})
} else {
None
Expand All @@ -301,7 +310,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
first_party: true,
..
})
)
) && preview_mode
});

// generate fixes that are shared across bindings in the statement
Expand Down