diff --git a/src/registry.rs b/src/registry.rs index a3ddcbf338..916c9af4c2 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -99,34 +99,34 @@ ruff_macros::define_rule_mapping!( A002 => violations::BuiltinArgumentShadowing, A003 => violations::BuiltinAttributeShadowing, // flake8-bugbear - B002 => violations::UnaryPrefixIncrement, - B003 => violations::AssignmentToOsEnviron, - B004 => violations::UnreliableCallableCheck, - B005 => violations::StripWithMultiCharacters, - B006 => violations::MutableArgumentDefault, - B007 => violations::UnusedLoopControlVariable, - B008 => violations::FunctionCallArgumentDefault, - B009 => violations::GetAttrWithConstant, - B010 => violations::SetAttrWithConstant, - B011 => violations::DoNotAssertFalse, - B012 => violations::JumpStatementInFinally, - B013 => violations::RedundantTupleInExceptionHandler, - B014 => violations::DuplicateHandlerException, - B015 => violations::UselessComparison, - B016 => violations::CannotRaiseLiteral, - B017 => violations::NoAssertRaisesException, - B018 => violations::UselessExpression, - B019 => violations::CachedInstanceMethod, - B020 => violations::LoopVariableOverridesIterator, - B021 => violations::FStringDocstring, - B022 => violations::UselessContextlibSuppress, - B023 => violations::FunctionUsesLoopVariable, - B024 => violations::AbstractBaseClassWithoutAbstractMethod, - B025 => violations::DuplicateTryBlockException, - B026 => violations::StarArgUnpackingAfterKeywordArg, - B027 => violations::EmptyMethodWithoutAbstractDecorator, - B904 => violations::RaiseWithoutFromInsideExcept, - B905 => violations::ZipWithoutExplicitStrict, + B002 => rules::flake8_bugbear::rules::UnaryPrefixIncrement, + B003 => rules::flake8_bugbear::rules::AssignmentToOsEnviron, + B004 => rules::flake8_bugbear::rules::UnreliableCallableCheck, + B005 => rules::flake8_bugbear::rules::StripWithMultiCharacters, + B006 => rules::flake8_bugbear::rules::MutableArgumentDefault, + B007 => rules::flake8_bugbear::rules::UnusedLoopControlVariable, + B008 => rules::flake8_bugbear::rules::FunctionCallArgumentDefault, + B009 => rules::flake8_bugbear::rules::GetAttrWithConstant, + B010 => rules::flake8_bugbear::rules::SetAttrWithConstant, + B011 => rules::flake8_bugbear::rules::DoNotAssertFalse, + B012 => rules::flake8_bugbear::rules::JumpStatementInFinally, + B013 => rules::flake8_bugbear::rules::RedundantTupleInExceptionHandler, + B014 => rules::flake8_bugbear::rules::DuplicateHandlerException, + B015 => rules::flake8_bugbear::rules::UselessComparison, + B016 => rules::flake8_bugbear::rules::CannotRaiseLiteral, + B017 => rules::flake8_bugbear::rules::NoAssertRaisesException, + B018 => rules::flake8_bugbear::rules::UselessExpression, + B019 => rules::flake8_bugbear::rules::CachedInstanceMethod, + B020 => rules::flake8_bugbear::rules::LoopVariableOverridesIterator, + B021 => rules::flake8_bugbear::rules::FStringDocstring, + B022 => rules::flake8_bugbear::rules::UselessContextlibSuppress, + B023 => rules::flake8_bugbear::rules::FunctionUsesLoopVariable, + B024 => rules::flake8_bugbear::rules::AbstractBaseClassWithoutAbstractMethod, + B025 => rules::flake8_bugbear::rules::DuplicateTryBlockException, + B026 => rules::flake8_bugbear::rules::StarArgUnpackingAfterKeywordArg, + B027 => rules::flake8_bugbear::rules::EmptyMethodWithoutAbstractDecorator, + B904 => rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept, + B905 => rules::flake8_bugbear::rules::ZipWithoutExplicitStrict, // flake8-blind-except BLE001 => violations::BlindExcept, // flake8-comprehensions diff --git a/src/rules/flake8_bugbear/rules/abstract_base_class.rs b/src/rules/flake8_bugbear/rules/abstract_base_class.rs index 0245ecf178..374b91ed3b 100644 --- a/src/rules/flake8_bugbear/rules/abstract_base_class.rs +++ b/src/rules/flake8_bugbear/rules/abstract_base_class.rs @@ -1,10 +1,38 @@ -use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Stmt, StmtKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::{Diagnostic, Rule}; -use crate::violations; +use crate::violation::Violation; use crate::visibility::{is_abstract, is_overload}; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Stmt, StmtKind}; + +define_violation!( + pub struct AbstractBaseClassWithoutAbstractMethod { + pub name: String, + } +); +impl Violation for AbstractBaseClassWithoutAbstractMethod { + #[derive_message_formats] + fn message(&self) -> String { + let AbstractBaseClassWithoutAbstractMethod { name } = self; + format!("`{name}` is an abstract base class, but it has no abstract methods") + } +} +define_violation!( + pub struct EmptyMethodWithoutAbstractDecorator { + pub name: String, + } +); +impl Violation for EmptyMethodWithoutAbstractDecorator { + #[derive_message_formats] + fn message(&self) -> String { + let EmptyMethodWithoutAbstractDecorator { name } = self; + format!( + "`{name}` is an empty method in an abstract base class, but has no abstract decorator" + ) + } +} fn is_abc_class(checker: &Checker, bases: &[Expr], keywords: &[Keyword]) -> bool { keywords.iter().any(|keyword| { @@ -91,7 +119,7 @@ pub fn abstract_base_class( if !has_abstract_decorator && is_empty_body(body) && !is_overload(checker, decorator_list) { checker.diagnostics.push(Diagnostic::new( - violations::EmptyMethodWithoutAbstractDecorator { + EmptyMethodWithoutAbstractDecorator { name: format!("{name}.{method_name}"), }, Range::from_located(stmt), @@ -105,7 +133,7 @@ pub fn abstract_base_class( { if !has_abstract_method { checker.diagnostics.push(Diagnostic::new( - violations::AbstractBaseClassWithoutAbstractMethod { + AbstractBaseClassWithoutAbstractMethod { name: name.to_string(), }, Range::from_located(stmt), diff --git a/src/rules/flake8_bugbear/rules/assert_false.rs b/src/rules/flake8_bugbear/rules/assert_false.rs index a832448f0a..95f2104c33 100644 --- a/src/rules/flake8_bugbear/rules/assert_false.rs +++ b/src/rules/flake8_bugbear/rules/assert_false.rs @@ -1,11 +1,26 @@ -use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind}; - use crate::ast::helpers::unparse_stmt; use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::fix::Fix; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::AlwaysAutofixableViolation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind}; + +define_violation!( + pub struct DoNotAssertFalse; +); +impl AlwaysAutofixableViolation for DoNotAssertFalse { + #[derive_message_formats] + fn message(&self) -> String { + format!("Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`") + } + + fn autofix_title(&self) -> String { + "Replace `assert False`".to_string() + } +} fn assertion_error(msg: Option<&Expr>) -> Stmt { Stmt::new( @@ -46,7 +61,7 @@ pub fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg: Option return; }; - let mut diagnostic = Diagnostic::new(violations::DoNotAssertFalse, Range::from_located(test)); + let mut diagnostic = Diagnostic::new(DoNotAssertFalse, Range::from_located(test)); if checker.patch(diagnostic.kind.rule()) { diagnostic.amend(Fix::replacement( unparse_stmt(&assertion_error(msg), checker.stylist), diff --git a/src/rules/flake8_bugbear/rules/assert_raises_exception.rs b/src/rules/flake8_bugbear/rules/assert_raises_exception.rs index b838ed5e41..de929691e3 100644 --- a/src/rules/flake8_bugbear/rules/assert_raises_exception.rs +++ b/src/rules/flake8_bugbear/rules/assert_raises_exception.rs @@ -7,13 +7,23 @@ //! typo. Either assert for a more specific exception (builtin or //! custom), use `assertRaisesRegex`, or use the context manager form of //! `assertRaises`. - -use rustpython_ast::{ExprKind, Stmt, Withitem}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{ExprKind, Stmt, Withitem}; + +define_violation!( + pub struct NoAssertRaisesException; +); +impl Violation for NoAssertRaisesException { + #[derive_message_formats] + fn message(&self) -> String { + format!("`assertRaises(Exception)` should be considered evil") + } +} /// B017 pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[Withitem]) { @@ -41,7 +51,7 @@ pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[With } checker.diagnostics.push(Diagnostic::new( - violations::NoAssertRaisesException, + NoAssertRaisesException, Range::from_located(stmt), )); } diff --git a/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs b/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs index 8fff3f172b..9acb69c732 100644 --- a/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs +++ b/src/rules/flake8_bugbear/rules/assignment_to_os_environ.rs @@ -1,10 +1,20 @@ -use rustpython_ast::{Expr, ExprKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind}; +define_violation!( + pub struct AssignmentToOsEnviron; +); +impl Violation for AssignmentToOsEnviron { + #[derive_message_formats] + fn message(&self) -> String { + format!("Assigning to `os.environ` doesn't clear the environment") + } +} /// B003 pub fn assignment_to_os_environ(checker: &mut Checker, targets: &[Expr]) { if targets.len() != 1 { @@ -24,7 +34,7 @@ pub fn assignment_to_os_environ(checker: &mut Checker, targets: &[Expr]) { return; } checker.diagnostics.push(Diagnostic::new( - violations::AssignmentToOsEnviron, + AssignmentToOsEnviron, Range::from_located(target), )); } diff --git a/src/rules/flake8_bugbear/rules/cached_instance_method.rs b/src/rules/flake8_bugbear/rules/cached_instance_method.rs index 5f638815ed..d43fd3643e 100644 --- a/src/rules/flake8_bugbear/rules/cached_instance_method.rs +++ b/src/rules/flake8_bugbear/rules/cached_instance_method.rs @@ -1,9 +1,22 @@ -use rustpython_ast::{Expr, ExprKind}; - use crate::ast::types::{Range, ScopeKind}; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind}; + +define_violation!( + pub struct CachedInstanceMethod; +); +impl Violation for CachedInstanceMethod { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks" + ) + } +} fn is_cache_func(checker: &Checker, expr: &Expr) -> bool { checker.resolve_call_path(expr).map_or(false, |call_path| { @@ -35,7 +48,7 @@ pub fn cached_instance_method(checker: &mut Checker, decorator_list: &[Expr]) { }, ) { checker.diagnostics.push(Diagnostic::new( - violations::CachedInstanceMethod, + CachedInstanceMethod, Range::from_located(decorator), )); } diff --git a/src/rules/flake8_bugbear/rules/cannot_raise_literal.rs b/src/rules/flake8_bugbear/rules/cannot_raise_literal.rs index b0eaa182f3..1cfbb0e246 100644 --- a/src/rules/flake8_bugbear/rules/cannot_raise_literal.rs +++ b/src/rules/flake8_bugbear/rules/cannot_raise_literal.rs @@ -1,9 +1,20 @@ -use rustpython_ast::{Expr, ExprKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind}; + +define_violation!( + pub struct CannotRaiseLiteral; +); +impl Violation for CannotRaiseLiteral { + #[derive_message_formats] + fn message(&self) -> String { + format!("Cannot raise a literal. Did you intend to return it or raise an Exception?") + } +} /// B016 pub fn cannot_raise_literal(checker: &mut Checker, expr: &Expr) { @@ -11,7 +22,7 @@ pub fn cannot_raise_literal(checker: &mut Checker, expr: &Expr) { return; }; checker.diagnostics.push(Diagnostic::new( - violations::CannotRaiseLiteral, + CannotRaiseLiteral, Range::from_located(expr), )); } diff --git a/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs b/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs index 83515f01f9..bfb4bebe8d 100644 --- a/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs +++ b/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs @@ -1,14 +1,50 @@ -use itertools::Itertools; -use rustc_hash::{FxHashMap, FxHashSet}; -use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Location}; - use crate::ast::helpers; use crate::ast::helpers::unparse_expr; use crate::ast::types::{CallPath, Range}; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::fix::Fix; use crate::registry::{Diagnostic, Rule}; -use crate::violations; +use crate::violation::{AlwaysAutofixableViolation, Violation}; +use itertools::Itertools; +use ruff_macros::derive_message_formats; +use rustc_hash::{FxHashMap, FxHashSet}; +use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind, Location}; + +define_violation!( + pub struct DuplicateTryBlockException { + pub name: String, + } +); +impl Violation for DuplicateTryBlockException { + #[derive_message_formats] + fn message(&self) -> String { + let DuplicateTryBlockException { name } = self; + format!("try-except block with duplicate exception `{name}`") + } +} +define_violation!( + pub struct DuplicateHandlerException { + pub names: Vec, + } +); +impl AlwaysAutofixableViolation for DuplicateHandlerException { + #[derive_message_formats] + fn message(&self) -> String { + let DuplicateHandlerException { names } = self; + if names.len() == 1 { + let name = &names[0]; + format!("Exception handler with duplicate exception: `{name}`") + } else { + let names = names.iter().map(|name| format!("`{name}`")).join(", "); + format!("Exception handler with duplicate exceptions: {names}") + } + } + + fn autofix_title(&self) -> String { + "De-duplicate exceptions".to_string() + } +} fn type_pattern(elts: Vec<&Expr>) -> Expr { Expr::new( @@ -49,7 +85,7 @@ fn duplicate_handler_exceptions<'a>( // TODO(charlie): Handle "BaseException" and redundant exception aliases. if !duplicates.is_empty() { let mut diagnostic = Diagnostic::new( - violations::DuplicateHandlerException { + DuplicateHandlerException { names: duplicates .into_iter() .map(|call_path| call_path.join(".")) @@ -115,7 +151,7 @@ pub fn duplicate_exceptions(checker: &mut Checker, handlers: &[Excepthandler]) { for (name, exprs) in duplicates { for expr in exprs { checker.diagnostics.push(Diagnostic::new( - violations::DuplicateTryBlockException { + DuplicateTryBlockException { name: name.join("."), }, Range::from_located(expr), diff --git a/src/rules/flake8_bugbear/rules/f_string_docstring.rs b/src/rules/flake8_bugbear/rules/f_string_docstring.rs index 17ff1b1268..c7e1676b92 100644 --- a/src/rules/flake8_bugbear/rules/f_string_docstring.rs +++ b/src/rules/flake8_bugbear/rules/f_string_docstring.rs @@ -1,9 +1,23 @@ -use rustpython_ast::{ExprKind, Stmt, StmtKind}; - use crate::ast::helpers; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{ExprKind, Stmt, StmtKind}; + +define_violation!( + pub struct FStringDocstring; +); +impl Violation for FStringDocstring { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "f-string used as docstring. This will be interpreted by python as a joined string \ + rather than a docstring." + ) + } +} /// B021 pub fn f_string_docstring(checker: &mut Checker, body: &[Stmt]) { @@ -17,7 +31,7 @@ pub fn f_string_docstring(checker: &mut Checker, body: &[Stmt]) { return; }; checker.diagnostics.push(Diagnostic::new( - violations::FStringDocstring, + FStringDocstring, helpers::identifier_range(stmt, checker.locator), )); } diff --git a/src/rules/flake8_bugbear/rules/function_call_argument_default.rs b/src/rules/flake8_bugbear/rules/function_call_argument_default.rs index 7ceb390026..9e12bbc390 100644 --- a/src/rules/flake8_bugbear/rules/function_call_argument_default.rs +++ b/src/rules/flake8_bugbear/rules/function_call_argument_default.rs @@ -1,13 +1,31 @@ -use rustpython_ast::{Arguments, Constant, Expr, ExprKind}; - use super::mutable_argument_default::is_mutable_func; use crate::ast::helpers::{compose_call_path, to_call_path}; use crate::ast::types::{CallPath, Range}; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::{Diagnostic, DiagnosticKind}; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Arguments, Constant, Expr, ExprKind}; + +define_violation!( + pub struct FunctionCallArgumentDefault { + pub name: Option, + } +); +impl Violation for FunctionCallArgumentDefault { + #[derive_message_formats] + fn message(&self) -> String { + let FunctionCallArgumentDefault { name } = self; + if let Some(name) = name { + format!("Do not perform function call `{name}` in argument defaults") + } else { + format!("Do not perform function call in argument defaults") + } + } +} const IMMUTABLE_FUNCS: &[&[&str]] = &[ &["", "tuple"], @@ -48,7 +66,7 @@ where && !is_nan_or_infinity(func, args) { self.diagnostics.push(( - violations::FunctionCallArgumentDefault { + FunctionCallArgumentDefault { name: compose_call_path(expr), } .into(), diff --git a/src/rules/flake8_bugbear/rules/function_uses_loop_variable.rs b/src/rules/flake8_bugbear/rules/function_uses_loop_variable.rs index 9f5afcd4ad..40403b5ace 100644 --- a/src/rules/flake8_bugbear/rules/function_uses_loop_variable.rs +++ b/src/rules/flake8_bugbear/rules/function_uses_loop_variable.rs @@ -1,13 +1,27 @@ -use rustc_hash::FxHashSet; -use rustpython_ast::{Comprehension, Expr, ExprContext, ExprKind, Stmt, StmtKind}; - use crate::ast::helpers::collect_arg_names; use crate::ast::types::{Node, Range}; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustc_hash::FxHashSet; +use rustpython_ast::{Comprehension, Expr, ExprContext, ExprKind, Stmt, StmtKind}; + +define_violation!( + pub struct FunctionUsesLoopVariable { + pub name: String, + } +); +impl Violation for FunctionUsesLoopVariable { + #[derive_message_formats] + fn message(&self) -> String { + let FunctionUsesLoopVariable { name } = self; + format!("Function definition does not bind loop variable `{name}`") + } +} #[derive(Default)] struct LoadedNamesVisitor<'a> { @@ -260,7 +274,7 @@ where if !checker.flake8_bugbear_seen.contains(&expr) { checker.flake8_bugbear_seen.push(expr); checker.diagnostics.push(Diagnostic::new( - violations::FunctionUsesLoopVariable { + FunctionUsesLoopVariable { name: name.to_string(), }, range, diff --git a/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/src/rules/flake8_bugbear/rules/getattr_with_constant.rs index 1b8b517ea1..70d945809c 100644 --- a/src/rules/flake8_bugbear/rules/getattr_with_constant.rs +++ b/src/rules/flake8_bugbear/rules/getattr_with_constant.rs @@ -1,14 +1,31 @@ -use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location}; - use crate::ast::helpers::unparse_expr; use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::fix::Fix; use crate::python::identifiers::{is_identifier, is_mangled_private}; use crate::python::keyword::KWLIST; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::AlwaysAutofixableViolation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location}; +define_violation!( + pub struct GetAttrWithConstant; +); +impl AlwaysAutofixableViolation for GetAttrWithConstant { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Do not call `getattr` with a constant attribute value. It is not any safer than \ + normal property access." + ) + } + + fn autofix_title(&self) -> String { + "Replace `getattr` with attribute access".to_string() + } +} fn attribute(value: &Expr, attr: &str) -> Expr { Expr::new( Location::default(), @@ -45,8 +62,7 @@ pub fn getattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar return; } - let mut diagnostic = - Diagnostic::new(violations::GetAttrWithConstant, Range::from_located(expr)); + let mut diagnostic = Diagnostic::new(GetAttrWithConstant, Range::from_located(expr)); if checker.patch(diagnostic.kind.rule()) { diagnostic.amend(Fix::replacement( diff --git a/src/rules/flake8_bugbear/rules/jump_statement_in_finally.rs b/src/rules/flake8_bugbear/rules/jump_statement_in_finally.rs index 71ff8e8f63..e6e23ef30a 100644 --- a/src/rules/flake8_bugbear/rules/jump_statement_in_finally.rs +++ b/src/rules/flake8_bugbear/rules/jump_statement_in_finally.rs @@ -1,15 +1,29 @@ -use rustpython_ast::{Stmt, StmtKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Stmt, StmtKind}; + +define_violation!( + pub struct JumpStatementInFinally { + pub name: String, + } +); +impl Violation for JumpStatementInFinally { + #[derive_message_formats] + fn message(&self) -> String { + let JumpStatementInFinally { name } = self; + format!("`{name}` inside `finally` blocks cause exceptions to be silenced") + } +} fn walk_stmt(checker: &mut Checker, body: &[Stmt], f: fn(&Stmt) -> bool) { for stmt in body { if f(stmt) { checker.diagnostics.push(Diagnostic::new( - violations::JumpStatementInFinally { + JumpStatementInFinally { name: match &stmt.node { StmtKind::Break { .. } => "break".to_string(), StmtKind::Continue { .. } => "continue".to_string(), diff --git a/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs b/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs index ac48d9c75d..2899632b11 100644 --- a/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs +++ b/src/rules/flake8_bugbear/rules/loop_variable_overrides_iterator.rs @@ -1,12 +1,26 @@ -use rustc_hash::FxHashMap; -use rustpython_ast::{Expr, ExprKind}; - use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustc_hash::FxHashMap; +use rustpython_ast::{Expr, ExprKind}; + +define_violation!( + pub struct LoopVariableOverridesIterator { + pub name: String, + } +); +impl Violation for LoopVariableOverridesIterator { + #[derive_message_formats] + fn message(&self) -> String { + let LoopVariableOverridesIterator { name } = self; + format!("Loop control variable `{name}` overrides iterable it iterates") + } +} #[derive(Default)] struct NameFinder<'a> { @@ -57,7 +71,7 @@ pub fn loop_variable_overrides_iterator(checker: &mut Checker, target: &Expr, it for (name, expr) in target_names { if iter_names.contains_key(name) { checker.diagnostics.push(Diagnostic::new( - violations::LoopVariableOverridesIterator { + LoopVariableOverridesIterator { name: name.to_string(), }, Range::from_located(expr), diff --git a/src/rules/flake8_bugbear/rules/mod.rs b/src/rules/flake8_bugbear/rules/mod.rs index 49459143f5..318872fb1e 100644 --- a/src/rules/flake8_bugbear/rules/mod.rs +++ b/src/rules/flake8_bugbear/rules/mod.rs @@ -1,29 +1,45 @@ -pub use abstract_base_class::abstract_base_class; -pub use assert_false::assert_false; -pub use assert_raises_exception::assert_raises_exception; -pub use assignment_to_os_environ::assignment_to_os_environ; -pub use cached_instance_method::cached_instance_method; -pub use cannot_raise_literal::cannot_raise_literal; -pub use duplicate_exceptions::duplicate_exceptions; -pub use f_string_docstring::f_string_docstring; -pub use function_call_argument_default::function_call_argument_default; -pub use function_uses_loop_variable::function_uses_loop_variable; -pub use getattr_with_constant::getattr_with_constant; -pub use jump_statement_in_finally::jump_statement_in_finally; -pub use loop_variable_overrides_iterator::loop_variable_overrides_iterator; -pub use mutable_argument_default::mutable_argument_default; -pub use raise_without_from_inside_except::raise_without_from_inside_except; -pub use redundant_tuple_in_exception_handler::redundant_tuple_in_exception_handler; -pub use setattr_with_constant::setattr_with_constant; -pub use star_arg_unpacking_after_keyword_arg::star_arg_unpacking_after_keyword_arg; -pub use strip_with_multi_characters::strip_with_multi_characters; -pub use unary_prefix_increment::unary_prefix_increment; -pub use unreliable_callable_check::unreliable_callable_check; -pub use unused_loop_control_variable::unused_loop_control_variable; -pub use useless_comparison::useless_comparison; -pub use useless_contextlib_suppress::useless_contextlib_suppress; -pub use useless_expression::useless_expression; -pub use zip_without_explicit_strict::zip_without_explicit_strict; +pub use abstract_base_class::{ + abstract_base_class, AbstractBaseClassWithoutAbstractMethod, + EmptyMethodWithoutAbstractDecorator, +}; +pub use assert_false::{assert_false, DoNotAssertFalse}; +pub use assert_raises_exception::{assert_raises_exception, NoAssertRaisesException}; +pub use assignment_to_os_environ::{assignment_to_os_environ, AssignmentToOsEnviron}; +pub use cached_instance_method::{cached_instance_method, CachedInstanceMethod}; +pub use cannot_raise_literal::{cannot_raise_literal, CannotRaiseLiteral}; +pub use duplicate_exceptions::{ + duplicate_exceptions, DuplicateHandlerException, DuplicateTryBlockException, +}; +pub use f_string_docstring::{f_string_docstring, FStringDocstring}; +pub use function_call_argument_default::{ + function_call_argument_default, FunctionCallArgumentDefault, +}; +pub use function_uses_loop_variable::{function_uses_loop_variable, FunctionUsesLoopVariable}; +pub use getattr_with_constant::{getattr_with_constant, GetAttrWithConstant}; +pub use jump_statement_in_finally::{jump_statement_in_finally, JumpStatementInFinally}; +pub use loop_variable_overrides_iterator::{ + loop_variable_overrides_iterator, LoopVariableOverridesIterator, +}; +pub use mutable_argument_default::{mutable_argument_default, MutableArgumentDefault}; +pub use raise_without_from_inside_except::{ + raise_without_from_inside_except, RaiseWithoutFromInsideExcept, +}; +pub use redundant_tuple_in_exception_handler::{ + redundant_tuple_in_exception_handler, RedundantTupleInExceptionHandler, +}; + +pub use setattr_with_constant::{setattr_with_constant, SetAttrWithConstant}; +pub use star_arg_unpacking_after_keyword_arg::{ + star_arg_unpacking_after_keyword_arg, StarArgUnpackingAfterKeywordArg, +}; +pub use strip_with_multi_characters::{strip_with_multi_characters, StripWithMultiCharacters}; +pub use unary_prefix_increment::{unary_prefix_increment, UnaryPrefixIncrement}; +pub use unreliable_callable_check::{unreliable_callable_check, UnreliableCallableCheck}; +pub use unused_loop_control_variable::{unused_loop_control_variable, UnusedLoopControlVariable}; +pub use useless_comparison::{useless_comparison, UselessComparison}; +pub use useless_contextlib_suppress::{useless_contextlib_suppress, UselessContextlibSuppress}; +pub use useless_expression::{useless_expression, UselessExpression}; +pub use zip_without_explicit_strict::{zip_without_explicit_strict, ZipWithoutExplicitStrict}; mod abstract_base_class; mod assert_false; diff --git a/src/rules/flake8_bugbear/rules/mutable_argument_default.rs b/src/rules/flake8_bugbear/rules/mutable_argument_default.rs index 8990c873ea..83257cc204 100644 --- a/src/rules/flake8_bugbear/rules/mutable_argument_default.rs +++ b/src/rules/flake8_bugbear/rules/mutable_argument_default.rs @@ -1,10 +1,20 @@ -use rustpython_ast::{Arguments, Constant, Expr, ExprKind, Operator}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Arguments, Constant, Expr, ExprKind, Operator}; +define_violation!( + pub struct MutableArgumentDefault; +); +impl Violation for MutableArgumentDefault { + #[derive_message_formats] + fn message(&self) -> String { + format!("Do not use mutable data structures for argument defaults") + } +} const MUTABLE_FUNCS: &[&[&str]] = &[ &["", "dict"], &["", "list"], @@ -152,7 +162,7 @@ pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { .map_or(false, |expr| is_immutable_annotation(checker, expr)) { checker.diagnostics.push(Diagnostic::new( - violations::MutableArgumentDefault, + MutableArgumentDefault, Range::from_located(default), )); } diff --git a/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs b/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs index 3542d58567..e015f86ecf 100644 --- a/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs +++ b/src/rules/flake8_bugbear/rules/raise_without_from_inside_except.rs @@ -1,11 +1,25 @@ -use rustpython_ast::{ExprKind, Stmt, StmtKind}; - use crate::ast::types::Range; use crate::ast::visitor::Visitor; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::python::string::is_lower; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{ExprKind, Stmt, StmtKind}; + +define_violation!( + pub struct RaiseWithoutFromInsideExcept; +); +impl Violation for RaiseWithoutFromInsideExcept { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Within an except clause, raise exceptions with `raise ... from err` or `raise ... \ + from None` to distinguish them from errors in exception handling" + ) + } +} struct RaiseVisitor { diagnostics: Vec, @@ -21,7 +35,7 @@ impl<'a> Visitor<'a> for RaiseVisitor { ExprKind::Name { id, .. } if is_lower(id) => {} _ => { self.diagnostics.push(Diagnostic::new( - violations::RaiseWithoutFromInsideExcept, + RaiseWithoutFromInsideExcept, Range::from_located(stmt), )); } diff --git a/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs b/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs index f08938cb1b..a1004b916f 100644 --- a/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs +++ b/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs @@ -1,11 +1,33 @@ -use rustpython_ast::{Excepthandler, ExcepthandlerKind, ExprKind}; - use crate::ast::helpers::unparse_expr; use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::fix::Fix; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::AlwaysAutofixableViolation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Excepthandler, ExcepthandlerKind, ExprKind}; + +define_violation!( + pub struct RedundantTupleInExceptionHandler { + pub name: String, + } +); +impl AlwaysAutofixableViolation for RedundantTupleInExceptionHandler { + #[derive_message_formats] + fn message(&self) -> String { + let RedundantTupleInExceptionHandler { name } = self; + format!( + "A length-one tuple literal is redundant. Write `except {name}` instead of `except \ + ({name},)`." + ) + } + + fn autofix_title(&self) -> String { + let RedundantTupleInExceptionHandler { name } = self; + format!("Replace with `except {name}`") + } +} /// B013 pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[Excepthandler]) { @@ -20,7 +42,7 @@ pub fn redundant_tuple_in_exception_handler(checker: &mut Checker, handlers: &[E continue; }; let mut diagnostic = Diagnostic::new( - violations::RedundantTupleInExceptionHandler { + RedundantTupleInExceptionHandler { name: unparse_expr(elt, checker.stylist), }, Range::from_located(type_), diff --git a/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/src/rules/flake8_bugbear/rules/setattr_with_constant.rs index 74cf7d0393..8be13cfbe6 100644 --- a/src/rules/flake8_bugbear/rules/setattr_with_constant.rs +++ b/src/rules/flake8_bugbear/rules/setattr_with_constant.rs @@ -1,14 +1,32 @@ -use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind}; - use crate::ast::helpers::unparse_stmt; use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::fix::Fix; use crate::python::identifiers::{is_identifier, is_mangled_private}; use crate::python::keyword::KWLIST; use crate::registry::Diagnostic; use crate::source_code::Stylist; -use crate::violations; +use crate::violation::AlwaysAutofixableViolation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, Location, Stmt, StmtKind}; + +define_violation!( + pub struct SetAttrWithConstant; +); +impl AlwaysAutofixableViolation for SetAttrWithConstant { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Do not call `setattr` with a constant attribute value. It is not any safer than \ + normal property access." + ) + } + + fn autofix_title(&self) -> String { + "Replace `setattr` with assignment".to_string() + } +} fn assignment(obj: &Expr, name: &str, value: &Expr, stylist: &Stylist) -> String { let stmt = Stmt::new( @@ -59,8 +77,7 @@ pub fn setattr_with_constant(checker: &mut Checker, expr: &Expr, func: &Expr, ar // (i.e., it's directly within an `StmtKind::Expr`). if let StmtKind::Expr { value: child } = &checker.current_stmt().node { if expr == child.as_ref() { - let mut diagnostic = - Diagnostic::new(violations::SetAttrWithConstant, Range::from_located(expr)); + let mut diagnostic = Diagnostic::new(SetAttrWithConstant, Range::from_located(expr)); if checker.patch(diagnostic.kind.rule()) { diagnostic.amend(Fix::replacement( diff --git a/src/rules/flake8_bugbear/rules/star_arg_unpacking_after_keyword_arg.rs b/src/rules/flake8_bugbear/rules/star_arg_unpacking_after_keyword_arg.rs index 8e73e9b855..e59b7a8682 100644 --- a/src/rules/flake8_bugbear/rules/star_arg_unpacking_after_keyword_arg.rs +++ b/src/rules/flake8_bugbear/rules/star_arg_unpacking_after_keyword_arg.rs @@ -7,12 +7,23 @@ //! by the unpacked sequence, and this change of ordering can surprise and //! mislead readers. -use rustpython_ast::{Expr, ExprKind, Keyword}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind, Keyword}; + +define_violation!( + pub struct StarArgUnpackingAfterKeywordArg; +); +impl Violation for StarArgUnpackingAfterKeywordArg { + #[derive_message_formats] + fn message(&self) -> String { + format!("Star-arg unpacking after a keyword argument is strongly discouraged") + } +} /// B026 pub fn star_arg_unpacking_after_keyword_arg( @@ -31,7 +42,7 @@ pub fn star_arg_unpacking_after_keyword_arg( continue; } checker.diagnostics.push(Diagnostic::new( - violations::StarArgUnpackingAfterKeywordArg, + StarArgUnpackingAfterKeywordArg, Range::from_located(arg), )); } diff --git a/src/rules/flake8_bugbear/rules/strip_with_multi_characters.rs b/src/rules/flake8_bugbear/rules/strip_with_multi_characters.rs index cf858b5f11..d0361ce7e6 100644 --- a/src/rules/flake8_bugbear/rules/strip_with_multi_characters.rs +++ b/src/rules/flake8_bugbear/rules/strip_with_multi_characters.rs @@ -1,10 +1,21 @@ -use itertools::Itertools; -use rustpython_ast::{Constant, Expr, ExprKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use itertools::Itertools; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, Expr, ExprKind}; + +define_violation!( + pub struct StripWithMultiCharacters; +); +impl Violation for StripWithMultiCharacters { + #[derive_message_formats] + fn message(&self) -> String { + format!("Using `.strip()` with multi-character strings is misleading the reader") + } +} /// B005 pub fn strip_with_multi_characters(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { @@ -27,7 +38,7 @@ pub fn strip_with_multi_characters(checker: &mut Checker, expr: &Expr, func: &Ex if value.len() > 1 && value.chars().unique().count() != value.len() { checker.diagnostics.push(Diagnostic::new( - violations::StripWithMultiCharacters, + StripWithMultiCharacters, Range::from_located(expr), )); } diff --git a/src/rules/flake8_bugbear/rules/unary_prefix_increment.rs b/src/rules/flake8_bugbear/rules/unary_prefix_increment.rs index 2f474af564..8e45cb4b62 100644 --- a/src/rules/flake8_bugbear/rules/unary_prefix_increment.rs +++ b/src/rules/flake8_bugbear/rules/unary_prefix_increment.rs @@ -17,12 +17,23 @@ //! n += 1 //! ``` -use rustpython_ast::{Expr, ExprKind, Unaryop}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind, Unaryop}; + +define_violation!( + pub struct UnaryPrefixIncrement; +); +impl Violation for UnaryPrefixIncrement { + #[derive_message_formats] + fn message(&self) -> String { + format!("Python does not support the unary prefix increment") + } +} /// B002 pub fn unary_prefix_increment(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) { @@ -36,7 +47,7 @@ pub fn unary_prefix_increment(checker: &mut Checker, expr: &Expr, op: &Unaryop, return; } checker.diagnostics.push(Diagnostic::new( - violations::UnaryPrefixIncrement, + UnaryPrefixIncrement, Range::from_located(expr), )); } diff --git a/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs b/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs index 6d95cab675..2b9dea1cad 100644 --- a/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs +++ b/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs @@ -1,9 +1,23 @@ -use rustpython_ast::{Constant, Expr, ExprKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, Expr, ExprKind}; + +define_violation!( + pub struct UnreliableCallableCheck; +); +impl Violation for UnreliableCallableCheck { + #[derive_message_formats] + fn message(&self) -> String { + format!( + " Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use \ + `callable(x)` for consistent results." + ) + } +} /// B004 pub fn unreliable_callable_check(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { @@ -27,7 +41,7 @@ pub fn unreliable_callable_check(checker: &mut Checker, expr: &Expr, func: &Expr return; } checker.diagnostics.push(Diagnostic::new( - violations::UnreliableCallableCheck, + UnreliableCallableCheck, Range::from_located(expr), )); } diff --git a/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs b/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs index 2d5de155d7..f53758f437 100644 --- a/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs +++ b/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs @@ -18,16 +18,52 @@ //! method() //! ``` -use rustc_hash::FxHashMap; -use rustpython_ast::{Expr, ExprKind, Stmt}; - use crate::ast::types::Range; use crate::ast::visitor::Visitor; use crate::ast::{helpers, visitor}; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::fix::Fix; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::{AutofixKind, Availability, Violation}; +use ruff_macros::derive_message_formats; +use rustc_hash::FxHashMap; +use rustpython_ast::{Expr, ExprKind, Stmt}; + +define_violation!( + pub struct UnusedLoopControlVariable { + /// The name of the loop control variable. + pub name: String, + /// Whether the variable is safe to rename. A variable is unsafe to + /// rename if it _might_ be used by magic control flow (e.g., + /// `locals()`). + pub safe: bool, + } +); +impl Violation for UnusedLoopControlVariable { + const AUTOFIX: Option = Some(AutofixKind::new(Availability::Always)); + + #[derive_message_formats] + fn message(&self) -> String { + let UnusedLoopControlVariable { name, safe } = self; + if *safe { + format!("Loop control variable `{name}` not used within loop body") + } else { + format!("Loop control variable `{name}` may not be used within loop body") + } + } + + fn autofix_title_formatter(&self) -> Option String> { + let UnusedLoopControlVariable { safe, .. } = self; + if *safe { + Some(|UnusedLoopControlVariable { name, .. }| { + format!("Rename unused `{name}` to `_{name}`") + }) + } else { + None + } + } +} /// Identify all `ExprKind::Name` nodes in an AST. struct NameFinder<'a> { @@ -84,7 +120,7 @@ pub fn unused_loop_control_variable(checker: &mut Checker, target: &Expr, body: let safe = !helpers::uses_magic_variable_access(checker, body); let mut diagnostic = Diagnostic::new( - violations::UnusedLoopControlVariable { + UnusedLoopControlVariable { name: name.to_string(), safe, }, diff --git a/src/rules/flake8_bugbear/rules/useless_comparison.rs b/src/rules/flake8_bugbear/rules/useless_comparison.rs index ecb65941d6..d330a2bd67 100644 --- a/src/rules/flake8_bugbear/rules/useless_comparison.rs +++ b/src/rules/flake8_bugbear/rules/useless_comparison.rs @@ -1,14 +1,28 @@ -use rustpython_ast::{Expr, ExprKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind}; + +define_violation!( + pub struct UselessComparison; +); +impl Violation for UselessComparison { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "Pointless comparison. This comparison does nothing but waste CPU instructions. \ + Either prepend `assert` or remove it." + ) + } +} pub fn useless_comparison(checker: &mut Checker, expr: &Expr) { if matches!(expr.node, ExprKind::Compare { .. }) { checker.diagnostics.push(Diagnostic::new( - violations::UselessComparison, + UselessComparison, Range::from_located(expr), )); } diff --git a/src/rules/flake8_bugbear/rules/useless_contextlib_suppress.rs b/src/rules/flake8_bugbear/rules/useless_contextlib_suppress.rs index b7e1e6bfa9..55bb43c960 100644 --- a/src/rules/flake8_bugbear/rules/useless_contextlib_suppress.rs +++ b/src/rules/flake8_bugbear/rules/useless_contextlib_suppress.rs @@ -1,9 +1,23 @@ -use rustpython_ast::Expr; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::Expr; + +define_violation!( + pub struct UselessContextlibSuppress; +); +impl Violation for UselessContextlibSuppress { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "No arguments passed to `contextlib.suppress`. No exceptions will be suppressed and \ + therefore this context manager is redundant" + ) + } +} /// B005 pub fn useless_contextlib_suppress(checker: &mut Checker, expr: &Expr, args: &[Expr]) { @@ -13,7 +27,7 @@ pub fn useless_contextlib_suppress(checker: &mut Checker, expr: &Expr, args: &[E }) { checker.diagnostics.push(Diagnostic::new( - violations::UselessContextlibSuppress, + UselessContextlibSuppress, Range::from_located(expr), )); } diff --git a/src/rules/flake8_bugbear/rules/useless_expression.rs b/src/rules/flake8_bugbear/rules/useless_expression.rs index 4016374c33..8adadfde2a 100644 --- a/src/rules/flake8_bugbear/rules/useless_expression.rs +++ b/src/rules/flake8_bugbear/rules/useless_expression.rs @@ -1,9 +1,20 @@ -use rustpython_ast::{Constant, ExprKind, Stmt, StmtKind}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Constant, ExprKind, Stmt, StmtKind}; + +define_violation!( + pub struct UselessExpression; +); +impl Violation for UselessExpression { + #[derive_message_formats] + fn message(&self) -> String { + format!("Found useless expression. Either assign it to a variable or remove it.") + } +} // B018 pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) { @@ -12,7 +23,7 @@ pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) { match &value.node { ExprKind::List { .. } | ExprKind::Dict { .. } | ExprKind::Set { .. } => { checker.diagnostics.push(Diagnostic::new( - violations::UselessExpression, + UselessExpression, Range::from_located(value), )); } @@ -20,7 +31,7 @@ pub fn useless_expression(checker: &mut Checker, body: &[Stmt]) { Constant::Str { .. } | Constant::Ellipsis => {} _ => { checker.diagnostics.push(Diagnostic::new( - violations::UselessExpression, + UselessExpression, Range::from_located(value), )); } diff --git a/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs b/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs index 96fe80ca60..34c60ce230 100644 --- a/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs +++ b/src/rules/flake8_bugbear/rules/zip_without_explicit_strict.rs @@ -1,9 +1,20 @@ -use rustpython_ast::{Expr, ExprKind, Keyword}; - use crate::ast::types::Range; use crate::checkers::ast::Checker; +use crate::define_violation; use crate::registry::Diagnostic; -use crate::violations; +use crate::violation::Violation; +use ruff_macros::derive_message_formats; +use rustpython_ast::{Expr, ExprKind, Keyword}; + +define_violation!( + pub struct ZipWithoutExplicitStrict; +); +impl Violation for ZipWithoutExplicitStrict { + #[derive_message_formats] + fn message(&self) -> String { + format!("`zip()` without an explicit `strict=` parameter") + } +} /// B905 pub fn zip_without_explicit_strict( @@ -24,7 +35,7 @@ pub fn zip_without_explicit_strict( }) { checker.diagnostics.push(Diagnostic::new( - violations::ZipWithoutExplicitStrict, + ZipWithoutExplicitStrict, Range::from_located(expr), )); } diff --git a/src/violations.rs b/src/violations.rs index e65c01351f..cb351f72a6 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -988,399 +988,6 @@ impl Violation for BuiltinAttributeShadowing { } } -// flake8-bugbear - -define_violation!( - pub struct UnaryPrefixIncrement; -); -impl Violation for UnaryPrefixIncrement { - #[derive_message_formats] - fn message(&self) -> String { - format!("Python does not support the unary prefix increment") - } -} - -define_violation!( - pub struct AssignmentToOsEnviron; -); -impl Violation for AssignmentToOsEnviron { - #[derive_message_formats] - fn message(&self) -> String { - format!("Assigning to `os.environ` doesn't clear the environment") - } -} - -define_violation!( - pub struct UnreliableCallableCheck; -); -impl Violation for UnreliableCallableCheck { - #[derive_message_formats] - fn message(&self) -> String { - format!( - " Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use \ - `callable(x)` for consistent results." - ) - } -} - -define_violation!( - pub struct StripWithMultiCharacters; -); -impl Violation for StripWithMultiCharacters { - #[derive_message_formats] - fn message(&self) -> String { - format!("Using `.strip()` with multi-character strings is misleading the reader") - } -} - -define_violation!( - pub struct MutableArgumentDefault; -); -impl Violation for MutableArgumentDefault { - #[derive_message_formats] - fn message(&self) -> String { - format!("Do not use mutable data structures for argument defaults") - } -} - -define_violation!( - pub struct UnusedLoopControlVariable { - /// The name of the loop control variable. - pub name: String, - /// Whether the variable is safe to rename. A variable is unsafe to - /// rename if it _might_ be used by magic control flow (e.g., - /// `locals()`). - pub safe: bool, - } -); -impl Violation for UnusedLoopControlVariable { - const AUTOFIX: Option = Some(AutofixKind::new(Availability::Always)); - - #[derive_message_formats] - fn message(&self) -> String { - let UnusedLoopControlVariable { name, safe } = self; - if *safe { - format!("Loop control variable `{name}` not used within loop body") - } else { - format!("Loop control variable `{name}` may not be used within loop body") - } - } - - fn autofix_title_formatter(&self) -> Option String> { - let UnusedLoopControlVariable { safe, .. } = self; - if *safe { - Some(|UnusedLoopControlVariable { name, .. }| { - format!("Rename unused `{name}` to `_{name}`") - }) - } else { - None - } - } -} - -define_violation!( - pub struct FunctionCallArgumentDefault { - pub name: Option, - } -); -impl Violation for FunctionCallArgumentDefault { - #[derive_message_formats] - fn message(&self) -> String { - let FunctionCallArgumentDefault { name } = self; - if let Some(name) = name { - format!("Do not perform function call `{name}` in argument defaults") - } else { - format!("Do not perform function call in argument defaults") - } - } -} - -define_violation!( - pub struct GetAttrWithConstant; -); -impl AlwaysAutofixableViolation for GetAttrWithConstant { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "Do not call `getattr` with a constant attribute value. It is not any safer than \ - normal property access." - ) - } - - fn autofix_title(&self) -> String { - "Replace `getattr` with attribute access".to_string() - } -} - -define_violation!( - pub struct SetAttrWithConstant; -); -impl AlwaysAutofixableViolation for SetAttrWithConstant { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "Do not call `setattr` with a constant attribute value. It is not any safer than \ - normal property access." - ) - } - - fn autofix_title(&self) -> String { - "Replace `setattr` with assignment".to_string() - } -} - -define_violation!( - pub struct DoNotAssertFalse; -); -impl AlwaysAutofixableViolation for DoNotAssertFalse { - #[derive_message_formats] - fn message(&self) -> String { - format!("Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`") - } - - fn autofix_title(&self) -> String { - "Replace `assert False`".to_string() - } -} - -define_violation!( - pub struct JumpStatementInFinally { - pub name: String, - } -); -impl Violation for JumpStatementInFinally { - #[derive_message_formats] - fn message(&self) -> String { - let JumpStatementInFinally { name } = self; - format!("`{name}` inside `finally` blocks cause exceptions to be silenced") - } -} - -define_violation!( - pub struct RedundantTupleInExceptionHandler { - pub name: String, - } -); -impl AlwaysAutofixableViolation for RedundantTupleInExceptionHandler { - #[derive_message_formats] - fn message(&self) -> String { - let RedundantTupleInExceptionHandler { name } = self; - format!( - "A length-one tuple literal is redundant. Write `except {name}` instead of `except \ - ({name},)`." - ) - } - - fn autofix_title(&self) -> String { - let RedundantTupleInExceptionHandler { name } = self; - format!("Replace with `except {name}`") - } -} - -define_violation!( - pub struct DuplicateHandlerException { - pub names: Vec, - } -); -impl AlwaysAutofixableViolation for DuplicateHandlerException { - #[derive_message_formats] - fn message(&self) -> String { - let DuplicateHandlerException { names } = self; - if names.len() == 1 { - let name = &names[0]; - format!("Exception handler with duplicate exception: `{name}`") - } else { - let names = names.iter().map(|name| format!("`{name}`")).join(", "); - format!("Exception handler with duplicate exceptions: {names}") - } - } - - fn autofix_title(&self) -> String { - "De-duplicate exceptions".to_string() - } -} - -define_violation!( - pub struct UselessComparison; -); -impl Violation for UselessComparison { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "Pointless comparison. This comparison does nothing but waste CPU instructions. \ - Either prepend `assert` or remove it." - ) - } -} - -define_violation!( - pub struct CannotRaiseLiteral; -); -impl Violation for CannotRaiseLiteral { - #[derive_message_formats] - fn message(&self) -> String { - format!("Cannot raise a literal. Did you intend to return it or raise an Exception?") - } -} - -define_violation!( - pub struct NoAssertRaisesException; -); -impl Violation for NoAssertRaisesException { - #[derive_message_formats] - fn message(&self) -> String { - format!("`assertRaises(Exception)` should be considered evil") - } -} - -define_violation!( - pub struct UselessExpression; -); -impl Violation for UselessExpression { - #[derive_message_formats] - fn message(&self) -> String { - format!("Found useless expression. Either assign it to a variable or remove it.") - } -} - -define_violation!( - pub struct CachedInstanceMethod; -); -impl Violation for CachedInstanceMethod { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks" - ) - } -} - -define_violation!( - pub struct LoopVariableOverridesIterator { - pub name: String, - } -); -impl Violation for LoopVariableOverridesIterator { - #[derive_message_formats] - fn message(&self) -> String { - let LoopVariableOverridesIterator { name } = self; - format!("Loop control variable `{name}` overrides iterable it iterates") - } -} - -define_violation!( - pub struct FStringDocstring; -); -impl Violation for FStringDocstring { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "f-string used as docstring. This will be interpreted by python as a joined string \ - rather than a docstring." - ) - } -} - -define_violation!( - pub struct UselessContextlibSuppress; -); -impl Violation for UselessContextlibSuppress { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "No arguments passed to `contextlib.suppress`. No exceptions will be suppressed and \ - therefore this context manager is redundant" - ) - } -} - -define_violation!( - pub struct FunctionUsesLoopVariable { - pub name: String, - } -); -impl Violation for FunctionUsesLoopVariable { - #[derive_message_formats] - fn message(&self) -> String { - let FunctionUsesLoopVariable { name } = self; - format!("Function definition does not bind loop variable `{name}`") - } -} - -define_violation!( - pub struct AbstractBaseClassWithoutAbstractMethod { - pub name: String, - } -); -impl Violation for AbstractBaseClassWithoutAbstractMethod { - #[derive_message_formats] - fn message(&self) -> String { - let AbstractBaseClassWithoutAbstractMethod { name } = self; - format!("`{name}` is an abstract base class, but it has no abstract methods") - } -} - -define_violation!( - pub struct DuplicateTryBlockException { - pub name: String, - } -); -impl Violation for DuplicateTryBlockException { - #[derive_message_formats] - fn message(&self) -> String { - let DuplicateTryBlockException { name } = self; - format!("try-except block with duplicate exception `{name}`") - } -} - -define_violation!( - pub struct StarArgUnpackingAfterKeywordArg; -); -impl Violation for StarArgUnpackingAfterKeywordArg { - #[derive_message_formats] - fn message(&self) -> String { - format!("Star-arg unpacking after a keyword argument is strongly discouraged") - } -} - -define_violation!( - pub struct EmptyMethodWithoutAbstractDecorator { - pub name: String, - } -); -impl Violation for EmptyMethodWithoutAbstractDecorator { - #[derive_message_formats] - fn message(&self) -> String { - let EmptyMethodWithoutAbstractDecorator { name } = self; - format!( - "`{name}` is an empty method in an abstract base class, but has no abstract decorator" - ) - } -} - -define_violation!( - pub struct RaiseWithoutFromInsideExcept; -); -impl Violation for RaiseWithoutFromInsideExcept { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "Within an except clause, raise exceptions with `raise ... from err` or `raise ... \ - from None` to distinguish them from errors in exception handling" - ) - } -} - -define_violation!( - pub struct ZipWithoutExplicitStrict; -); -impl Violation for ZipWithoutExplicitStrict { - #[derive_message_formats] - fn message(&self) -> String { - format!("`zip()` without an explicit `strict=` parameter") - } -} - // flake8-blind-except define_violation!(