Implement unnecessary-literal-within-dict-call (C418) (#3969)

crates/ruff/resources/test/fixtures/flake8_comprehensions/C418.py

@@ -0,0 +1,10 @@
+dict({})
+dict({'a': 1})
+dict({'x': 1 for x in range(10)})
+dict(
+    {'x': 1 for x in range(10)}
+)
+
+dict({}, a=1)
+dict({x: 1 for x in range(1)}, a=1)
+

crates/ruff/src/checkers/ast/mod.rs

@@ -2732,7 +2732,7 @@ where
                     .enabled(Rule::UnnecessaryLiteralWithinTupleCall)
                 {
                     flake8_comprehensions::rules::unnecessary_literal_within_tuple_call(
-                        self, expr, func, args,
+                        self, expr, func, args, keywords,
                     );
                 }
                 if self
@@ -2741,7 +2741,16 @@ where
                     .enabled(Rule::UnnecessaryLiteralWithinListCall)
                 {
                     flake8_comprehensions::rules::unnecessary_literal_within_list_call(
-                        self, expr, func, args,
+                        self, expr, func, args, keywords,
+                    );
+                }
+                if self
+                    .settings
+                    .rules
+                    .enabled(Rule::UnnecessaryLiteralWithinDictCall)
+                {
+                    flake8_comprehensions::rules::unnecessary_literal_within_dict_call(
+                        self, expr, func, args, keywords,
                     );
                 }
                 if self.settings.rules.enabled(Rule::UnnecessaryListCall) {

crates/ruff/src/codes.rs

@@ -263,6 +263,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Flake8Comprehensions, "15") => Rule::UnnecessarySubscriptReversal,
         (Flake8Comprehensions, "16") => Rule::UnnecessaryComprehension,
         (Flake8Comprehensions, "17") => Rule::UnnecessaryMap,
+        (Flake8Comprehensions, "18") => Rule::UnnecessaryLiteralWithinDictCall,
 
         // flake8-debugger
         (Flake8Debugger, "0") => Rule::Debugger,

crates/ruff/src/registry.rs

@@ -244,6 +244,7 @@ ruff_macros::register_rules!(
     rules::flake8_comprehensions::rules::UnnecessarySubscriptReversal,
     rules::flake8_comprehensions::rules::UnnecessaryComprehension,
     rules::flake8_comprehensions::rules::UnnecessaryMap,
+    rules::flake8_comprehensions::rules::UnnecessaryLiteralWithinDictCall,
     // flake8-debugger
     rules::flake8_debugger::rules::Debugger,
     // mccabe

crates/ruff/src/rules/flake8_comprehensions/fixes.rs

@@ -1173,3 +1173,31 @@ pub fn fix_unnecessary_map(
         bail!("Should have two arguments");
     }
 }
+
+/// (C418) Convert `dict({"a": 1})` to `{"a": 1}`
+pub fn fix_unnecessary_literal_within_dict_call(
+    locator: &Locator,
+    stylist: &Stylist,
+    expr: &rustpython_parser::ast::Expr,
+) -> Result<Edit> {
+    let module_text = locator.slice(expr);
+    let mut tree = match_module(module_text)?;
+    let mut body = match_expr(&mut tree)?;
+    let call = match_call(body)?;
+    let arg = match_arg(call)?;
+
+    body.value = arg.value.clone();
+
+    let mut state = CodegenState {
+        default_newline: &stylist.line_ending(),
+        default_indent: stylist.indentation(),
+        ..CodegenState::default()
+    };
+    tree.codegen(&mut state);
+
+    Ok(Edit::replacement(
+        state.to_string(),
+        expr.location,
+        expr.end_location.unwrap(),
+    ))
+}

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

@@ -32,6 +32,7 @@ mod tests {
     #[test_case(Rule::UnnecessarySubscriptReversal, Path::new("C415.py"); "C415")]
     #[test_case(Rule::UnnecessaryComprehension, Path::new("C416.py"); "C416")]
     #[test_case(Rule::UnnecessaryMap, Path::new("C417.py"); "C417")]
+    #[test_case(Rule::UnnecessaryLiteralWithinDictCall, Path::new("C418.py"); "C418")]
     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/src/rules/flake8_comprehensions/rules/mod.rs

@@ -20,6 +20,9 @@ pub use unnecessary_list_comprehension_set::{
 };
 pub use unnecessary_literal_dict::{unnecessary_literal_dict, UnnecessaryLiteralDict};
 pub use unnecessary_literal_set::{unnecessary_literal_set, UnnecessaryLiteralSet};
+pub use unnecessary_literal_within_dict_call::{
+    unnecessary_literal_within_dict_call, UnnecessaryLiteralWithinDictCall,
+};
 pub use unnecessary_literal_within_list_call::{
     unnecessary_literal_within_list_call, UnnecessaryLiteralWithinListCall,
 };
@@ -44,6 +47,7 @@ mod unnecessary_list_comprehension_dict;
 mod unnecessary_list_comprehension_set;
 mod unnecessary_literal_dict;
 mod unnecessary_literal_set;
+mod unnecessary_literal_within_dict_call;
 mod unnecessary_literal_within_list_call;
 mod unnecessary_literal_within_tuple_call;
 mod unnecessary_map;

crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs

@@ -0,0 +1,99 @@
+use rustpython_parser::ast::{Expr, ExprKind, Keyword};
+use std::fmt;
+
+use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::types::Range;
+
+use crate::checkers::ast::Checker;
+use crate::registry::AsRule;
+use crate::rules::flake8_comprehensions::fixes;
+
+use super::helpers;
+
+#[derive(Debug, PartialEq, Eq)]
+pub enum DictKind {
+    Literal,
+    Comprehension,
+}
+
+impl fmt::Display for DictKind {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            DictKind::Literal => fmt.write_str("literal"),
+            DictKind::Comprehension => fmt.write_str("comprehension"),
+        }
+    }
+}
+
+/// ## What it does
+/// Checks for `dict` calls that take unnecessary `dict` literals or `dict`
+/// comprehensions as arguments.
+///
+/// ## Why is it bad?
+/// It's unnecessary to wrap a `dict` literal or comprehension within a `dict`
+/// call, since the literal or comprehension syntax already returns a `dict`.
+///
+/// ## Examples
+/// ```python
+/// dict({})
+/// dict({"a": 1})
+/// ```
+///
+/// Use instead:
+/// ```python
+/// {}
+/// {"a": 1}
+/// ```
+#[violation]
+pub struct UnnecessaryLiteralWithinDictCall {
+    pub kind: DictKind,
+}
+
+impl AlwaysAutofixableViolation for UnnecessaryLiteralWithinDictCall {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let UnnecessaryLiteralWithinDictCall { kind } = self;
+        format!("Unnecessary `dict` {kind} passed to `dict()` (remove the outer call to `dict()`)")
+    }
+
+    fn autofix_title(&self) -> String {
+        "Remove outer `dict` call".to_string()
+    }
+}
+
+/// C418
+pub fn unnecessary_literal_within_dict_call(
+    checker: &mut Checker,
+    expr: &Expr,
+    func: &Expr,
+    args: &[Expr],
+    keywords: &[Keyword],
+) {
+    if !keywords.is_empty() {
+        return;
+    }
+    let Some(argument) = helpers::first_argument_with_matching_function("dict", func, args) else {
+        return;
+    };
+    if !checker.ctx.is_builtin("dict") {
+        return;
+    }
+    let argument_kind = match argument {
+        ExprKind::DictComp { .. } => DictKind::Comprehension,
+        ExprKind::Dict { .. } => DictKind::Literal,
+        _ => return,
+    };
+    let mut diagnostic = Diagnostic::new(
+        UnnecessaryLiteralWithinDictCall {
+            kind: argument_kind,
+        },
+        Range::from(expr),
+    );
+    if checker.patch(diagnostic.kind.rule()) {
+        diagnostic.try_set_fix(|| {
+            fixes::fix_unnecessary_literal_within_dict_call(checker.locator, checker.stylist, expr)
+        });
+    }
+    checker.diagnostics.push(diagnostic);
+}

crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_list_call.rs

@@ -1,4 +1,4 @@
-use rustpython_parser::ast::{Expr, ExprKind};
+use rustpython_parser::ast::{Expr, ExprKind, Keyword};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
 use ruff_macros::{derive_message_formats, violation};
@@ -72,7 +72,11 @@ pub fn unnecessary_literal_within_list_call(
     expr: &Expr,
     func: &Expr,
     args: &[Expr],
+    keywords: &[Keyword],
 ) {
+    if !keywords.is_empty() {
+        return;
+    }
     let Some(argument) = helpers::first_argument_with_matching_function("list", func, args) else {
         return;
     };

crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs

@@ -1,4 +1,4 @@
-use rustpython_parser::ast::{Expr, ExprKind};
+use rustpython_parser::ast::{Expr, ExprKind, Keyword};
 
 use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic};
 use ruff_macros::{derive_message_formats, violation};
@@ -73,7 +73,11 @@ pub fn unnecessary_literal_within_tuple_call(
     expr: &Expr,
     func: &Expr,
     args: &[Expr],
+    keywords: &[Keyword],
 ) {
+    if !keywords.is_empty() {
+        return;
+    }
     let Some(argument) = helpers::first_argument_with_matching_function("tuple", func, args) else {
         return;
     };

crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C418_C418.py.snap

@@ -0,0 +1,83 @@
+---
+source: crates/ruff/src/rules/flake8_comprehensions/mod.rs
+---
+C418.py:1:1: C418 [*] Unnecessary `dict` literal passed to `dict()` (remove the outer call to `dict()`)
+  |
+1 | dict({})
+  | ^^^^^^^^ C418
+2 | dict({'a': 1})
+3 | dict({'x': 1 for x in range(10)})
+  |
+  = help: Remove outer `dict` call
+
+ℹ Suggested fix
+1   |-dict({})
+  1 |+{}
+2 2 | dict({'a': 1})
+3 3 | dict({'x': 1 for x in range(10)})
+4 4 | dict(
+
+C418.py:2:1: C418 [*] Unnecessary `dict` literal passed to `dict()` (remove the outer call to `dict()`)
+  |
+2 | dict({})
+3 | dict({'a': 1})
+  | ^^^^^^^^^^^^^^ C418
+4 | dict({'x': 1 for x in range(10)})
+5 | dict(
+  |
+  = help: Remove outer `dict` call
+
+ℹ Suggested fix
+1 1 | dict({})
+2   |-dict({'a': 1})
+  2 |+{'a': 1}
+3 3 | dict({'x': 1 for x in range(10)})
+4 4 | dict(
+5 5 |     {'x': 1 for x in range(10)}
+
+C418.py:3:1: C418 [*] Unnecessary `dict` comprehension passed to `dict()` (remove the outer call to `dict()`)
+  |
+3 | dict({})
+4 | dict({'a': 1})
+5 | dict({'x': 1 for x in range(10)})
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C418
+6 | dict(
+7 |     {'x': 1 for x in range(10)}
+  |
+  = help: Remove outer `dict` call
+
+ℹ Suggested fix
+1 1 | dict({})
+2 2 | dict({'a': 1})
+3   |-dict({'x': 1 for x in range(10)})
+  3 |+{'x': 1 for x in range(10)}
+4 4 | dict(
+5 5 |     {'x': 1 for x in range(10)}
+6 6 | )
+
+C418.py:4:1: C418 [*] Unnecessary `dict` comprehension passed to `dict()` (remove the outer call to `dict()`)
+   |
+ 4 |   dict({'a': 1})
+ 5 |   dict({'x': 1 for x in range(10)})
+ 6 | / dict(
+ 7 | |     {'x': 1 for x in range(10)}
+ 8 | | )
+   | |_^ C418
+ 9 |   
+10 |   dict({}, a=1)
+   |
+   = help: Remove outer `dict` call
+
+ℹ Suggested fix
+1 1 | dict({})
+2 2 | dict({'a': 1})
+3 3 | dict({'x': 1 for x in range(10)})
+4   |-dict(
+5   |-    {'x': 1 for x in range(10)}
+6   |-)
+  4 |+{'x': 1 for x in range(10)}
+7 5 | 
+8 6 | dict({}, a=1)
+9 7 | dict({x: 1 for x in range(1)}, a=1)
+
+