-
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
feat(B029): Add B029 from flake8-bugbear #3068
Changes from 10 commits
24eec97
3a2f049
101ee9e
4f249f0
ff2cce7
c0bbb55
7a6173a
7dae7f2
e8f6c41
47fbbc6
fc57139
330e6e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
""" | ||
Should emit: | ||
B029 - on lines 7 | ||
""" | ||
try: | ||
x = 1 | ||
except(): | ||
print("error") | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
use ruff_macros::{define_violation, derive_message_formats}; | ||
use rustpython_parser::ast::Excepthandler; | ||
|
||
use crate::ast::types::Range; | ||
use crate::checkers::ast::Checker; | ||
use crate::registry::Diagnostic; | ||
use crate::violation::Violation; | ||
|
||
use rustpython_parser::ast::{ExcepthandlerKind, ExprKind}; | ||
|
||
define_violation!( | ||
pub struct ExceptWithEmptyTuple; | ||
); | ||
impl Violation for ExceptWithEmptyTuple { | ||
#[derive_message_formats] | ||
fn message(&self) -> String { | ||
format!("Using except (): with an empty tuple does not handle/catch anything. Add exceptions to handle.") | ||
} | ||
} | ||
|
||
/// B029 | ||
pub fn except_with_empty_tuple(checker: &mut Checker, excepthandler: &Excepthandler) { | ||
let ExcepthandlerKind::ExceptHandler { type_, .. } = &excepthandler.node; | ||
if type_.is_none() { | ||
return; | ||
} | ||
let ExprKind::Tuple { elts, .. } = &type_.as_ref().unwrap().node else { | ||
return; | ||
}; | ||
if elts.is_empty() { | ||
checker.diagnostics.push(Diagnostic::new( | ||
ExceptWithEmptyTuple, | ||
Range::from_located(excepthandler), | ||
)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
--- | ||
source: crates/ruff/src/rules/flake8_bugbear/mod.rs | ||
expression: diagnostics | ||
--- | ||
- kind: | ||
ExceptWithEmptyTuple: ~ | ||
location: | ||
row: 7 | ||
column: 0 | ||
end_location: | ||
row: 8 | ||
column: 18 | ||
fix: ~ | ||
parent: ~ | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @charliermarsh yeah it's weird I did run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, you need to add the test to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @charliermarsh nvm 🤦 I accidentally deleted it upon resolving the merge conflict here will restore it asap There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happens to the best of us :D |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Can we replace this with the version from flake8-bugbear itself?
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.
@charliermarsh sure!