Skip to content

Commit

Permalink
Redirect RUF011 to B035
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 11, 2024
1 parent 4a3bb67 commit 9558c11
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 25 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1424,7 +1424,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
if checker.enabled(Rule::StaticKeyDictComprehension) {
ruff::rules::static_key_dict_comprehension(checker, dict_comp);
flake8_bugbear::rules::static_key_dict_comprehension(checker, dict_comp);
}
}
Expr::GeneratorExp(
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bugbear, "032") => (RuleGroup::Stable, rules::flake8_bugbear::rules::UnintentionalTypeAnnotation),
(Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
(Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
(Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension),
(Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
(Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),

Expand Down Expand Up @@ -910,7 +911,6 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "008") => (RuleGroup::Stable, rules::ruff::rules::MutableDataclassDefault),
(Ruff, "009") => (RuleGroup::Stable, rules::ruff::rules::FunctionCallInDataclassDefaultArgument),
(Ruff, "010") => (RuleGroup::Stable, rules::ruff::rules::ExplicitFStringTypeConversion),
(Ruff, "011") => (RuleGroup::Stable, rules::ruff::rules::StaticKeyDictComprehension),
(Ruff, "012") => (RuleGroup::Stable, rules::ruff::rules::MutableClassDefault),
(Ruff, "013") => (RuleGroup::Stable, rules::ruff::rules::ImplicitOptional),
#[cfg(feature = "unreachable-code")] // When removing this feature gate, also update rules_selector.rs
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rule_redirects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,6 @@ static REDIRECTS: Lazy<HashMap<&'static str, &'static str>> = Lazy::new(|| {
("T002", "FIX002"),
("T003", "FIX003"),
("T004", "FIX004"),
("RUF011", "B035"),
])
});
7 changes: 4 additions & 3 deletions crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ mod tests {
#[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::MutableArgumentDefault, Path::new("B006_1.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_2.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_3.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_4.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_5.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_6.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_7.py"))]
#[test_case(Rule::MutableArgumentDefault, Path::new("B006_B008.py"))]
#[test_case(Rule::NoExplicitStacklevel, Path::new("B028.py"))]
#[test_case(Rule::RaiseLiteral, Path::new("B016.py"))]
#[test_case(Rule::RaiseWithoutFromInsideExcept, Path::new("B904.py"))]
Expand All @@ -49,16 +49,17 @@ mod tests {
#[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::StaticKeyDictComprehension, Path::new("B035.py"))]
#[test_case(Rule::StripWithMultiCharacters, Path::new("B005.py"))]
#[test_case(Rule::UnaryPrefixIncrementDecrement, 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::UselessComparison, Path::new("B015.ipynb"))]
#[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::UselessExpression, Path::new("B018.ipynb"))]
#[test_case(Rule::UselessExpression, Path::new("B018.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub(crate) use redundant_tuple_in_exception_handler::*;
pub(crate) use reuse_of_groupby_generator::*;
pub(crate) use setattr_with_constant::*;
pub(crate) use star_arg_unpacking_after_keyword_arg::*;
pub(crate) use static_key_dict_comprehension::*;
pub(crate) use strip_with_multi_characters::*;
pub(crate) use unary_prefix_increment_decrement::*;
pub(crate) use unintentional_type_annotation::*;
Expand Down Expand Up @@ -56,6 +57,7 @@ mod redundant_tuple_in_exception_handler;
mod reuse_of_groupby_generator;
mod setattr_with_constant;
mod star_arg_unpacking_after_keyword_arg;
mod static_key_dict_comprehension;
mod strip_with_multi_characters;
mod unary_prefix_increment_decrement;
mod unintentional_type_annotation;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
use rustc_hash::FxHashMap;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::NameFinder;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast, Expr};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::fix::snippet::SourceCodeSnippet;

/// ## What it does
/// Checks for dictionary comprehensions that use a static key, like a string
/// literal or a variable defined outside the comprehension.
///
/// ## Why is this bad?
/// Using a static key (like a string literal) in a dictionary comprehension
/// is usually a mistake, as it will result in a dictionary with only one key,
/// despite the comprehension iterating over multiple values.
///
/// ## Example
/// ```python
/// data = ["some", "Data"]
/// {"key": value.upper() for value in data}
/// ```
///
/// Use instead:
/// ```python
/// data = ["some", "Data"]
/// {value: value.upper() for value in data}
/// ```
#[violation]
pub struct StaticKeyDictComprehension {
key: SourceCodeSnippet,
}

impl Violation for StaticKeyDictComprehension {
#[derive_message_formats]
fn message(&self) -> String {
let StaticKeyDictComprehension { key } = self;
if let Some(key) = key.full_display() {
format!("Dictionary comprehension uses static key: `{key}`")
} else {
format!("Dictionary comprehension uses static key")
}
}
}

/// RUF011
pub(crate) fn static_key_dict_comprehension(checker: &mut Checker, dict_comp: &ast::ExprDictComp) {
// Collect the bound names in the comprehension's generators.
let names = {
let mut visitor = NameFinder::default();
for generator in &dict_comp.generators {
visitor.visit_expr(&generator.target);
}
visitor.names
};

if is_constant(&dict_comp.key, &names) {
checker.diagnostics.push(Diagnostic::new(
StaticKeyDictComprehension {
key: SourceCodeSnippet::from_str(checker.locator().slice(dict_comp.key.as_ref())),
},
dict_comp.key.range(),
));
}
}

/// Returns `true` if the given expression is a constant in the context of the dictionary
/// comprehension.
fn is_constant(key: &Expr, names: &FxHashMap<&str, &ast::ExprName>) -> bool {
match key {
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(|elt| is_constant(elt, names)),
Expr::Name(ast::ExprName { id, .. }) => !names.contains_key(id.as_str()),
Expr::Attribute(ast::ExprAttribute { value, .. }) => is_constant(value, names),
Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => {
is_constant(value, names) && is_constant(slice, names)
}
Expr::BinOp(ast::ExprBinOp { left, right, .. }) => {
is_constant(left, names) && is_constant(right, names)
}
Expr::BoolOp(ast::ExprBoolOp { values, .. }) => {
values.iter().all(|value| is_constant(value, names))
}
Expr::UnaryOp(ast::ExprUnaryOp { operand, .. }) => is_constant(operand, names),
expr if expr.is_literal_expr() => true,
_ => false,
}
}
Original file line number Diff line number Diff line change
@@ -1,80 +1,80 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
RUF011.py:15:2: RUF011 Dictionary comprehension uses static key: `"key"`
B035.py:15:2: B035 Dictionary comprehension uses static key: `"key"`
|
14 | # Errors
15 | {"key": value.upper() for value in data}
| ^^^^^ RUF011
| ^^^^^ B035
16 | {True: value.upper() for value in data}
17 | {0: value.upper() for value in data}
|

RUF011.py:16:2: RUF011 Dictionary comprehension uses static key: `True`
B035.py:16:2: B035 Dictionary comprehension uses static key: `True`
|
14 | # Errors
15 | {"key": value.upper() for value in data}
16 | {True: value.upper() for value in data}
| ^^^^ RUF011
| ^^^^ B035
17 | {0: value.upper() for value in data}
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
|

RUF011.py:17:2: RUF011 Dictionary comprehension uses static key: `0`
B035.py:17:2: B035 Dictionary comprehension uses static key: `0`
|
15 | {"key": value.upper() for value in data}
16 | {True: value.upper() for value in data}
17 | {0: value.upper() for value in data}
| ^ RUF011
| ^ B035
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {constant: value.upper() for value in data}
|

RUF011.py:18:2: RUF011 Dictionary comprehension uses static key: `(1, "a")`
B035.py:18:2: B035 Dictionary comprehension uses static key: `(1, "a")`
|
16 | {True: value.upper() for value in data}
17 | {0: value.upper() for value in data}
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
| ^^^^^^^^ RUF011
| ^^^^^^^^ B035
19 | {constant: value.upper() for value in data}
20 | {constant + constant: value.upper() for value in data}
|

RUF011.py:19:2: RUF011 Dictionary comprehension uses static key: `constant`
B035.py:19:2: B035 Dictionary comprehension uses static key: `constant`
|
17 | {0: value.upper() for value in data}
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {constant: value.upper() for value in data}
| ^^^^^^^^ RUF011
| ^^^^^^^^ B035
20 | {constant + constant: value.upper() for value in data}
21 | {constant.attribute: value.upper() for value in data}
|

RUF011.py:20:2: RUF011 Dictionary comprehension uses static key: `constant + constant`
B035.py:20:2: B035 Dictionary comprehension uses static key: `constant + constant`
|
18 | {(1, "a"): value.upper() for value in data} # Constant tuple
19 | {constant: value.upper() for value in data}
20 | {constant + constant: value.upper() for value in data}
| ^^^^^^^^^^^^^^^^^^^ RUF011
| ^^^^^^^^^^^^^^^^^^^ B035
21 | {constant.attribute: value.upper() for value in data}
22 | {constant[0]: value.upper() for value in data}
|

RUF011.py:21:2: RUF011 Dictionary comprehension uses static key: `constant.attribute`
B035.py:21:2: B035 Dictionary comprehension uses static key: `constant.attribute`
|
19 | {constant: value.upper() for value in data}
20 | {constant + constant: value.upper() for value in data}
21 | {constant.attribute: value.upper() for value in data}
| ^^^^^^^^^^^^^^^^^^ RUF011
| ^^^^^^^^^^^^^^^^^^ B035
22 | {constant[0]: value.upper() for value in data}
|

RUF011.py:22:2: RUF011 Dictionary comprehension uses static key: `constant[0]`
B035.py:22:2: B035 Dictionary comprehension uses static key: `constant[0]`
|
20 | {constant + constant: value.upper() for value in data}
21 | {constant.attribute: value.upper() for value in data}
22 | {constant[0]: value.upper() for value in data}
| ^^^^^^^^^^^ RUF011
| ^^^^^^^^^^^ B035
|


1 change: 0 additions & 1 deletion crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ mod tests {
#[test_case(Rule::MutableClassDefault, Path::new("RUF012.py"))]
#[test_case(Rule::MutableDataclassDefault, Path::new("RUF008.py"))]
#[test_case(Rule::PairwiseOverZipped, Path::new("RUF007.py"))]
#[test_case(Rule::StaticKeyDictComprehension, Path::new("RUF011.py"))]
#[test_case(
Rule::UnnecessaryIterableAllocationForFirstElement,
Path::new("RUF015.py")
Expand Down
1 change: 0 additions & 1 deletion crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ pub(crate) use never_union::*;
pub(crate) use pairwise_over_zipped::*;
pub(crate) use parenthesize_logical_operators::*;
pub(crate) use quadratic_list_summation::*;
pub(crate) use static_key_dict_comprehension::*;
pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*;
#[cfg(feature = "unreachable-code")]
Expand Down
2 changes: 1 addition & 1 deletion ruff.schema.json

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

0 comments on commit 9558c11

Please sign in to comment.