Skip to content

Commit

Permalink
[refurb] Implement repeated-append rule (FURB113) (#6702)
Browse files Browse the repository at this point in the history
## Summary

As an initial effort with replicating `refurb` rules (#1348 ), this PR
adds support for
[FURB113](https://github.com/dosisod/refurb/blob/master/refurb/checks/builtin/list_extend.py)
and adds a new category of checks.

## Test Plan

I included a new test + checked that all other tests pass.
  • Loading branch information
SavchenkoValeriy authored Aug 28, 2023
1 parent 1439bb5 commit 26d53c5
Show file tree
Hide file tree
Showing 13 changed files with 1,003 additions and 11 deletions.
169 changes: 169 additions & 0 deletions crates/ruff/resources/test/fixtures/refurb/FURB113.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
from typing import List


def func():
pass


nums = []
nums2 = []
nums3: list[int] = func()
nums4: List[int]


class C:
def append(self, x):
pass


# Errors.


# FURB113
nums.append(1)
nums.append(2)
pass


# FURB113
nums3.append(1)
nums3.append(2)
pass


# FURB113
nums4.append(1)
nums4.append(2)
pass


# FURB113
nums.append(1)
nums2.append(1)
nums.append(2)
nums.append(3)
pass


# FURB113
nums.append(1)
nums2.append(1)
nums.append(2)
# FURB113
nums3.append(1)
nums.append(3)
# FURB113
nums4.append(1)
nums4.append(2)
nums3.append(2)
pass

# FURB113
nums.append(1)
nums.append(2)
nums.append(3)


if True:
# FURB113
nums.append(1)
nums.append(2)


if True:
# FURB113
nums.append(1)
nums.append(2)
pass


if True:
# FURB113
nums.append(1)
nums2.append(1)
nums.append(2)
nums.append(3)


def yes_one(x: list[int]):
# FURB113
x.append(1)
x.append(2)


def yes_two(x: List[int]):
# FURB113
x.append(1)
x.append(2)


def yes_three(*, x: list[int]):
# FURB113
x.append(1)
x.append(2)


def yes_four(x: list[int], /):
# FURB113
x.append(1)
x.append(2)


def yes_five(x: list[int], y: list[int]):
# FURB113
x.append(1)
x.append(2)
y.append(1)
x.append(3)


def yes_six(x: list):
# FURB113
x.append(1)
x.append(2)


# Non-errors.

nums.append(1)
pass
nums.append(2)


if True:
nums.append(1)
pass
nums.append(2)


nums.append(1)
pass


nums.append(1)
nums2.append(2)


nums.copy()
nums.copy()


c = C()
c.append(1)
c.append(2)


def not_one(x):
x.append(1)
x.append(2)


def not_two(x: C):
x.append(1)
x.append(2)


# redefining a list variable with a new type shouldn't confuse ruff.
nums2 = C()
nums2.append(1)
nums2.append(2)
5 changes: 4 additions & 1 deletion crates/ruff/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::rules::{
flake8_django, flake8_errmsg, flake8_import_conventions, flake8_pie, flake8_pyi,
flake8_pytest_style, flake8_raise, flake8_return, flake8_simplify, flake8_slots,
flake8_tidy_imports, flake8_type_checking, mccabe, pandas_vet, pep8_naming, perflint,
pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, tryceratops,
};
use crate::settings::types::PythonVersion;

Expand Down Expand Up @@ -1484,6 +1484,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::AsyncioDanglingTask) {
ruff::rules::asyncio_dangling_task(checker, value);
}
if checker.enabled(Rule::RepeatedAppend) {
refurb::rules::repeated_append(checker, stmt);
}
}
_ => {}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Slots, "001") => (RuleGroup::Unspecified, rules::flake8_slots::rules::NoSlotsInTupleSubclass),
(Flake8Slots, "002") => (RuleGroup::Unspecified, rules::flake8_slots::rules::NoSlotsInNamedtupleSubclass),

// refurb
(Refurb, "113") => (RuleGroup::Nursery, rules::refurb::rules::RepeatedAppend),

_ => return None,
})
}
3 changes: 3 additions & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ pub enum Linter {
/// [Perflint](https://pypi.org/project/perflint/)
#[prefix = "PERF"]
Perflint,
/// [refurb](https://pypi.org/project/refurb/)
#[prefix = "FURB"]
Refurb,
/// Ruff-specific rules
#[prefix = "RUF"]
Ruff,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,6 @@ pub mod pyflakes;
pub mod pygrep_hooks;
pub mod pylint;
pub mod pyupgrade;
pub mod refurb;
pub mod ruff;
pub mod tryceratops;
25 changes: 25 additions & 0 deletions crates/ruff/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! Rules from [refurb](https://pypi.org/project/refurb/)/
pub(crate) mod rules;

#[cfg(test)]
mod tests {
use std::path::Path;

use anyhow::Result;
use test_case::test_case;

use crate::registry::Rule;
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::RepeatedAppend, Path::new("FURB113.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("refurb").join(path).as_path(),
&settings::Settings::for_rule(rule_code),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
3 changes: 3 additions & 0 deletions crates/ruff/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub(crate) use repeated_append::*;

mod repeated_append;
Loading

0 comments on commit 26d53c5

Please sign in to comment.