From b64040cbb2cede772d5966ec633019644215bdb7 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Sat, 15 Oct 2022 01:34:00 +0900 Subject: [PATCH] Implement C417 (#426) --- README.md | 3 +- resources/test/fixtures/C417.py | 6 ++ src/ast/checkers.rs | 58 ++++++++++++++ src/check_ast.rs | 6 ++ src/checks.rs | 11 +++ src/linter.rs | 1 + .../ruff__linter__tests__C417_C417.py.snap | 77 +++++++++++++++++++ 7 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 resources/test/fixtures/C417.py create mode 100644 src/snapshots/ruff__linter__tests__C417_C417.py.snap diff --git a/README.md b/README.md index e23409f4fb..11bed1e478 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ ruff also implements some of the most popular Flake8 plugins natively, including - [`flake8-builtins`](https://pypi.org/project/flake8-builtins/) - [`flake8-super`](https://pypi.org/project/flake8-super/) - [`flake8-print`](https://pypi.org/project/flake8-print/) -- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) (15/16) +- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) - [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32) - [`flake8-docstrings`](https://pypi.org/project/flake8-docstrings/) (41/48) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34) @@ -302,6 +302,7 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com | C414 | UnnecessaryDoubleCastOrProcess | Unnecessary call within (). | | | | C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within () | | | | C416 | UnnecessaryComprehension | Unnecessary comprehension - rewrite using () | | | +| C417 | UnnecessaryMap | Unnecessary map usage - rewrite using a comprehension | | | | T201 | PrintFound | `print` found | | 🛠 | | T203 | PPrintFound | `pprint` found | | 🛠 | | U001 | UselessMetaclassType | `__metaclass__ = type` is implied | | 🛠 | diff --git a/resources/test/fixtures/C417.py b/resources/test/fixtures/C417.py new file mode 100644 index 0000000000..50816e4cb9 --- /dev/null +++ b/resources/test/fixtures/C417.py @@ -0,0 +1,6 @@ +nums = [1, 2, 3] +map(lambda x: x + 1, nums) +map(lambda x: str(x), nums) +list(map(lambda x: x * 2, nums)) +set(map(lambda x: x % 2 == 0, nums)) +dict(map(lambda v: (v, v**2), nums)) diff --git a/src/ast/checkers.rs b/src/ast/checkers.rs index 15d3ecdcfa..e42aed4f8f 100644 --- a/src/ast/checkers.rs +++ b/src/ast/checkers.rs @@ -1175,6 +1175,64 @@ pub fn unnecessary_comprehension( None } +pub fn unnecessary_map(expr: &Expr, func: &Expr, args: &[Expr]) -> Option { + if let ExprKind::Name { id, .. } = &func.node { + if id == "map" { + if args.len() == 2 { + if let ExprKind::Lambda { .. } = &args[0].node { + return Some(Check::new( + CheckKind::UnnecessaryMap("generator".to_string()), + Range::from_located(expr), + )); + } + } + } else if id == "list" || id == "set" { + if let Some(arg) = args.first() { + if let ExprKind::Call { func, args, .. } = &arg.node { + if let ExprKind::Name { id: f, .. } = &func.node { + if f == "map" { + if let Some(arg) = args.first() { + if let ExprKind::Lambda { .. } = &arg.node { + return Some(Check::new( + CheckKind::UnnecessaryMap(id.to_string()), + Range::from_located(expr), + )); + } + } + } + } + } + } + } else if id == "dict" { + if args.len() == 1 { + if let ExprKind::Call { func, args, .. } = &args[0].node { + if let ExprKind::Name { id: f, .. } = &func.node { + if f == "map" { + if let Some(arg) = args.first() { + if let ExprKind::Lambda { body, .. } = &arg.node { + match &body.node { + ExprKind::Tuple { elts, .. } + | ExprKind::List { elts, .. } + if elts.len() == 2 => + { + return Some(Check::new( + CheckKind::UnnecessaryMap(id.to_string()), + Range::from_located(expr), + )) + } + _ => {} + } + } + } + } + } + } + } + } + } + None +} + // flake8-super /// Check that `super()` has no args pub fn super_args( diff --git a/src/check_ast.rs b/src/check_ast.rs index 5b58db0bd8..59bd8c4c28 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -865,6 +865,12 @@ where }; } + if self.settings.enabled.contains(&CheckCode::C417) { + if let Some(check) = checkers::unnecessary_map(expr, func, args) { + self.checks.push(check); + }; + } + // pyupgrade if self.settings.enabled.contains(&CheckCode::U002) && self.settings.target_version >= PythonVersion::Py310 diff --git a/src/checks.rs b/src/checks.rs index aea68a778c..0bdf8d4d4c 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -142,6 +142,7 @@ pub enum CheckCode { C414, C415, C416, + C417, // flake8-print T201, T203, @@ -284,6 +285,7 @@ pub enum CheckKind { UnnecessaryDoubleCastOrProcess(String, String), UnnecessarySubscriptReversal(String), UnnecessaryComprehension(String), + UnnecessaryMap(String), // flake8-print PrintFound, PPrintFound, @@ -440,6 +442,7 @@ impl CheckCode { CheckKind::UnnecessarySubscriptReversal("".to_string()) } CheckCode::C416 => CheckKind::UnnecessaryComprehension("".to_string()), + CheckCode::C417 => CheckKind::UnnecessaryMap("".to_string()), // flake8-print CheckCode::T201 => CheckKind::PrintFound, CheckCode::T203 => CheckKind::PPrintFound, @@ -582,6 +585,7 @@ impl CheckKind { CheckKind::UnnecessaryDoubleCastOrProcess(..) => &CheckCode::C414, CheckKind::UnnecessarySubscriptReversal(_) => &CheckCode::C415, CheckKind::UnnecessaryComprehension(..) => &CheckCode::C416, + CheckKind::UnnecessaryMap(_) => &CheckCode::C417, // flake8-print CheckKind::PrintFound => &CheckCode::T201, CheckKind::PPrintFound => &CheckCode::T203, @@ -863,6 +867,13 @@ impl CheckKind { CheckKind::UnnecessaryComprehension(obj_type) => { format!(" Unnecessary {obj_type} comprehension - rewrite using {obj_type}()") } + CheckKind::UnnecessaryMap(obj_type) => { + if obj_type == "generator" { + "Unnecessary map usage - rewrite using a generator expression".to_string() + } else { + format!("Unnecessary map usage - rewrite using a {obj_type} comprehension") + } + } // flake8-print CheckKind::PrintFound => "`print` found".to_string(), CheckKind::PPrintFound => "`pprint` found".to_string(), diff --git a/src/linter.rs b/src/linter.rs index 8a0bba6173..09479e995f 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -259,6 +259,7 @@ mod tests { #[test_case(CheckCode::C414, Path::new("C414.py"); "C414")] #[test_case(CheckCode::C415, Path::new("C415.py"); "C415")] #[test_case(CheckCode::C416, Path::new("C416.py"); "C416")] + #[test_case(CheckCode::C417, Path::new("C417.py"); "C417")] #[test_case(CheckCode::D100, Path::new("D.py"); "D100")] #[test_case(CheckCode::D101, Path::new("D.py"); "D101")] #[test_case(CheckCode::D102, Path::new("D.py"); "D102")] diff --git a/src/snapshots/ruff__linter__tests__C417_C417.py.snap b/src/snapshots/ruff__linter__tests__C417_C417.py.snap new file mode 100644 index 0000000000..c221c26246 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__C417_C417.py.snap @@ -0,0 +1,77 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: + UnnecessaryMap: generator + location: + row: 2 + column: 1 + end_location: + row: 2 + column: 27 + fix: ~ +- kind: + UnnecessaryMap: generator + location: + row: 3 + column: 1 + end_location: + row: 3 + column: 28 + fix: ~ +- kind: + UnnecessaryMap: list + location: + row: 4 + column: 1 + end_location: + row: 4 + column: 33 + fix: ~ +- kind: + UnnecessaryMap: generator + location: + row: 4 + column: 6 + end_location: + row: 4 + column: 32 + fix: ~ +- kind: + UnnecessaryMap: set + location: + row: 5 + column: 1 + end_location: + row: 5 + column: 37 + fix: ~ +- kind: + UnnecessaryMap: generator + location: + row: 5 + column: 5 + end_location: + row: 5 + column: 36 + fix: ~ +- kind: + UnnecessaryMap: dict + location: + row: 6 + column: 1 + end_location: + row: 6 + column: 37 + fix: ~ +- kind: + UnnecessaryMap: generator + location: + row: 6 + column: 6 + end_location: + row: 6 + column: 36 + fix: ~ +