From a489b96a65d14a3a12db1effaf82b7122ffb062f Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Sun, 20 Aug 2023 06:59:11 +0900 Subject: [PATCH] [`flake8-pie`] Implement `unnecessary-range-start` (`PIE808`) (#6690) --- .../test/fixtures/flake8_pie/PIE808.py | 13 +++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff/src/codes.rs | 1 + crates/ruff/src/rules/flake8_pie/mod.rs | 1 + crates/ruff/src/rules/flake8_pie/rules/mod.rs | 2 + .../rules/unnecessary_range_start.rs | 95 +++++++++++++++++++ ...__flake8_pie__tests__PIE808_PIE808.py.snap | 22 +++++ ruff.schema.json | 1 + 8 files changed, 138 insertions(+) create mode 100644 crates/ruff/resources/test/fixtures/flake8_pie/PIE808.py create mode 100644 crates/ruff/src/rules/flake8_pie/rules/unnecessary_range_start.rs create mode 100644 crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE808_PIE808.py.snap diff --git a/crates/ruff/resources/test/fixtures/flake8_pie/PIE808.py b/crates/ruff/resources/test/fixtures/flake8_pie/PIE808.py new file mode 100644 index 0000000000000..6765a20137f6b --- /dev/null +++ b/crates/ruff/resources/test/fixtures/flake8_pie/PIE808.py @@ -0,0 +1,13 @@ +# PIE808 +range(0, 10) + +# OK +range(x, 10) +range(-15, 10) +range(10) +range(0) +range(0, 10, x) +range(0, 10, 1) +range(0, 10, step=1) +range(start=0, stop=10) +range(0, stop=10) diff --git a/crates/ruff/src/checkers/ast/analyze/expression.rs b/crates/ruff/src/checkers/ast/analyze/expression.rs index 14278acc53fa9..94f10560dcec7 100644 --- a/crates/ruff/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff/src/checkers/ast/analyze/expression.rs @@ -531,6 +531,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryDictKwargs) { flake8_pie::rules::unnecessary_dict_kwargs(checker, expr, keywords); } + if checker.enabled(Rule::UnnecessaryRangeStart) { + flake8_pie::rules::unnecessary_range_start(checker, call); + } if checker.enabled(Rule::ExecBuiltin) { flake8_bandit::rules::exec_used(checker, func); } diff --git a/crates/ruff/src/codes.rs b/crates/ruff/src/codes.rs index d7f115bb29065..a94ccc69fe8b4 100644 --- a/crates/ruff/src/codes.rs +++ b/crates/ruff/src/codes.rs @@ -707,6 +707,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Pie, "800") => (RuleGroup::Unspecified, rules::flake8_pie::rules::UnnecessarySpread), (Flake8Pie, "804") => (RuleGroup::Unspecified, rules::flake8_pie::rules::UnnecessaryDictKwargs), (Flake8Pie, "807") => (RuleGroup::Unspecified, rules::flake8_pie::rules::ReimplementedListBuiltin), + (Flake8Pie, "808") => (RuleGroup::Unspecified, rules::flake8_pie::rules::UnnecessaryRangeStart), (Flake8Pie, "810") => (RuleGroup::Unspecified, rules::flake8_pie::rules::MultipleStartsEndsWith), // flake8-commas diff --git a/crates/ruff/src/rules/flake8_pie/mod.rs b/crates/ruff/src/rules/flake8_pie/mod.rs index a1cfb8349c99a..d31962dc28c3a 100644 --- a/crates/ruff/src/rules/flake8_pie/mod.rs +++ b/crates/ruff/src/rules/flake8_pie/mod.rs @@ -15,6 +15,7 @@ mod tests { #[test_case(Rule::DuplicateClassFieldDefinition, Path::new("PIE794.py"))] #[test_case(Rule::UnnecessaryDictKwargs, Path::new("PIE804.py"))] #[test_case(Rule::MultipleStartsEndsWith, Path::new("PIE810.py"))] + #[test_case(Rule::UnnecessaryRangeStart, Path::new("PIE808.py"))] #[test_case(Rule::UnnecessaryPass, Path::new("PIE790.py"))] #[test_case(Rule::UnnecessarySpread, Path::new("PIE800.py"))] #[test_case(Rule::ReimplementedListBuiltin, Path::new("PIE807.py"))] diff --git a/crates/ruff/src/rules/flake8_pie/rules/mod.rs b/crates/ruff/src/rules/flake8_pie/rules/mod.rs index e86cd7a19dc51..79012144acf6d 100644 --- a/crates/ruff/src/rules/flake8_pie/rules/mod.rs +++ b/crates/ruff/src/rules/flake8_pie/rules/mod.rs @@ -4,6 +4,7 @@ pub(crate) use no_unnecessary_pass::*; pub(crate) use non_unique_enums::*; pub(crate) use reimplemented_list_builtin::*; pub(crate) use unnecessary_dict_kwargs::*; +pub(crate) use unnecessary_range_start::*; pub(crate) use unnecessary_spread::*; mod duplicate_class_field_definition; @@ -12,4 +13,5 @@ mod no_unnecessary_pass; mod non_unique_enums; mod reimplemented_list_builtin; mod unnecessary_dict_kwargs; +mod unnecessary_range_start; mod unnecessary_spread; diff --git a/crates/ruff/src/rules/flake8_pie/rules/unnecessary_range_start.rs b/crates/ruff/src/rules/flake8_pie/rules/unnecessary_range_start.rs new file mode 100644 index 0000000000000..07067d6f75bb4 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/unnecessary_range_start.rs @@ -0,0 +1,95 @@ +use num_bigint::BigInt; + +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::{AlwaysAutofixableViolation, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Constant, Expr, Ranged}; + +use crate::autofix::edits::{remove_argument, Parentheses}; +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for `range` calls with an unnecessary `start` argument. +/// +/// ## Why is this bad? +/// `range(0, x)` is equivalent to `range(x)`, as `0` is the default value for +/// the `start` argument. Omitting the `start` argument makes the code more +/// concise and idiomatic. +/// +/// ## Example +/// ```python +/// range(0, 3) +/// ``` +/// +/// Use instead: +/// ```python +/// range(3) +/// ``` +/// +/// ## References +/// - [Python documentation: `range`](https://docs.python.org/3/library/stdtypes.html#range) +#[violation] +pub struct UnnecessaryRangeStart; + +impl AlwaysAutofixableViolation for UnnecessaryRangeStart { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary `start` argument in `range`") + } + + fn autofix_title(&self) -> String { + format!("Remove `start` argument") + } +} + +/// PIE808 +pub(crate) fn unnecessary_range_start(checker: &mut Checker, call: &ast::ExprCall) { + // Verify that the call is to the `range` builtin. + let Expr::Name(ast::ExprName { id, .. }) = call.func.as_ref() else { + return; + }; + if id != "range" { + return; + }; + if !checker.semantic().is_builtin("range") { + return; + }; + + // `range` doesn't accept keyword arguments. + if !call.arguments.keywords.is_empty() { + return; + } + + // Verify that the call has exactly two arguments (no `step`). + let [start, _] = call.arguments.args.as_slice() else { + return; + }; + + // Verify that the `start` argument is the literal `0`. + let Expr::Constant(ast::ExprConstant { + value: Constant::Int(value), + .. + }) = start + else { + return; + }; + if *value != BigInt::from(0) { + return; + }; + + let mut diagnostic = Diagnostic::new(UnnecessaryRangeStart, start.range()); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.try_set_fix(|| { + remove_argument( + &start, + &call.arguments, + Parentheses::Preserve, + checker.locator(), + checker.source_type, + ) + .map(Fix::automatic) + }); + } + checker.diagnostics.push(diagnostic); +} diff --git a/crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE808_PIE808.py.snap b/crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE808_PIE808.py.snap new file mode 100644 index 0000000000000..0215ece47a8cb --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/snapshots/ruff__rules__flake8_pie__tests__PIE808_PIE808.py.snap @@ -0,0 +1,22 @@ +--- +source: crates/ruff/src/rules/flake8_pie/mod.rs +--- +PIE808.py:2:7: PIE808 [*] Unnecessary `start` argument in `range` + | +1 | # PIE808 +2 | range(0, 10) + | ^ PIE808 +3 | +4 | # OK + | + = help: Remove `start` argument + +ℹ Fix +1 1 | # PIE808 +2 |-range(0, 10) + 2 |+range(10) +3 3 | +4 4 | # OK +5 5 | range(x, 10) + + diff --git a/ruff.schema.json b/ruff.schema.json index 8afd482e95186..faef882ebdb8d 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2159,6 +2159,7 @@ "PIE800", "PIE804", "PIE807", + "PIE808", "PIE81", "PIE810", "PL",