Skip to content

Commit

Permalink
Implement unnecessary-literal-within-dict-call (C418)
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 13, 2023
1 parent d8718dc commit 2eaa31f
Show file tree
Hide file tree
Showing 11 changed files with 248 additions and 4 deletions.
10 changes: 10 additions & 0 deletions crates/ruff/resources/test/fixtures/flake8_comprehensions/C418.py
Original file line number Diff line number Diff line change
@@ -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)

13 changes: 11 additions & 2 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions crates/ruff/src/rules/flake8_comprehensions/fixes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
))
}
1 change: 1 addition & 0 deletions crates/ruff/src/rules/flake8_comprehensions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff/src/rules/flake8_comprehensions/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
};
Expand Down
Original file line number Diff line number Diff line change
@@ -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)


0 comments on commit 2eaa31f

Please sign in to comment.