Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Move duplicate-value rule to flake8-bugbear (#4882)

This commit is contained in:
Charlie Marsh 2023-06-05 17:43:47 -04:00 committed by GitHub
parent a70afa7de7
commit c67029ded9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 113 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

0
crates/ruff/resources/test/fixtures/pylint/duplicate_value.py → crates/ruff/resources/test/fixtures/flake8_bugbear/B033.py vendored
View File

2
crates/ruff/src/checkers/ast/mod.rs
View File

@ -3060,7 +3060,7 @@ where
}
Expr::Set(ast::ExprSet { elts, range: _ }) => {
if self.enabled(Rule::DuplicateValue) {
pylint::rules::duplicate_value(self, elts);
flake8_bugbear::rules::duplicate_value(self, elts);
}
}
Expr::Yield(_) => {

2
crates/ruff/src/codes.rs
View File

@ -197,7 +197,6 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R5501") => (RuleGroup::Unspecified, rules::pylint::rules::CollapsibleElseIf),
(Pylint, "W0120") => (RuleGroup::Unspecified, rules::pylint::rules::UselessElseOnLoop),
(Pylint, "W0129") => (RuleGroup::Unspecified, rules::pylint::rules::AssertOnStringLiteral),
(Pylint, "W0130") => (RuleGroup::Unspecified, rules::pylint::rules::DuplicateValue),
(Pylint, "W0131") => (RuleGroup::Unspecified, rules::pylint::rules::NamedExprWithoutContext),
(Pylint, "W0406") => (RuleGroup::Unspecified, rules::pylint::rules::ImportSelf),
(Pylint, "W0602") => (RuleGroup::Unspecified, rules::pylint::rules::GlobalVariableNotAssigned),
@ -249,6 +248,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bugbear, "030") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::ExceptWithNonExceptionClasses),
(Flake8Bugbear, "031") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::ReuseOfGroupbyGenerator),
(Flake8Bugbear, "032") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::UnintentionalTypeAnnotation),
(Flake8Bugbear, "033") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::DuplicateValue),
(Flake8Bugbear, "904") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
(Flake8Bugbear, "905") => (RuleGroup::Unspecified, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),

1
crates/ruff/src/rule_redirects.rs
View File

@ -93,5 +93,6 @@ static REDIRECTS: Lazy<HashMap<&'static str, &'static str>> = Lazy::new(|| {
// TODO(charlie): Remove by 2023-06-01.
("RUF004", "B026"),
("PIE802", "C419"),
("PLW0130", "B033"),
])
});

53
crates/ruff/src/rules/flake8_bugbear/mod.rs
View File

