-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[pylint] Implement misplaced-bare-raise (E0704) #7961
[pylint] Implement misplaced-bare-raise (E0704) #7961
Conversation
a9a4e0c
to
4b1ece8
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
Thanks! Do you mind taking a look at some of the violations flagged in the ecosystem check, and seeing how they compare to pylint's behavior? |
They are the same with pylint. I had to run pylint on each file though, I'm not sure what I'm doing wrong. For example, with pip:
|
#[derive(Default)] | ||
struct RaiseFinder<'a> { | ||
raises: Vec<&'a Stmt>, | ||
} | ||
|
||
impl<'a, 'b> StatementVisitor<'b> for RaiseFinder<'a> | ||
where | ||
'b: 'a, | ||
{ | ||
fn visit_stmt(&mut self, stmt: &'b Stmt) { | ||
match stmt { | ||
Stmt::Raise(_) => self.raises.push(stmt), | ||
_ => statement_visitor::walk_stmt(self, stmt), | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to use ruff_python_ast::helpers::RaiseStatementVisitor
, but it doesn't store the raise statement itself.
It's only used in two rules, so maybe I can refactor that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah -- can we change RaiseStatementVisitor
to store Vec<&'a ast::StmtRaise>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall I do that in a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be cool with including it here, but either is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed anymore, so I guess I won't touch RaiseStatementVisitor
.
4b1ece8
to
7adbd0e
Compare
crates/ruff_linter/src/rules/pylint/rules/misplaced_bare_raise.rs
Outdated
Show resolved
Hide resolved
7adbd0e
to
d887d2a
Compare
d887d2a
to
6f33ee4
Compare
Related issue: #970
Summary
What it does
This rule triggers an error when a bare raise statement is not in an except or finally block.
Why is this bad?
If raise statement is not in an except or finally block, there is no active exception to
re-raise, so it will fail with a
RuntimeError
exception.Example
Use instead:
Test Plan
Added unit test and snapshot.
Manually compared ruff and pylint outputs on pylint's tests.
References