Ignore unused imports in `ModuleNotFoundError` blocks (#3288)

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

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

crates/ruff/src/ast/helpers.rs

@@ -1,5 +1,6 @@
 use std::path::Path;
 
+use bitflags::bitflags;
 use itertools::Itertools;
 use log::error;
 use once_cell::sync::Lazy;
@@ -575,33 +576,32 @@ 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_handler_names(handlers: &[Excepthandler]) -> Vec<CallPath> {
+pub fn extract_handled_exceptions(handlers: &[Excepthandler]) -> Vec<&Expr> {
-    // TODO(charlie): Use `resolve_call_path` to avoid false positives for
+    let mut handled_exceptions = Vec::new();
-    // overridden builtins.
-    let mut handler_names = vec![];
     for handler in handlers {
         match &handler.node {
             ExcepthandlerKind::ExceptHandler { type_, .. } => {
                 if let Some(type_) = type_ {
                     if let ExprKind::Tuple { elts, .. } = &type_.node {
                         for type_ in elts {
-                            let call_path = collect_call_path(type_);
+                            handled_exceptions.push(type_);
-                            if !call_path.is_empty() {
-                                handler_names.push(call_path);
-                            }
                         }
                     } else {
-                        let call_path = collect_call_path(type_);
+                        handled_exceptions.push(type_);
-                        if !call_path.is_empty() {
-                            handler_names.push(call_path);
                     }
                 }
             }
         }
     }
-    }
+    handled_exceptions
-    handler_names
 }
 
 /// Return the set of all bound argument names.

crates/ruff/src/checkers/ast.rs

@@ -19,7 +19,8 @@ use rustpython_parser::ast::{
 use smallvec::smallvec;
 
 use crate::ast::helpers::{
-    binding_range, collect_call_path, extract_handler_names, from_relative_import, to_module_path,
+    binding_range, collect_call_path, extract_handled_exceptions, from_relative_import,
+    to_module_path, Exceptions,
 };
 use crate::ast::operations::{extract_all_names, AllNamesFlags};
 use crate::ast::relocate::relocate_expr;
@@ -109,7 +110,7 @@ pub struct Checker<'a> {
     pub(crate) seen_import_boundary: bool,
     futures_allowed: bool,
     annotations_future_enabled: bool,
-    except_handlers: Vec<Vec<Vec<&'a str>>>,
+    handled_exceptions: Vec<Exceptions>,
     // Check-specific state.
     pub(crate) flake8_bugbear_seen: Vec<&'a Expr>,
 }
@@ -178,7 +179,7 @@ impl<'a> Checker<'a> {
             seen_import_boundary: false,
             futures_allowed: true,
             annotations_future_enabled: is_interface_definition,
-            except_handlers: vec![],
+            handled_exceptions: vec![],
             // Check-specific state.
             flake8_bugbear_seen: vec![],
         }
@@ -970,6 +971,13 @@ where
                     pyupgrade::rules::rewrite_mock_import(self, stmt);
                 }
 
+                // If a module is imported within a `ModuleNotFoundError` body, treat that as a
+                // synthetic usage.
+                let is_handled = self
+                    .handled_exceptions
+                    .iter()
+                    .any(|exceptions| exceptions.contains(Exceptions::MODULE_NOT_FOUND_ERROR));
+
                 for alias in names {
                     if alias.node.name == "__future__" {
                         let name = alias.node.asname.as_ref().unwrap_or(&alias.node.name);
@@ -1012,7 +1020,18 @@ where
                             Binding {
                                 kind: BindingKind::SubmoduleImportation(name, full_name),
                                 runtime_usage: None,
-                                synthetic_usage: None,
+                                synthetic_usage: if is_handled {
+                                    Some((
+                                        self.scopes[*(self
+                                            .scope_stack
+                                            .last()
+                                            .expect("No current scope found"))]
+                                        .id,
+                                        Range::from_located(alias),
+                                    ))
+                                } else {
+                                    None
+                                },
                                 typing_usage: None,
                                 range: Range::from_located(alias),
                                 source: Some(self.current_stmt().clone()),
@@ -1020,6 +1039,14 @@ where
                             },
                         );
                     } else {
+                        // Treat explicit re-export as usage (e.g., `from .applications
+                        // import FastAPI as FastAPI`).
+                        let is_explicit_reexport = alias
+                            .node
+                            .asname
+                            .as_ref()
+                            .map_or(false, |asname| asname == &alias.node.name);
+
                         // Given `import foo`, `name` and `full_name` would both be `foo`.
                         // Given `import foo as bar`, `name` would be `bar` and `full_name` would
                         // be `foo`.
@@ -1030,14 +1057,7 @@ where
                             Binding {
                                 kind: BindingKind::Importation(name, full_name),
                                 runtime_usage: None,
-                                // Treat explicit re-export as usage (e.g., `import applications
+                                synthetic_usage: if is_handled || is_explicit_reexport {
-                                // as applications`).
-                                synthetic_usage: if alias
-                                    .node
-                                    .asname
-                                    .as_ref()
-                                    .map_or(false, |asname| asname == &alias.node.name)
-                                {
                                     Some((
                                         self.scopes[*(self
                                             .scope_stack
@@ -1293,6 +1313,13 @@ where
                     }
                 }
 
+                // If a module is imported within a `ModuleNotFoundError` body, treat that as a
+                // synthetic usage.
+                let is_handled = self
+                    .handled_exceptions
+                    .iter()
+                    .any(|exceptions| exceptions.contains(Exceptions::MODULE_NOT_FOUND_ERROR));
+
                 for alias in names {
                     if let Some("__future__") = module.as_deref() {
                         let name = alias.node.asname.as_ref().unwrap_or(&alias.node.name);
@@ -1339,7 +1366,18 @@ where
                             Binding {
                                 kind: BindingKind::StarImportation(*level, module.clone()),
                                 runtime_usage: None,
-                                synthetic_usage: None,
+                                synthetic_usage: if is_handled {
+                                    Some((
+                                        self.scopes[*(self
+                                            .scope_stack
+                                            .last()
+                                            .expect("No current scope found"))]
+                                        .id,
+                                        Range::from_located(alias),
+                                    ))
+                                } else {
+                                    None
+                                },
                                 typing_usage: None,
                                 range: Range::from_located(stmt),
                                 source: Some(self.current_stmt().clone()),
@@ -1383,6 +1421,14 @@ where
                             self.check_builtin_shadowing(asname, stmt, false);
                         }
 
+                        // Treat explicit re-export as usage (e.g., `from .applications
+                        // import FastAPI as FastAPI`).
+                        let is_explicit_reexport = alias
+                            .node
+                            .asname
+                            .as_ref()
+                            .map_or(false, |asname| asname == &alias.node.name);
+
                         // Given `from foo import bar`, `name` would be "bar" and `full_name` would
                         // be "foo.bar". Given `from foo import bar as baz`, `name` would be "baz"
                         // and `full_name` would be "foo.bar".
@@ -1392,33 +1438,25 @@ where
                             module.as_deref(),
                             &alias.node.name,
                         );
-                        let range = Range::from_located(alias);
                         self.add_binding(
                             name,
                             Binding {
                                 kind: BindingKind::FromImportation(name, full_name),
                                 runtime_usage: None,
-                                // Treat explicit re-export as usage (e.g., `from .applications
+                                synthetic_usage: if is_handled || is_explicit_reexport {
-                                // import FastAPI as FastAPI`).
-                                synthetic_usage: if alias
-                                    .node
-                                    .asname
-                                    .as_ref()
-                                    .map_or(false, |asname| asname == &alias.node.name)
-                                {
                                     Some((
                                         self.scopes[*(self
                                             .scope_stack
                                             .last()
                                             .expect("No current scope found"))]
                                         .id,
-                                        range,
+                                        Range::from_located(alias),
                                     ))
                                 } else {
                                     None
                                 },
                                 typing_usage: None,
-                                range,
+                                range: Range::from_located(alias),
                                 source: Some(self.current_stmt().clone()),
                                 context: self.execution_context(),
                             },
@@ -2128,17 +2166,23 @@ where
                 orelse,
                 finalbody,
             } => {
-                // TODO(charlie): The use of `smallvec` here leads to a lifetime issue.
+                let mut handled_exceptions = Exceptions::empty();
-                let handler_names = extract_handler_names(handlers)
+                for type_ in extract_handled_exceptions(handlers) {
-                    .into_iter()
+                    if let Some(call_path) = self.resolve_call_path(type_) {
-                    .map(|call_path| call_path.to_vec())
+                        if call_path.as_slice() == ["", "NameError"] {
-                    .collect();
+                            handled_exceptions |= Exceptions::NAME_ERROR;
-                self.except_handlers.push(handler_names);
+                        } else if call_path.as_slice() == ["", "ModuleNotFoundError"] {
+                            handled_exceptions |= Exceptions::MODULE_NOT_FOUND_ERROR;
+                        }
+                    }
+                }
+
+                self.handled_exceptions.push(handled_exceptions);
                 if self.settings.rules.enabled(&Rule::JumpStatementInFinally) {
                     flake8_bugbear::rules::jump_statement_in_finally(self, finalbody);
                 }
                 self.visit_body(body);
-                self.except_handlers.pop();
+                self.handled_exceptions.pop();
 
                 self.in_exception_handler = true;
                 for excepthandler in handlers {
@@ -4493,14 +4537,13 @@ impl<'a> Checker<'a> {
             }
 
             // Avoid flagging if NameError is handled.
-            if let Some(handler_names) = self.except_handlers.last() {
+            if self
-                if handler_names
+                .handled_exceptions
                 .iter()
-                    .any(|call_path| call_path.as_slice() == ["NameError"])
+                .any(|handler_names| handler_names.contains(Exceptions::NAME_ERROR))
             {
                 return;
             }
-            }
 
             self.diagnostics.push(Diagnostic::new(
                 pyflakes::rules::UndefinedName { name: id.clone() },

crates/ruff/src/rules/flake8_simplify/rules/use_contextlib_suppress.rs

@@ -1,7 +1,9 @@
-use ruff_macros::{define_violation, derive_message_formats};
 use rustpython_parser::ast::{Excepthandler, ExcepthandlerKind, Located, Stmt, StmtKind};
 
+use ruff_macros::{define_violation, derive_message_formats};
+
 use crate::ast::helpers;
+use crate::ast::helpers::compose_call_path;
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::registry::Diagnostic;
@@ -52,9 +54,9 @@ pub fn use_contextlib_suppress(
     let ExcepthandlerKind::ExceptHandler { body, .. } = &handler.node;
     if body.len() == 1 {
         if matches!(body[0].node, StmtKind::Pass) {
-            let handler_names: Vec<_> = helpers::extract_handler_names(handlers)
+            let handler_names: Vec<String> = helpers::extract_handled_exceptions(handlers)
                 .into_iter()
-                .map(|call_path| helpers::format_call_path(&call_path))
+                .filter_map(compose_call_path)
                 .collect();
             let exception = if handler_names.is_empty() {
                 "Exception".to_string()

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

@@ -32,6 +32,7 @@ mod tests {
     #[test_case(Rule::UnusedImport, Path::new("F401_7.py"); "F401_7")]
     #[test_case(Rule::UnusedImport, Path::new("F401_8.py"); "F401_8")]
     #[test_case(Rule::UnusedImport, Path::new("F401_9.py"); "F401_9")]
+    #[test_case(Rule::UnusedImport, Path::new("F401_10.py"); "F401_10")]
     #[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.py"); "F402")]
     #[test_case(Rule::ImportStar, Path::new("F403.py"); "F403")]
     #[test_case(Rule::LateFutureImport, Path::new("F404.py"); "F404")]

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

@@ -0,0 +1,6 @@
+---
+source: crates/ruff/src/rules/pyflakes/mod.rs
+expression: diagnostics
+---
+[]
+

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

@@ -1,5 +1,5 @@
 ---
-source: src/rules/pyflakes/mod.rs
+source: crates/ruff/src/rules/pyflakes/mod.rs
 expression: diagnostics
 ---
 - kind: