Flag, but don't fix, unused imports in ModuleNotFoundError blocks (#…

…3658)

crates/ruff/resources/test/fixtures/pyflakes/F401_10.py

@@ -1,10 +1,19 @@
-"""Test: imports within `ModuleNotFoundError` handlers."""
+"""Test: imports within `ModuleNotFoundError` and `ImportError` handlers."""
 
 
-def check_orjson():
+def module_not_found_error():
     try:
         import orjson
 
         return True
     except ModuleNotFoundError:
         return False
+
+
+def import_error():
+    try:
+        import orjson
+
+        return True
+    except ImportError:
+        return False

crates/ruff/src/rules/pyflakes/rules/imports.rs

@@ -9,46 +9,52 @@ use ruff_python_stdlib::future::ALL_FEATURE_NAMES;
 
 use crate::checkers::ast::Checker;
 
+#[derive(Debug, PartialEq, Eq)]
+pub enum UnusedImportContext {
+    ExceptHandler,
+    Init,
+}
+
 #[violation]
 pub struct UnusedImport {
     pub name: String,
-    pub ignore_init: bool,
+    pub context: Option<UnusedImportContext>,
     pub multiple: bool,
 }
 
-fn fmt_unused_import_autofix_msg(unused_import: &UnusedImport) -> String {
-    let UnusedImport { name, multiple, .. } = unused_import;
-    if *multiple {
-        "Remove unused import".to_string()
-    } else {
-        format!("Remove unused import: `{name}`")
-    }
-}
 impl Violation for UnusedImport {
     const AUTOFIX: AutofixKind = AutofixKind::Sometimes;
 
     #[derive_message_formats]
     fn message(&self) -> String {
-        let UnusedImport {
-            name, ignore_init, ..
-        } = self;
-        if *ignore_init {
-            format!(
-                "`{name}` imported but unused; consider adding to `__all__` or using a redundant \
-                 alias"
-            )
-        } else {
-            format!("`{name}` imported but unused")
+        let UnusedImport { name, context, .. } = self;
+        match context {
+            Some(UnusedImportContext::ExceptHandler) => {
+                format!(
+                    "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability"
+                )
+            }
+            Some(UnusedImportContext::Init) => {
+                format!(
+                    "`{name}` imported but unused; consider adding to `__all__` or using a redundant \
+                     alias"
+                )
+            }
+            None => format!("`{name}` imported but unused"),
         }
     }
 
     fn autofix_title_formatter(&self) -> Option<fn(&Self) -> String> {
-        let UnusedImport { ignore_init, .. } = self;
-        if *ignore_init {
-            None
-        } else {
-            Some(fmt_unused_import_autofix_msg)
-        }
+        let UnusedImport { context, .. } = self;
+        context
+            .is_none()
+            .then_some(|UnusedImport { name, multiple, .. }| {
+                if *multiple {
+                    "Remove unused import".to_string()
+                } else {
+                    format!("Remove unused import: `{name}`")
+                }
+            })
     }
 }
 #[violation]

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

@@ -10,7 +10,7 @@ pub use if_tuple::{if_tuple, IfTuple};
 pub use imports::{
     future_feature_not_defined, FutureFeatureNotDefined, ImportShadowedByLoopVar, LateFutureImport,
     UndefinedLocalWithImportStar, UndefinedLocalWithImportStarUsage,
-    UndefinedLocalWithNestedImportStarUsage, UnusedImport,
+    UndefinedLocalWithNestedImportStarUsage, UnusedImport, UnusedImportContext,
 };
 pub use invalid_literal_comparisons::{invalid_literal_comparison, IsLiteral};
 pub use invalid_print_syntax::{invalid_print_syntax, InvalidPrintSyntax};

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_10.py.snap

@@ -2,5 +2,37 @@
 source: crates/ruff/src/rules/pyflakes/mod.rs
 expression: diagnostics
 ---
-[]
+- kind:
+    name: UnusedImport
+    body: "`orjson` imported but unused; consider using `importlib.util.find_spec` to test for availability"
+    suggestion: ~
+    fixable: false
+  location:
+    row: 6
+    column: 15
+  end_location:
+    row: 6
+    column: 21
+  fix: ~
+  parent: ~
+- kind:
+    name: UnusedImport
+    body: "`orjson` imported but unused"
+    suggestion: "Remove unused import: `orjson`"
+    fixable: true
+  location:
+    row: 15
+    column: 15
+  end_location:
+    row: 15
+    column: 21
+  fix:
+    content: ""
+    location:
+      row: 15
+      column: 0
+    end_location:
+      row: 16
+      column: 0
+  parent: ~
 

crates/ruff_python_ast/src/context.rs

@@ -8,10 +8,10 @@ use smallvec::smallvec;
 use ruff_python_stdlib::path::is_python_stub_file;
 use ruff_python_stdlib::typing::TYPING_EXTENSIONS;
 
-use crate::helpers::{collect_call_path, from_relative_import, Exceptions};
+use crate::helpers::{collect_call_path, from_relative_import};
 use crate::scope::{
-    Binding, BindingId, BindingKind, Bindings, ExecutionContext, Scope, ScopeId, ScopeKind,
-    ScopeStack, Scopes,
+    Binding, BindingId, BindingKind, Bindings, Exceptions, ExecutionContext, Scope, ScopeId,
+    ScopeKind, ScopeStack, Scopes,
 };
 use crate::types::{CallPath, RefEquality};
 use crate::typing::AnnotationKind;
@@ -309,6 +309,7 @@ impl<'a> Context<'a> {
         self.in_exception_handler
     }
 
+    /// Return the [`ExecutionContext`] of the current scope.
     pub const fn execution_context(&self) -> ExecutionContext {
         if self.in_type_checking_block
             || self.in_annotation
@@ -319,4 +320,13 @@ impl<'a> Context<'a> {
             ExecutionContext::Runtime
         }
     }
+
+    /// Return the union of all handled exceptions as an [`Exceptions`] bitflag.
+    pub fn exceptions(&self) -> Exceptions {
+        let mut exceptions = Exceptions::empty();
+        for exception in &self.handled_exceptions {
+            exceptions.insert(*exception);
+        }
+        exceptions
+    }
 }

