diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F401_10.py b/crates/ruff/resources/test/fixtures/pyflakes/F401_10.py new file mode 100644 index 0000000000..14e116f673 --- /dev/null +++ b/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 diff --git a/crates/ruff/src/ast/helpers.rs b/crates/ruff/src/ast/helpers.rs index 67b6b3f5df..22001d310a 100644 --- a/crates/ruff/src/ast/helpers.rs +++ b/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 { - // TODO(charlie): Use `resolve_call_path` to avoid false positives for - // overridden builtins. - let mut handler_names = vec![]; +pub fn extract_handled_exceptions(handlers: &[Excepthandler]) -> Vec<&Expr> { + let mut handled_exceptions = Vec::new(); 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_); - if !call_path.is_empty() { - handler_names.push(call_path); - } + handled_exceptions.push(type_); } } else { - let call_path = collect_call_path(type_); - if !call_path.is_empty() { - handler_names.push(call_path); - } + handled_exceptions.push(type_); } } } } } - handler_names + handled_exceptions } /// Return the set of all bound argument names. diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index 426000ce2b..2497ed5109 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/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>>, + handled_exceptions: Vec, // 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 - // as applications`). - synthetic_usage: if alias - .node - .asname - .as_ref() - .map_or(false, |asname| asname == &alias.node.name) - { + synthetic_usage: if is_handled || is_explicit_reexport { 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 - // import FastAPI as FastAPI`). - synthetic_usage: if alias - .node - .asname - .as_ref() - .map_or(false, |asname| asname == &alias.node.name) - { + synthetic_usage: if is_handled || is_explicit_reexport { 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 handler_names = extract_handler_names(handlers) - .into_iter() - .map(|call_path| call_path.to_vec()) - .collect(); - self.except_handlers.push(handler_names); + let mut handled_exceptions = Exceptions::empty(); + for type_ in extract_handled_exceptions(handlers) { + if let Some(call_path) = self.resolve_call_path(type_) { + if call_path.as_slice() == ["", "NameError"] { + handled_exceptions |= Exceptions::NAME_ERROR; + } 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,13 +4537,12 @@ impl<'a> Checker<'a> { } // Avoid flagging if NameError is handled. - if let Some(handler_names) = self.except_handlers.last() { - if handler_names - .iter() - .any(|call_path| call_path.as_slice() == ["NameError"]) - { - return; - } + if self + .handled_exceptions + .iter() + .any(|handler_names| handler_names.contains(Exceptions::NAME_ERROR)) + { + return; } self.diagnostics.push(Diagnostic::new( diff --git a/crates/ruff/src/rules/flake8_simplify/rules/use_contextlib_suppress.rs b/crates/ruff/src/rules/flake8_simplify/rules/use_contextlib_suppress.rs index 1eb00ac1ff..ecdf70a12f 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/use_contextlib_suppress.rs +++ b/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 = 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() diff --git a/crates/ruff/src/rules/pyflakes/mod.rs b/crates/ruff/src/rules/pyflakes/mod.rs index a63cdf7f61..146c9505e9 100644 --- a/crates/ruff/src/rules/pyflakes/mod.rs +++ b/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")] diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_10.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_10.py.snap new file mode 100644 index 0000000000..bd5537b631 --- /dev/null +++ b/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 +--- +[] + diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_12.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_12.py.snap index cb7c7df1a7..52982a6b60 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F811_F811_12.py.snap +++ b/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: