[ruff] Detect unnecessary dict comprehensions for iterables (RUF025)

crates/ruff_linter/resources/test/fixtures/ruff/RUF025.py

@@ -0,0 +1,69 @@
+# Violation cases: RUF025
+
+def func():
+    numbers = [1,2,3]
+    {n: None for n in numbers} # RUF025
+
+def func():
+    for key, value in {n: 1 for n in [1,2,3]}.items(): # RUF025
+        pass
+
+def func():
+    {n: 1.1 for n in [1,2,3]} # RUF025
+
+def func():
+    {n: complex(3, 5) for n in [1,2,3]} # RUF025
+
+def func():
+    def f(data):
+        return data
+    f({c: "a" for c in "12345"}) # RUF025
+
+def func():
+    {n: True for n in [1,2,2]} # RUF025
+
+def func():
+    {n: b'hello' for n in (1,2,2)} # RUF025
+
+def func():
+    {n: ... for n in [1,2,3]} # RUF025
+
+def func():
+    {n: False for n in {1: "a", 2: "b"}} # RUF025
+
+def func():
+    {(a, b): 1 for (a, b) in [(1,2), (3, 4)]} # RUF025
+
+
+# Non-violation cases: RUF025
+
+def func():
+    {n: 1 for n in [1,2,3,4,5] if n < 3} # OK
+
+def func():
+    {n: 1 for c in [1,2,3,4,5] for n in [1,2,3] if c < 3} # OK
+
+def func():
+    def f():
+        pass
+    {n: f() for n in [1,2,3]} # OK
+
+def func():
+    {n: n for n in [1,2,3,4,5]} # OK
+
+def func():
+    def f():
+        return {n: 1 for c in [1,2,3,4,5] for n in [1,2,3]} # OK
+
+    f()
+
+def func():
+    def f():
+        return 1
+
+    a = f()
+    {n: a for n in [1, 2, 3]}  # RUF025
+
+def func():
+    values = ["a", "b", "c"]
+    [{n: values for n in [1,2,3]}] # RUF025

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1422,11 +1422,17 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
             if checker.enabled(Rule::UnnecessaryDictIndexLookup) {
                 pylint::rules::unnecessary_dict_index_lookup_comprehension(checker, expr);
             }
+
             if checker.enabled(Rule::UnnecessaryComprehension) {
                 flake8_comprehensions::rules::unnecessary_dict_comprehension(
                     checker, expr, key, value, generators,
                 );
             }
+
+            if checker.enabled(Rule::UnnecessaryDictComprehensionForIterable) {
+                ruff::rules::unnecessary_dict_comprehension_for_iterable(checker, dict_comp);
+            }
+
             if checker.enabled(Rule::FunctionUsesLoopVariable) {
                 flake8_bugbear::rules::function_uses_loop_variable(checker, &Node::Expr(expr));
             }

crates/ruff_linter/src/codes.rs

@@ -928,6 +928,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Ruff, "022") => (RuleGroup::Preview, rules::ruff::rules::UnsortedDunderAll),
         (Ruff, "023") => (RuleGroup::Preview, rules::ruff::rules::UnsortedDunderSlots),
         (Ruff, "024") => (RuleGroup::Preview, rules::ruff::rules::MutableFromkeysValue),
