Skip to content
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

[flake8-bugbear] Add no-explicit-stacklevel (B028) #3550

Merged
merged 3 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_bugbear/B028.py
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_bugbear/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/flake8_bugbear/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I truncated this message a bit and instead moved the additional context into docs.

}
}

/// 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)));
}
Original file line number Diff line number Diff line change
@@ -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: ~

1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.