From 24eec97b3e63cc1145755fd3d84e6c639d1058d0 Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 17:56:08 +0000 Subject: [PATCH 01/11] feat(B029): init add ExceptWithEmptyTuple --- crates/ruff/src/registry.rs | 1 + .../rules/except_with_empty_tuple.rs | 53 +++++++++++++++++++ .../src/rules/flake8_bugbear/rules/mod.rs | 2 + 3 files changed, 56 insertions(+) create mode 100644 crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 659b05a454e48..e1a352eed8e43 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -181,6 +181,7 @@ ruff_macros::register_rules!( rules::flake8_bugbear::rules::EmptyMethodWithoutAbstractDecorator, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict, + rules::flake8_bugbear::rules::ExceptWithEmptyTuple, // flake8-blind-except rules::flake8_blind_except::rules::BlindExcept, // flake8-comprehensions diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs new file mode 100644 index 0000000000000..957f3b483dbcf --- /dev/null +++ b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs @@ -0,0 +1,53 @@ +//! Checks for `++n`. +//! +//! ## Why is this bad? +//! +//! Python does not support the unary prefix increment. Writing `++n` is +//! equivalent to `+(+(n))`, which equals `n`. +//! +//! ## Example +//! +//! ```python +//! ++n; +//! ``` +//! +//! Use instead: +//! +//! ```python +//! n += 1 +//! ``` + +use ruff_macros::{define_violation, derive_message_formats}; +use rustpython_parser::ast::{Expr, ExprKind, Unaryop}; + +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::registry::Diagnostic; +use crate::violation::Violation; + +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, expr: &Expr, op: &Unaryop, operand: &Expr) { + if !matches!(op, Unaryop::UAdd) { + return; + } + let ExprKind::UnaryOp { op, .. } = &operand.node else { + return; + }; + if !matches!(op, Unaryop::UAdd) { + return; + } + checker.diagnostics.push(Diagnostic::new( + ExceptWithEmptyTuple, + Range::from_located(expr), + )); +} diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs b/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs index a9e936115c7bd..73283b204382b 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs @@ -10,6 +10,7 @@ pub use cannot_raise_literal::{cannot_raise_literal, CannotRaiseLiteral}; pub use duplicate_exceptions::{ duplicate_exceptions, DuplicateHandlerException, DuplicateTryBlockException, }; +pub use except_with_empty_tuple::{except_with_empty_tuple, ExceptWithEmptyTuple}; pub use f_string_docstring::{f_string_docstring, FStringDocstring}; pub use function_call_argument_default::{ function_call_argument_default, FunctionCallArgumentDefault, @@ -47,6 +48,7 @@ mod assignment_to_os_environ; mod cached_instance_method; mod cannot_raise_literal; mod duplicate_exceptions; +mod except_with_empty_tuple; mod f_string_docstring; mod function_call_argument_default; mod function_uses_loop_variable; From 3a2f049c35707306173722297d35715c2930acd4 Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 18:02:09 +0000 Subject: [PATCH 02/11] add codes --- crates/ruff/src/codes.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 5074aafbd1c43..5ec402cafe8de 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -175,6 +175,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Bugbear, "022") => Rule::UselessContextlibSuppress, (Flake8Bugbear, "023") => Rule::FunctionUsesLoopVariable, (Flake8Bugbear, "024") => Rule::AbstractBaseClassWithoutAbstractMethod, + (Flake8Bugbear, "028") => Rule::ExceptWithEmptyTuple, (Flake8Bugbear, "025") => Rule::DuplicateTryBlockException, (Flake8Bugbear, "026") => Rule::StarArgUnpackingAfterKeywordArg, (Flake8Bugbear, "027") => Rule::EmptyMethodWithoutAbstractDecorator, From 101ee9e6dedd024c29ff277833002b200865f40b Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 18:02:17 +0000 Subject: [PATCH 03/11] basic implementation --- crates/ruff/src/checkers/ast.rs | 3 +++ .../rules/except_with_empty_tuple.rs | 15 +++------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/crates/ruff/src/checkers/ast.rs b/crates/ruff/src/checkers/ast.rs index ac732166e80c7..8aaabed3509f7 100644 --- a/crates/ruff/src/checkers/ast.rs +++ b/crates/ruff/src/checkers/ast.rs @@ -3688,6 +3688,9 @@ where self.settings.flake8_bandit.check_typed_exception, ); } + if self.settings.rules.enabled(&Rule::ExceptWithEmptyTuple) { + flake8_bugbear::rules::except_with_empty_tuple(self, excepthandler); + } if self.settings.rules.enabled(&Rule::ReraiseNoCause) { tryceratops::rules::reraise_no_cause(self, body); } diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs index 957f3b483dbcf..4e90c76258158 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs @@ -18,7 +18,7 @@ //! ``` use ruff_macros::{define_violation, derive_message_formats}; -use rustpython_parser::ast::{Expr, ExprKind, Unaryop}; +use rustpython_parser::ast::Excepthandler; use crate::ast::types::Range; use crate::checkers::ast::Checker; @@ -36,18 +36,9 @@ impl Violation for ExceptWithEmptyTuple { } /// B029 -pub fn except_with_empty_tuple(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) { - if !matches!(op, Unaryop::UAdd) { - return; - } - let ExprKind::UnaryOp { op, .. } = &operand.node else { - return; - }; - if !matches!(op, Unaryop::UAdd) { - return; - } +pub fn except_with_empty_tuple(checker: &mut Checker, excepthandler: &Excepthandler) { checker.diagnostics.push(Diagnostic::new( ExceptWithEmptyTuple, - Range::from_located(expr), + Range::from_located(excepthandler), )); } From 4f249f0e85562f8bf6159a48c5e6598fc1ad7ed9 Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 18:28:31 +0000 Subject: [PATCH 04/11] change order --- crates/ruff/src/codes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index 5ec402cafe8de..cba0246e46e67 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -175,10 +175,10 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Bugbear, "022") => Rule::UselessContextlibSuppress, (Flake8Bugbear, "023") => Rule::FunctionUsesLoopVariable, (Flake8Bugbear, "024") => Rule::AbstractBaseClassWithoutAbstractMethod, - (Flake8Bugbear, "028") => Rule::ExceptWithEmptyTuple, (Flake8Bugbear, "025") => Rule::DuplicateTryBlockException, (Flake8Bugbear, "026") => Rule::StarArgUnpackingAfterKeywordArg, (Flake8Bugbear, "027") => Rule::EmptyMethodWithoutAbstractDecorator, + (Flake8Bugbear, "029") => Rule::ExceptWithEmptyTuple, (Flake8Bugbear, "904") => Rule::RaiseWithoutFromInsideExcept, (Flake8Bugbear, "905") => Rule::ZipWithoutExplicitStrict, From ff2cce7b1ce798e30a70179a16fad144dddc87e0 Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 19:48:22 +0000 Subject: [PATCH 05/11] rule logic --- .../rules/except_with_empty_tuple.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs index 4e90c76258158..4279dcaeddf9f 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs @@ -25,6 +25,8 @@ use crate::checkers::ast::Checker; use crate::registry::Diagnostic; use crate::violation::Violation; +use rustpython_parser::ast::{ExcepthandlerKind, ExprKind}; + define_violation!( pub struct ExceptWithEmptyTuple; ); @@ -37,8 +39,17 @@ impl Violation for ExceptWithEmptyTuple { /// B029 pub fn except_with_empty_tuple(checker: &mut Checker, excepthandler: &Excepthandler) { - checker.diagnostics.push(Diagnostic::new( - ExceptWithEmptyTuple, - Range::from_located(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), + )); + } } From c0bbb55482b87a556a782f2b8b29f5e9d75307ce Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 19:54:20 +0000 Subject: [PATCH 06/11] add test --- .../ruff/resources/test/fixtures/flake8_bugbear/B029.py | 8 ++++++++ crates/ruff/src/rules/flake8_bugbear/mod.rs | 1 + 2 files changed, 9 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py b/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py new file mode 100644 index 0000000000000..80558501c321c --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py @@ -0,0 +1,8 @@ +""" +Should emit: +B029 - on lines 7 +""" +try: + x = 1 +except(): + print("error") diff --git a/crates/ruff/src/rules/flake8_bugbear/mod.rs b/crates/ruff/src/rules/flake8_bugbear/mod.rs index 6b03217507c11..7270b4438d352 100644 --- a/crates/ruff/src/rules/flake8_bugbear/mod.rs +++ b/crates/ruff/src/rules/flake8_bugbear/mod.rs @@ -40,6 +40,7 @@ mod tests { #[test_case(Rule::DuplicateTryBlockException, Path::new("B025.py"); "B025")] #[test_case(Rule::StarArgUnpackingAfterKeywordArg, Path::new("B026.py"); "B026")] #[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.py"); "B027")] + #[test_case(Rule::ExceptWithEmptyTuple, Path::new("B029.py"); "B029")] #[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"); "B904")] #[test_case(Rule::ZipWithoutExplicitStrict, Path::new("B905.py"); "B905")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { From 7a6173ae287096311914cd94ba0a03ee6798804b Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 19:54:30 +0000 Subject: [PATCH 07/11] remove comment --- .../rules/except_with_empty_tuple.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs index 4279dcaeddf9f..c9e4e7f682d61 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/except_with_empty_tuple.rs @@ -1,22 +1,3 @@ -//! Checks for `++n`. -//! -//! ## Why is this bad? -//! -//! Python does not support the unary prefix increment. Writing `++n` is -//! equivalent to `+(+(n))`, which equals `n`. -//! -//! ## Example -//! -//! ```python -//! ++n; -//! ``` -//! -//! Use instead: -//! -//! ```python -//! n += 1 -//! ``` - use ruff_macros::{define_violation, derive_message_formats}; use rustpython_parser::ast::Excepthandler; From 7dae7f244b42d44d213063ce4a416608d7ef1d8d Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 19:54:37 +0000 Subject: [PATCH 08/11] add scheme --- ruff.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/ruff.schema.json b/ruff.schema.json index 2a203158d572c..9f969806c9c52 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1405,6 +1405,7 @@ "B025", "B026", "B027", + "BO29", "B9", "B90", "B904", From e8f6c419cfa6a3dae0dc0543f592346492bb230c Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 20:04:21 +0000 Subject: [PATCH 09/11] update generated files --- ...ules__flake8_bugbear__tests__B029_B029.py.snap | 15 +++++++++++++++ ruff.schema.json | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap new file mode 100644 index 0000000000000..1c7c4243bd6ab --- /dev/null +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap @@ -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: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index 9f969806c9c52..5324db8333c98 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1405,7 +1405,7 @@ "B025", "B026", "B027", - "BO29", + "B029", "B9", "B90", "B904", From fc571390256d4d38d5619544c2f8f5377fd74832 Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 20:26:38 +0000 Subject: [PATCH 10/11] update test --- .../resources/test/fixtures/flake8_bugbear/B029.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py b/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py index 80558501c321c..beb04a4059392 100644 --- a/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B029.py @@ -1,8 +1,14 @@ """ Should emit: -B029 - on lines 7 +B029 - on lines 8 and 13 """ + try: - x = 1 -except(): - print("error") + pass +except (): + pass + +try: + pass +except () as e: + pass \ No newline at end of file From 330e6e1c7114444cc5c783397a43dacd1e89479c Mon Sep 17 00:00:00 2001 From: carlosmiei <43336371+carlosmiei@users.noreply.github.com> Date: Mon, 20 Feb 2023 20:49:58 +0000 Subject: [PATCH 11/11] restore --- crates/ruff/src/rules/flake8_bugbear/mod.rs | 1 + ...les__flake8_bugbear__tests__B029_B029.py.snap | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/ruff/src/rules/flake8_bugbear/mod.rs b/crates/ruff/src/rules/flake8_bugbear/mod.rs index 0765b834c10cd..c91d9757dc6f8 100644 --- a/crates/ruff/src/rules/flake8_bugbear/mod.rs +++ b/crates/ruff/src/rules/flake8_bugbear/mod.rs @@ -41,6 +41,7 @@ mod tests { #[test_case(Rule::StarArgUnpackingAfterKeywordArg, Path::new("B026.py"); "B026")] #[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.py"); "B027")] #[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.pyi"); "B027_pyi")] + #[test_case(Rule::ExceptWithEmptyTuple, Path::new("B029.py"); "B029")] #[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"); "B904")] #[test_case(Rule::ZipWithoutExplicitStrict, Path::new("B905.py"); "B905")] fn rules(rule_code: Rule, path: &Path) -> Result<()> { diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap index 1c7c4243bd6ab..d7949e900e228 100644 --- a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B029_B029.py.snap @@ -5,11 +5,21 @@ expression: diagnostics - kind: ExceptWithEmptyTuple: ~ location: - row: 7 + row: 8 column: 0 end_location: - row: 8 - column: 18 + row: 9 + column: 8 + fix: ~ + parent: ~ +- kind: + ExceptWithEmptyTuple: ~ + location: + row: 13 + column: 0 + end_location: + row: 14 + column: 8 fix: ~ parent: ~