Allow non-verbose raise when cause is present (#2816)

The motivating issue here is of the following form:

```py
try:
    raise Exception("We want to hide this error message")
except Exception:
    try:
        raise Exception("We want to show this")
    except Exception as exc:
        raise exc from None
```

However, I think we should avoid this if _any_ cause is present, since causes require a named exception.

Closes #2814.
This commit is contained in:
Charlie Marsh 2023-02-12 11:48:13 -05:00 committed by GitHub
parent 9ddd5e4cfe
commit 099d5414f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 29 additions and 14 deletions

View file

@ -27,6 +27,7 @@ def good():
logger.exception("process failed") logger.exception("process failed")
raise raise
def still_good(): def still_good():
try: try:
process() process()
@ -35,6 +36,14 @@ def still_good():
raise raise
def still_good_too():
try:
process()
except MyException as e:
print(e)
raise e from None
def still_actually_good(): def still_actually_good():
try: try:
process() process()
@ -60,5 +69,6 @@ def bad_that_needs_recursion_2():
except MyException as e: except MyException as e:
logger.exception("process failed") logger.exception("process failed")
if True: if True:
def foo(): def foo():
raise e raise e

View file

@ -20,7 +20,7 @@ impl Violation for VerboseRaise {
#[derive(Default)] #[derive(Default)]
struct RaiseStatementVisitor<'a> { struct RaiseStatementVisitor<'a> {
raises: Vec<Option<&'a Expr>>, raises: Vec<(Option<&'a Expr>, Option<&'a Expr>)>,
} }
impl<'a, 'b> Visitor<'b> for RaiseStatementVisitor<'a> impl<'a, 'b> Visitor<'b> for RaiseStatementVisitor<'a>
@ -29,7 +29,7 @@ where
{ {
fn visit_stmt(&mut self, stmt: &'b Stmt) { fn visit_stmt(&mut self, stmt: &'b Stmt) {
match &stmt.node { match &stmt.node {
StmtKind::Raise { exc, .. } => self.raises.push(exc.as_ref().map(|expr| &**expr)), StmtKind::Raise { exc, cause } => self.raises.push((exc.as_deref(), cause.as_deref())),
StmtKind::Try { StmtKind::Try {
body, finalbody, .. body, finalbody, ..
} => { } => {
@ -59,13 +59,18 @@ pub fn verbose_raise(checker: &mut Checker, handlers: &[Excepthandler]) {
} }
visitor.raises visitor.raises
}; };
for expr in raises.into_iter().flatten() { for (exc, cause) in raises {
if cause.is_some() {
continue;
}
if let Some(exc) = exc {
// ...and the raised object is bound to the same name... // ...and the raised object is bound to the same name...
if let ExprKind::Name { id, .. } = &expr.node { if let ExprKind::Name { id, .. } = &exc.node {
if id == exception_name { if id == exception_name {
checker checker
.diagnostics .diagnostics
.push(Diagnostic::new(VerboseRaise, Range::from_located(expr))); .push(Diagnostic::new(VerboseRaise, Range::from_located(exc)));
}
} }
} }
} }

View file

@ -1,5 +1,5 @@
--- ---
source: src/rules/tryceratops/mod.rs source: crates/ruff/src/rules/tryceratops/mod.rs
expression: diagnostics expression: diagnostics
--- ---
- kind: - kind:
@ -15,20 +15,20 @@ expression: diagnostics
- kind: - kind:
VerboseRaise: ~ VerboseRaise: ~
location: location:
row: 54 row: 63
column: 18 column: 18
end_location: end_location:
row: 54 row: 63
column: 19 column: 19
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
VerboseRaise: ~ VerboseRaise: ~
location: location:
row: 64 row: 74
column: 22 column: 22
end_location: end_location:
row: 64 row: 74
column: 23 column: 23
fix: ~ fix: ~
parent: ~ parent: ~