@ -14,39 +14,40 @@ mod tests {
use crate::settings::Settings;
use crate::test::test_path;
#[test_case(Rule::UnaryPrefixIncrement, Path::new("B002.py"))]
#[test_case(Rule::AssignmentToOsEnviron, Path::new("B003.py"))]
#[test_case(Rule::UnreliableCallableCheck, Path::new("B004.py"))]
#[test_case(Rule::StripWithMultiCharacters, Path::new("B005.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
#[test_case(Rule::UnusedLoopControlVariable, Path::new("B007.py"))]
#[test_case(Rule::FunctionCallInDefaultArgument, Path::new("B006_B008.py"))]
#[test_case(Rule::GetAttrWithConstant, Path::new("B009_B010.py"))]
#[test_case(Rule::SetAttrWithConstant, Path::new("B009_B010.py"))]
#[test_case(Rule::AssertFalse, Path::new("B011.py"))]
#[test_case(Rule::JumpStatementInFinally, Path::new("B012.py"))]
#[test_case(Rule::RedundantTupleInExceptionHandler, Path::new("B013.py"))]
#[test_case(Rule::DuplicateHandlerException, Path::new("B014.py"))]
#[test_case(Rule::UselessComparison, Path::new("B015.py"))]
#[test_case(Rule::CannotRaiseLiteral, Path::new("B016.py"))]
#[test_case(Rule::AssertRaisesException, Path::new("B017.py"))]
#[test_case(Rule::UselessExpression, Path::new("B018.py"))]
#[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))]
#[test_case(Rule::LoopVariableOverridesIterator, Path::new("B020.py"))]
#[test_case(Rule::FStringDocstring, Path::new("B021.py"))]
#[test_case(Rule::UselessContextlibSuppress, Path::new("B022.py"))]
#[test_case(Rule::FunctionUsesLoopVariable, Path::new("B023.py"))]
#[test_case(Rule::AbstractBaseClassWithoutAbstractMethod, Path::new("B024.py"))]
#[test_case(Rule::AssertFalse, Path::new("B011.py"))]
#[test_case(Rule::AssertRaisesException, Path::new("B017.py"))]
#[test_case(Rule::AssignmentToOsEnviron, Path::new("B003.py"))]
#[test_case(Rule::CachedInstanceMethod, Path::new("B019.py"))]
#[test_case(Rule::CannotRaiseLiteral, Path::new("B016.py"))]
#[test_case(Rule::DuplicateHandlerException, Path::new("B014.py"))]
#[test_case(Rule::DuplicateTryBlockException, Path::new("B025.py"))]
#[test_case(Rule::StarArgUnpackingAfterKeywordArg, Path::new("B026.py"))]
#[test_case(Rule::DuplicateValue, Path::new("B033.py"))]
#[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.py"))]
#[test_case(Rule::EmptyMethodWithoutAbstractDecorator, Path::new("B027.pyi"))]
#[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
#[test_case(Rule::ExceptWithEmptyTuple, Path::new("B029.py"))]
#[test_case(Rule::ExceptWithNonExceptionClasses, Path::new("B030.py"))]
#[test_case(Rule::ReuseOfGroupbyGenerator, Path::new("B031.py"))]
#[test_case(Rule::UnintentionalTypeAnnotation, Path::new("B032.py"))]
#[test_case(Rule::FStringDocstring, Path::new("B021.py"))]
#[test_case(Rule::FunctionCallInDefaultArgument, Path::new("B006_B008.py"))]
#[test_case(Rule::FunctionUsesLoopVariable, Path::new("B023.py"))]
#[test_case(Rule::GetAttrWithConstant, Path::new("B009_B010.py"))]
#[test_case(Rule::JumpStatementInFinally, Path::new("B012.py"))]
#[test_case(Rule::LoopVariableOverridesIterator, Path::new("B020.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
#[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
#[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"))]
#[test_case(Rule::RedundantTupleInExceptionHandler, Path::new("B013.py"))]
#[test_case(Rule::ReuseOfGroupbyGenerator, Path::new("B031.py"))]
#[test_case(Rule::SetAttrWithConstant, Path::new("B009_B010.py"))]
#[test_case(Rule::StarArgUnpackingAfterKeywordArg, Path::new("B026.py"))]
#[test_case(Rule::StripWithMultiCharacters, Path::new("B005.py"))]
#[test_case(Rule::UnaryPrefixIncrement, Path::new("B002.py"))]
#[test_case(Rule::UnintentionalTypeAnnotation, Path::new("B032.py"))]
#[test_case(Rule::UnreliableCallableCheck, Path::new("B004.py"))]
#[test_case(Rule::UnusedLoopControlVariable, Path::new("B007.py"))]
#[test_case(Rule::UselessComparison, Path::new("B015.py"))]
#[test_case(Rule::UselessContextlibSuppress, Path::new("B022.py"))]
#[test_case(Rule::UselessExpression, Path::new("B018.py"))]
#[test_case(Rule::ZipWithoutExplicitStrict, Path::new("B905.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());

57
crates/ruff/src/rules/flake8_bugbear/rules/duplicate_value.rs Normal file
View File

@ -0,0 +1,57 @@
use rustc_hash::FxHashSet;
use rustpython_parser::ast::{self, Expr, Ranged};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::comparable::ComparableExpr;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for set literals that contain duplicate items.
///
/// ## Why is this bad?
/// In Python, sets are unordered collections of unique elements. Including a
/// duplicate item in a set literal is redundant, as the duplicate item will be
/// replaced with a single item at runtime.
///
/// ## Example
/// ```python
/// {1, 2, 3, 1}
/// ```
///
/// Use instead:
/// ```python
/// {1, 2, 3}
/// ```
#[violation]
pub struct DuplicateValue {
value: String,
}
impl Violation for DuplicateValue {
#[derive_message_formats]
fn message(&self) -> String {
let DuplicateValue { value } = self;
format!("Sets should not contain duplicate item `{value}`")
}
}
/// B033
pub(crate) fn duplicate_value(checker: &mut Checker, elts: &Vec<Expr>) {
let mut seen_values: FxHashSet<ComparableExpr> = FxHashSet::default();
for elt in elts {
if let Expr::Constant(ast::ExprConstant { value, .. }) = elt {
let comparable_value: ComparableExpr = elt.into();
if !seen_values.insert(comparable_value) {
checker.diagnostics.push(Diagnostic::new(
DuplicateValue {
value: checker.generator().constant(value),
},
elt.range(),
));
}
};
}
}

2
crates/ruff/src/rules/flake8_bugbear/rules/mod.rs
View File

@ -10,6 +10,7 @@ pub(crate) use cannot_raise_literal::{cannot_raise_literal, CannotRaiseLiteral};
pub(crate) use duplicate_exceptions::{
duplicate_exceptions, DuplicateHandlerException, DuplicateTryBlockException,
};
pub(crate) use duplicate_value::{duplicate_value, DuplicateValue};
pub(crate) use except_with_empty_tuple::{except_with_empty_tuple, ExceptWithEmptyTuple};
pub(crate) use except_with_non_exception_classes::{
except_with_non_exception_classes, ExceptWithNonExceptionClasses,
@ -66,6 +67,7 @@ mod assignment_to_os_environ;
mod cached_instance_method;
mod cannot_raise_literal;
mod duplicate_exceptions;
mod duplicate_value;
mod except_with_empty_tuple;
mod except_with_non_exception_classes;
mod f_string_docstring;

23
crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B033_B033.py.snap Normal file
View File

@ -0,0 +1,23 @@
---
source: crates/ruff/src/rules/flake8_bugbear/mod.rs
---
B033.py:4:35: B033 Sets should not contain duplicate item `"value1"`
|
4 | # Errors.
5 | ###
6 | incorrect_set = {"value1", 23, 5, "value1"}
| ^^^^^^^^ B033
7 | incorrect_set = {1, 1}
|
B033.py:5:21: B033 Sets should not contain duplicate item `1`
|
5 | ###
6 | incorrect_set = {"value1", 23, 5, "value1"}
7 | incorrect_set = {1, 1}
| ^ B033
8 |
9 | ###
|

1
crates/ruff/src/rules/pylint/mod.rs
View File

@ -54,7 +54,6 @@ mod tests {
#[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))]
#[test_case(Rule::InvalidStrReturnType, Path::new("invalid_return_type_str.py"))]
#[test_case(Rule::DuplicateBases, Path::new("duplicate_bases.py"))]
#[test_case(Rule::DuplicateValue, Path::new("duplicate_value.py"))]
#[test_case(Rule::InvalidCharacterBackspace, Path::new("invalid_characters.py"))]
#[test_case(Rule::InvalidCharacterEsc, Path::new("invalid_characters.py"))]
#[test_case(Rule::InvalidCharacterNul, Path::new("invalid_characters.py"))]

2
crates/ruff/src/rules/pylint/rules/mod.rs
View File

@ -9,7 +9,6 @@ pub(crate) use compare_to_empty_string::{compare_to_empty_string, CompareToEmpty
pub(crate) use comparison_of_constant::{comparison_of_constant, ComparisonOfConstant};
pub(crate) use continue_in_finally::{continue_in_finally, ContinueInFinally};
pub(crate) use duplicate_bases::{duplicate_bases, DuplicateBases};
pub(crate) use duplicate_value::{duplicate_value, DuplicateValue};
pub(crate) use global_statement::{global_statement, GlobalStatement};
pub(crate) use global_variable_not_assigned::GlobalVariableNotAssigned;
pub(crate) use import_self::{import_from_self, import_self, ImportSelf};
@ -66,7 +65,6 @@ mod compare_to_empty_string;
mod comparison_of_constant;
mod continue_in_finally;
mod duplicate_bases;
mod duplicate_value;
mod global_statement;
mod global_variable_not_assigned;
mod import_self;

23
crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW0130_duplicate_value.py.snap
View File

@ -1,23 +0,0 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
---
duplicate_value.py:4:35: PLW0130 Duplicate value `"value1"` in set
|
4 | # Errors.
5 | ###
6 | incorrect_set = {"value1", 23, 5, "value1"}
| ^^^^^^^^ PLW0130
7 | incorrect_set = {1, 1}
|
duplicate_value.py:5:21: PLW0130 Duplicate value `1` in set
|
5 | ###
6 | incorrect_set = {"value1", 23, 5, "value1"}
7 | incorrect_set = {1, 1}
| ^ PLW0130
8 |
9 | ###
|

2
ruff.schema.json generated
View File

@ -1639,6 +1639,7 @@
"B030",
"B031",
"B032",
"B033",
"B9",
"B90",
"B904",
@ -2137,7 +2138,6 @@
"PLW0120",
"PLW0129",
"PLW013",
"PLW0130",
"PLW0131",
"PLW04",
"PLW040",