+        (Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryDictComprehensionForIterable),
         (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
         (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
 

crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE001_1.py.snap

@@ -9,4 +9,3 @@ EXE001_1.py:1:1: EXE001 Shebang is present but file is not executable
 3 | if __name__ == '__main__':
   |
 
-

crates/ruff_linter/src/rules/flake8_executable/snapshots/ruff_linter__rules__flake8_executable__tests__EXE002_1.py.snap

@@ -7,5 +7,3 @@ EXE002_1.py:1:1: EXE002 The file is executable but no shebang is present
   |  EXE002
 2 |     print('I should be executable.')
   |
-
-

crates/ruff_linter/src/rules/ruff/mod.rs

@@ -45,6 +45,7 @@ mod tests {
     #[test_case(Rule::UnsortedDunderAll, Path::new("RUF022.py"))]
     #[test_case(Rule::UnsortedDunderSlots, Path::new("RUF023.py"))]
     #[test_case(Rule::MutableFromkeysValue, Path::new("RUF024.py"))]
+    #[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("RUF025.py"))]
     fn rules(rule_code: Rule, path: &Path) -> Result<()> {
         let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
         let diagnostics = test_path(

crates/ruff_linter/src/rules/ruff/rules/mod.rs

@@ -17,6 +17,7 @@ pub(crate) use quadratic_list_summation::*;
 pub(crate) use sort_dunder_all::*;
 pub(crate) use sort_dunder_slots::*;
 pub(crate) use static_key_dict_comprehension::*;
+pub(crate) use unnecessary_dict_comprehension_for_iterable::*;
 pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
 pub(crate) use unnecessary_key_check::*;
 pub(crate) use unused_noqa::*;
@@ -42,6 +43,7 @@ mod sequence_sorting;
 mod sort_dunder_all;
 mod sort_dunder_slots;
 mod static_key_dict_comprehension;
+mod unnecessary_dict_comprehension_for_iterable;
 mod unnecessary_iterable_allocation_for_first_element;
 mod unnecessary_key_check;
 mod unused_noqa;

crates/ruff_linter/src/rules/ruff/rules/unnecessary_dict_comprehension_for_iterable.rs

@@ -0,0 +1,128 @@
+use ast::ExprName;
+use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::comparable::ComparableExpr;
+use ruff_python_ast::helpers::is_constant;
+use ruff_python_ast::{self as ast, Arguments, Comprehension, Expr, ExprCall, ExprContext};
+use ruff_text_size::{Ranged, TextRange};
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for unnecessary `dict` comprehension when creating a dictionary from
+/// an iterable.
+///
+/// ## Why is this bad?
+/// It's unnecessary to use a `dict` comprehension to build a dictionary from
+/// an iterable when the value is static.
+///
+/// Prefer `dict.fromkeys(iterable)` over `{value: None for value in iterable}`,
+/// as `dict.fromkeys` is more readable and efficient.
+///
+/// ## Examples
+/// ```python
+/// {a: None for a in iterable}
+/// {a: 1 for a in iterable}
+/// ```
+///
+/// Use instead:
+/// ```python
+/// dict.fromkeys(iterable)
+/// dict.fromkeys(iterable, 1)
+/// ```
+#[violation]
+pub struct UnnecessaryDictComprehensionForIterable {
+    is_value_none_literal: bool,
+}
+
+impl Violation for UnnecessaryDictComprehensionForIterable {
+    const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;
+
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Unnecessary dict comprehension for iterable; use `dict.fromkeys` instead")
+    }
+
+    fn fix_title(&self) -> Option<String> {
+        if self.is_value_none_literal {
+            Some(format!("Replace with `dict.fromkeys(iterable, value)`)"))
+        } else {
+            Some(format!("Replace with `dict.fromkeys(iterable)`)"))
+        }
+    }
+}
+
+/// RUF025
+pub(crate) fn unnecessary_dict_comprehension_for_iterable(
+    checker: &mut Checker,
+    dict_comp: &ast::ExprDictComp,
+) {
+    let [generator] = dict_comp.generators.as_slice() else {
+        return;
+    };
+
+    // Don't suggest `dict.fromkeys` for:
+    // - async generator expressions, because `dict.fromkeys` is not async.
+    // - nested generator expressions, because `dict.fromkeys` might be error-prone option at least for fixing.
+    // - generator expressions with `if` clauses, because `dict.fromkeys` might not be valid option.
+    if !generator.ifs.is_empty() && generator.is_async {
+        return;
+    }
+
+    // Don't suggest `dict.keys` if the target is not the same as the key.
+    if ComparableExpr::from(&generator.target) != ComparableExpr::from(dict_comp.key.as_ref()) {
+        return;
+    }
+
+    if !is_constant(dict_comp.value.as_ref()) {
+        return;
+    }
+
+    let mut diagnostic = Diagnostic::new(
+        UnnecessaryDictComprehensionForIterable {
+            is_value_none_literal: dict_comp.value.is_none_literal_expr(),
+        },
+        dict_comp.range(),
+    );
+
+    if checker.semantic().is_builtin("dict") {
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
+            checker
+                .generator()
+                .expr(&fix_unnecessary_dict_comprehension(
+                    dict_comp.value.as_ref(),
+                    generator,
+                )),
+            dict_comp.range(),
+        )));
+    }
+
+    checker.diagnostics.push(diagnostic);
+}
+
+/// Generate a [`Fix`] to replace `dict` comprehension with `dict.fromkeys`.
+///
+/// For example:
+/// - Given `{n: None for n in [1,2,3]}`, generate `dict.fromkeys([1,2,3])`.
+/// - Given `{n: 1 for n in [1,2,3]}`, generate `dict.fromkeys([1,2,3], 1)`.
+fn fix_unnecessary_dict_comprehension(value: &Expr, generator: &Comprehension) -> Expr {
+    let iterable = generator.iter.clone();
+    let args = Arguments {
+        args: if value.is_none_literal_expr() {
+            vec![iterable]
+        } else {
+            vec![iterable, value.clone()]
+        },
+        keywords: vec![],
+        range: TextRange::default(),
+    };
+    Expr::Call(ExprCall {
+        func: Box::new(Expr::Name(ExprName {
+            id: "dict.fromkeys".into(),
+            ctx: ExprContext::Load,
+            range: TextRange::default(),
+        })),
+        arguments: args,
+        range: TextRange::default(),
+    })
+}