Remove F401 fix for __init__ imports by default and allow opt-in …

…to unsafe fix (#10365)

Re-implementation of #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 #5697
Supersedes #5845

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

crates/ruff/tests/snapshots/integration_test__rule_f401.snap

@@ -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

crates/ruff/tests/snapshots/show_settings__display_default_settings.snap

@@ -201,7 +201,7 @@ linter.allowed_confusables = []
 linter.builtins = []
 linter.dummy_variable_rgx = ^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$
 linter.external = []
-linter.ignore_init_module_imports = false
+linter.ignore_init_module_imports = true
 linter.logger_objects = []
 linter.namespace_packages = []
 linter.src = [

crates/ruff_linter/src/rules/pyflakes/mod.rs

@@ -223,6 +223,19 @@ mod tests {
         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]
     fn default_builtins() -> Result<()> {
         let diagnostics = test_path(

crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs

@@ -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};
@@ -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
@@ -90,7 +95,7 @@ impl Violation for UnusedImport {
             }
             Some(UnusedImportContext::Init) => {
                 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"),
@@ -154,8 +159,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
         }
     }
 
-    let in_init =
-        checker.settings.ignore_init_module_imports && checker.path().ends_with("__init__.py");
+    let in_init = 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
     // 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);
         let multiple = imports.len() > 1;
 
-        let fix = if !in_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
         };
@@ -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);
 
@@ -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__.py` 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),
+        )),
+    )
 }

crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__init.snap

@@ -1,17 +1,11 @@
 ---
 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
   |        ^^ F401
 2 | 
 3 | print(__path__)
   |
   = help: Remove unused import: `os`
-
-ℹ Safe fix
-1   |-import os
-2 1 | 
-3 2 | print(__path__)
-4 3 |

crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__init_unused_import_opt_in_to_fix.snap

@@ -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 |

crates/ruff_linter/src/settings/mod.rs

@@ -383,7 +383,7 @@ impl LinterSettings {
             dummy_variable_rgx: DUMMY_VARIABLE_RGX.clone(),
 
             external: vec![],
-            ignore_init_module_imports: false,
+            ignore_init_module_imports: true,
             logger_objects: vec![],
             namespace_packages: vec![],
 

crates/ruff_workspace/src/configuration.rs

@@ -237,6 +237,7 @@ impl Configuration {
                 project_root: project_root.to_path_buf(),
             },
 
+            #[allow(deprecated)]
             linter: LinterSettings {
                 rules: lint.as_rule_table(lint_preview)?,
                 exclude: FilePatternSet::try_from_iter(lint.exclude.unwrap_or_default())?,
@@ -253,7 +254,7 @@ impl Configuration {
                     .dummy_variable_rgx
                     .unwrap_or_else(|| DUMMY_VARIABLE_RGX.clone()),
                 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,
                 tab_size: self.indent_width.unwrap_or_default(),
                 namespace_packages: self.namespace_packages.unwrap_or_default(),
@@ -650,6 +651,10 @@ impl LintConfiguration {
             .flatten()
             .chain(options.common.extend_unfixable.into_iter().flatten())
             .collect();
+
+        #[allow(deprecated)]
+        let ignore_init_module_imports = options.common.ignore_init_module_imports;
+
         Ok(LintConfiguration {
             exclude: options.exclude.map(|paths| {
                 paths
@@ -692,7 +697,7 @@ impl LintConfiguration {
                 })
                 .unwrap_or_default(),
             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,
             per_file_ignores: options.common.per_file_ignores.map(|per_file_ignores| {
                 per_file_ignores
@@ -1316,6 +1321,7 @@ fn warn_about_deprecated_top_level_lint_options(
         used_options.push("extend-unsafe-fixes");
     }
 
+    #[allow(deprecated)]
     if top_level_options.ignore_init_module_imports.is_some() {
         used_options.push("ignore-init-module-imports");
     }

crates/ruff_workspace/src/options.rs

@@ -693,11 +693,14 @@ pub struct LintCommonOptions {
     /// 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`).
+    ///
+    /// This option is enabled by default, but you can opt-in to removal of imports
+    /// via an unsafe fix.
     #[option(
-        default = "false",
+        default = "true",
         value_type = "bool",
         example = r#"
-            ignore-init-module-imports = true
+            ignore-init-module-imports = false
         "#
     )]
     pub ignore_init_module_imports: Option<bool>,