Fix B025 location (#1360)

astral-sh · Dec 24, 2022 · 63772e3 · 63772e3
1 parent 7f4ff1e
commit 63772e3
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 37 deletions.
diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs
@@ -1151,7 +1151,7 @@ where
                 if self.settings.enabled.contains(&CheckCode::B014)
                     || self.settings.enabled.contains(&CheckCode::B025)
                 {
-                    flake8_bugbear::plugins::duplicate_exceptions(self, stmt, handlers);
+                    flake8_bugbear::plugins::duplicate_exceptions(self, handlers);
                 }
                 if self.settings.enabled.contains(&CheckCode::B013) {
                     flake8_bugbear::plugins::redundant_tuple_in_exception_handler(self, handlers);

diff --git a/src/flake8_bugbear/plugins/duplicate_exceptions.rs b/src/flake8_bugbear/plugins/duplicate_exceptions.rs
@@ -1,8 +1,6 @@
 use itertools::Itertools;
-use rustc_hash::FxHashSet;
-use rustpython_ast::{
-    Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Location, Stmt,
-};
+use rustc_hash::{FxHashMap, FxHashSet};
+use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Location};
 
 use crate::ast::helpers;
 use crate::ast::types::Range;
@@ -26,17 +24,17 @@ fn duplicate_handler_exceptions<'a>(
     checker: &mut Checker,
     expr: &'a Expr,
     elts: &'a [Expr],
-) -> FxHashSet<Vec<&'a str>> {
-    let mut seen: FxHashSet<Vec<&str>> = FxHashSet::default();
+) -> FxHashMap<Vec<&'a str>, &'a Expr> {
+    let mut seen: FxHashMap<Vec<&str>, &Expr> = FxHashMap::default();
     let mut duplicates: FxHashSet<Vec<&str>> = FxHashSet::default();
     let mut unique_elts: Vec<&Expr> = Vec::default();
     for type_ in elts {
         let call_path = helpers::collect_call_paths(type_);
         if !call_path.is_empty() {
-            if seen.contains(&call_path) {
+            if seen.contains_key(&call_path) {
                 duplicates.insert(call_path);
             } else {
-                seen.insert(call_path);
+                seen.entry(call_path).or_insert(type_);
                 unique_elts.push(type_);
             }
         }
@@ -77,9 +75,9 @@ fn duplicate_handler_exceptions<'a>(
     seen
 }
 
-pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Excepthandler]) {
+pub fn duplicate_exceptions(checker: &mut Checker, handlers: &[Excepthandler]) {
     let mut seen: FxHashSet<Vec<&str>> = FxHashSet::default();
-    let mut duplicates: FxHashSet<Vec<&str>> = FxHashSet::default();
+    let mut duplicates: FxHashMap<Vec<&str>, Vec<&Expr>> = FxHashMap::default();
     for handler in handlers {
         let ExcepthandlerKind::ExceptHandler { type_: Some(type_), .. } = &handler.node else {
             continue;
@@ -89,16 +87,16 @@ pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Exce
                 let call_path = helpers::collect_call_paths(type_);
                 if !call_path.is_empty() {
                     if seen.contains(&call_path) {
-                        duplicates.insert(call_path);
+                        duplicates.entry(call_path).or_default().push(type_);
                     } else {
                         seen.insert(call_path);
                     }
                 }
             }
             ExprKind::Tuple { elts, .. } => {
-                for name in duplicate_handler_exceptions(checker, type_, elts) {
+                for (name, expr) in duplicate_handler_exceptions(checker, type_, elts) {
                     if seen.contains(&name) {
-                        duplicates.insert(name);
+                        duplicates.entry(name).or_default().push(expr);
                     } else {
                         seen.insert(name);
                     }
@@ -109,11 +107,13 @@ pub fn duplicate_exceptions(checker: &mut Checker, stmt: &Stmt, handlers: &[Exce
     }
 
     if checker.settings.enabled.contains(&CheckCode::B025) {
-        for duplicate in duplicates.into_iter().sorted() {
-            checker.add_check(Check::new(
-                CheckKind::DuplicateTryBlockException(duplicate.join(".")),
-                Range::from_located(stmt),
-            ));
+        for (name, exprs) in duplicates {
+            for expr in exprs {
+                checker.add_check(Check::new(
+                    CheckKind::DuplicateTryBlockException(name.join(".")),
+                    Range::from_located(expr),
+                ));
+            }
         }
     }
 }
diff --git a/src/flake8_bugbear/snapshots/ruff__flake8_bugbear__tests__B025_B025.py.snap b/src/flake8_bugbear/snapshots/ruff__flake8_bugbear__tests__B025_B025.py.snap
@@ -5,37 +5,37 @@ expression: checks
 - kind:
     DuplicateTryBlockException: ValueError
   location:
-    row: 15
-    column: 0
+    row: 19
+    column: 7
   end_location:
-    row: 20
-    column: 9
+    row: 19
+    column: 17
   fix: ~
 - kind:
     DuplicateTryBlockException: pickle.PickleError
   location:
-    row: 22
-    column: 0
+    row: 28
+    column: 7
   end_location:
-    row: 29
-    column: 9
+    row: 28
+    column: 25
   fix: ~
 - kind:
-    DuplicateTryBlockException: TypeError
+    DuplicateTryBlockException: ValueError
   location:
-    row: 31
-    column: 0
+    row: 35
+    column: 7
   end_location:
-    row: 38
-    column: 9
+    row: 35
+    column: 17
   fix: ~
 - kind:
-    DuplicateTryBlockException: ValueError
+    DuplicateTryBlockException: TypeError
   location:
-    row: 31
-    column: 0
+    row: 37
+    column: 17
   end_location:
-    row: 38
-    column: 9
+    row: 37
+    column: 26
   fix: ~