From bd935cbd49f3de9b5d4949391bea77478d2cdd2e Mon Sep 17 00:00:00 2001 From: Johan Date: Fri, 17 Mar 2023 20:20:08 +0100 Subject: [PATCH] [`flake8-bugbear`] Add `no-explicit-stacklevel` (`B028`) (#3550) --- .../test/fixtures/flake8_bugbear/B028.py | 11 +++ crates/ruff/src/checkers/ast/mod.rs | 3 + crates/ruff/src/codes.rs | 1 + crates/ruff/src/registry.rs | 1 + crates/ruff/src/rules/flake8_bugbear/mod.rs | 1 + .../src/rules/flake8_bugbear/rules/mod.rs | 2 + .../rules/no_explicit_stacklevel.rs | 68 +++++++++++++++++++ ...__flake8_bugbear__tests__B028_B028.py.snap | 31 +++++++++ ruff.schema.json | 1 + 9 files changed, 119 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flake8_bugbear/B028.py create mode 100644 crates/ruff/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs create mode 100644 crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B028_B028.py.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B028.py b/crates/ruff/resources/test/fixtures/flake8_bugbear/B028.py new file mode 100644 index 0000000000000..a2915f29adf08 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B028.py @@ -0,0 +1,11 @@ +import warnings + +""" +Should emit: +B028 - on lines 8 and 9 +""" + +warnings.warn(DeprecationWarning("test")) +warnings.warn(DeprecationWarning("test"), source=None) +warnings.warn(DeprecationWarning("test"), source=None, stacklevel=2) +warnings.warn(DeprecationWarning("test"), stacklevel=1) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 5c4bff9c642fb..0b24ebc6eaa96 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2456,6 +2456,9 @@ where { flake8_bugbear::rules::zip_without_explicit_strict(self, expr, func, keywords); } + if self.settings.rules.enabled(Rule::NoExplicitStacklevel) { + flake8_bugbear::rules::no_explicit_stacklevel(self, func, args, keywords); + } // flake8-pie if self.settings.rules.enabled(Rule::UnnecessaryDictKwargs) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index f4d2f5592ba45..952576fc55058 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -227,6 +227,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Bugbear, "025") => Rule::DuplicateTryBlockException, (Flake8Bugbear, "026") => Rule::StarArgUnpackingAfterKeywordArg, (Flake8Bugbear, "027") => Rule::EmptyMethodWithoutAbstractDecorator, + (Flake8Bugbear, "028") => Rule::NoExplicitStacklevel, (Flake8Bugbear, "029") => Rule::ExceptWithEmptyTuple, (Flake8Bugbear, "030") => Rule::ExceptWithNonExceptionClasses, (Flake8Bugbear, "032") => Rule::UnintentionalTypeAnnotation, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 04e5bfc147c3c..e833fe8b9852f 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -186,6 +186,7 @@ ruff_macros::register_rules!( rules::flake8_bugbear::rules::UnreliableCallableCheck, rules::flake8_bugbear::rules::StripWithMultiCharacters, rules::flake8_bugbear::rules::MutableArgumentDefault, + rules::flake8_bugbear::rules::NoExplicitStacklevel, rules::flake8_bugbear::rules::UnusedLoopControlVariable, rules::flake8_bugbear::rules::FunctionCallArgumentDefault, rules::flake8_bugbear::rules::GetAttrWithConstant, diff --git a/crates/ruff/src/rules/flake8_bugbear/mod.rs b/crates/ruff/src/rules/flake8_bugbear/mod.rs index a20926156cdfb..4f3a166ee5e77 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::NoExplicitStacklevel, Path::new("B028.py"); "B028")] #[test_case(Rule::ExceptWithEmptyTuple, Path::new("B029.py"); "B029")] #[test_case(Rule::ExceptWithNonExceptionClasses, Path::new("B030.py"); "B030")] #[test_case(Rule::UnintentionalTypeAnnotation, Path::new("B032.py"); "B032")] diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs b/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs index 1c1f2ada284a6..7802109c4eebe 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/mod.rs @@ -25,6 +25,7 @@ pub use loop_variable_overrides_iterator::{ loop_variable_overrides_iterator, LoopVariableOverridesIterator, }; pub use mutable_argument_default::{mutable_argument_default, MutableArgumentDefault}; +pub use no_explicit_stacklevel::{no_explicit_stacklevel, NoExplicitStacklevel}; pub use raise_without_from_inside_except::{ raise_without_from_inside_except, RaiseWithoutFromInsideExcept, }; @@ -63,6 +64,7 @@ mod getattr_with_constant; mod jump_statement_in_finally; mod loop_variable_overrides_iterator; mod mutable_argument_default; +mod no_explicit_stacklevel; mod raise_without_from_inside_except; mod redundant_tuple_in_exception_handler; mod setattr_with_constant; diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs b/crates/ruff/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs new file mode 100644 index 0000000000000..b93961fed8a92 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bugbear/rules/no_explicit_stacklevel.rs @@ -0,0 +1,68 @@ +use rustpython_parser::ast::{Expr, Keyword}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::SimpleCallArgs; +use ruff_python_ast::types::Range; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `warnings.warn` calls without an explicit `stacklevel` keyword +/// argument. +/// +/// ## Why is this bad? +/// The `warnings.warn` method uses a `stacklevel` of 1 by default, which +/// limits the rendered stack trace to that of the line on which the +/// `warn` method is called. +/// +/// It's recommended to use a `stacklevel` of 2 or higher, give the caller +/// more context about the warning. +/// +/// ## Example +/// ```python +/// warnings.warn("This is a warning") +/// ``` +/// +/// Use instead: +/// ```python +/// warnings.warn("This is a warning", stacklevel=2) +/// ``` +#[violation] +pub struct NoExplicitStacklevel; + +impl Violation for NoExplicitStacklevel { + #[derive_message_formats] + fn message(&self) -> String { + format!("No explicit `stacklevel` keyword argument found") + } +} + +/// B028 +pub fn no_explicit_stacklevel( + checker: &mut Checker, + func: &Expr, + args: &[Expr], + keywords: &[Keyword], +) { + if !checker + .ctx + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["warnings", "warn"] + }) + { + return; + } + + if SimpleCallArgs::new(args, keywords) + .keyword_argument("stacklevel") + .is_some() + { + return; + } + + checker + .diagnostics + .push(Diagnostic::new(NoExplicitStacklevel, Range::from(func))); +} diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B028_B028.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B028_B028.py.snap new file mode 100644 index 0000000000000..e2c47aa6d1d00 --- /dev/null +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B028_B028.py.snap @@ -0,0 +1,31 @@ +--- +source: crates/ruff/src/rules/flake8_bugbear/mod.rs +expression: diagnostics +--- +- kind: + name: NoExplicitStacklevel + body: "No explicit `stacklevel` keyword argument found" + suggestion: ~ + fixable: false + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 13 + fix: ~ + parent: ~ +- kind: + name: NoExplicitStacklevel + body: "No explicit `stacklevel` keyword argument found" + suggestion: ~ + fixable: false + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 13 + fix: ~ + parent: ~ + diff --git a/ruff.schema.json b/ruff.schema.json index 2d5f187c93d00..c25cf1582777c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1469,6 +1469,7 @@ "B025", "B026", "B027", + "B028", "B029", "B03", "B030",