crates/ruff_python_ast/src/helpers.rs

@@ -1,6 +1,5 @@
 use std::path::Path;
 
-use bitflags::bitflags;
 use itertools::Itertools;
 use log::error;
 use once_cell::sync::Lazy;
@@ -577,13 +576,6 @@ pub fn has_non_none_keyword(keywords: &[Keyword], keyword: &str) -> bool {
     })
 }
 
-bitflags! {
-    pub struct Exceptions: u32 {
-        const NAME_ERROR = 0b0000_0001;
-        const MODULE_NOT_FOUND_ERROR = 0b0000_0010;
-    }
-}
-
 /// Extract the names of all handled exceptions.
 pub fn extract_handled_exceptions(handlers: &[Excepthandler]) -> Vec<&Expr> {
     let mut handled_exceptions = Vec::new();

crates/ruff_python_ast/src/scope.rs

@@ -1,4 +1,5 @@
 use crate::types::{Range, RefEquality};
+use bitflags::bitflags;
 use rustc_hash::FxHashMap;
 use rustpython_parser::ast::{Arguments, Expr, Keyword, Stmt};
 use std::num::TryFromIntError;
@@ -222,6 +223,14 @@ impl Default for ScopeStack {
     }
 }
 
+bitflags! {
+    pub struct Exceptions: u32 {
+        const NAME_ERROR = 0b0000_0001;
+        const MODULE_NOT_FOUND_ERROR = 0b0000_0010;
+        const IMPORT_ERROR = 0b0000_0100;
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct Binding<'a> {
     pub kind: BindingKind<'a>,
@@ -241,6 +250,8 @@ pub struct Binding<'a> {
     /// (e.g.) `__future__` imports, explicit re-exports, and other bindings
     /// that should be considered used even if they're never referenced.
     pub synthetic_usage: Option<(ScopeId, Range)>,
+    /// The exceptions that were handled when the binding was defined.
+    pub exceptions: Exceptions,
 }
 
 impl<'a> Binding<'a> {