diff --git a/crates/ruff/resources/test/fixtures/flake8_comprehensions/C418.py b/crates/ruff/resources/test/fixtures/flake8_comprehensions/C418.py new file mode 100644 index 00000000000000..01e9be2c0d5975 --- /dev/null +++ b/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) + diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 3029ec57aa4350..4e1427087b9f52 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/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) { diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index ae67c325f2a039..9296a468eeebf3 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -263,6 +263,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option { (Flake8Comprehensions, "15") => Rule::UnnecessarySubscriptReversal, (Flake8Comprehensions, "16") => Rule::UnnecessaryComprehension, (Flake8Comprehensions, "17") => Rule::UnnecessaryMap, + (Flake8Comprehensions, "18") => Rule::UnnecessaryLiteralWithinDictCall, // flake8-debugger (Flake8Debugger, "0") => Rule::Debugger, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 290d9a869977a2..8132097632e982 100644 --- a/crates/ruff/src/registry.rs +++ b/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 diff --git a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs index 24381353b2db47..4cb0449fdbf535 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs +++ b/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 { + 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(), + )) +} diff --git a/crates/ruff/src/rules/flake8_comprehensions/mod.rs b/crates/ruff/src/rules/flake8_comprehensions/mod.rs index 0a90f35c5f3c57..a2463c2a7c0cc9 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/mod.rs +++ b/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( diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs index cc0f6834766dc0..54012fd85b7549 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs +++ b/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; diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_dict_call.rs new file mode 100644 index 00000000000000..be047d5d274a13 --- /dev/null +++ b/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); +} diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_list_call.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_list_call.rs index 7ccc30fb75beb8..9cb6e2601a4099 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_list_call.rs +++ b/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; }; diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs index 41aed3b54cf0c5..ff39c76eb183a1 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_within_tuple_call.rs +++ b/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; }; diff --git a/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C418_C418.py.snap b/crates/ruff/src/rules/flake8_comprehensions/snapshots/ruff__rules__flake8_comprehensions__tests__C418_C418.py.snap new file mode 100644 index 00000000000000..6e515a6f5fc717 --- /dev/null +++ b/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) + +