[perflint] Add perflint plugin, add first rule PERF102 (#4821)

## Summary

Adds boilerplate for implementing the
[perflint](https://github.com/tonybaloney/perflint/) plugin, plus a
first rule.

## Test Plan

Fixture added for PER8102

## Issue link

Refers: https://github.com/charliermarsh/ruff/issues/4789
This commit is contained in:
qdegraaf 2023-06-13 03:54:44 +02:00 committed by GitHub
parent be2fa6d217
commit a477720f4e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 480 additions and 1 deletions

25
LICENSE
View file

@ -1199,6 +1199,31 @@ are:
- flake8-django, licensed under the GPL license.
- perflint, licensed as follows:
"""
MIT License
Copyright (c) 2022 Anthony Shaw
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
"""
- rust-analyzer/text-size, licensed under the MIT license:
"""
Permission is hereby granted, free of charge, to any

View file

@ -0,0 +1,71 @@
some_dict = {"a": 12, "b": 32, "c": 44}
for _, value in some_dict.items(): # PERF102
print(value)
for key, _ in some_dict.items(): # PERF102
print(key)
for weird_arg_name, _ in some_dict.items(): # PERF102
print(weird_arg_name)
for name, (_, _) in some_dict.items(): # PERF102
pass
for name, (value1, _) in some_dict.items(): # OK
pass
for (key1, _), (_, _) in some_dict.items(): # PERF102
pass
for (_, (_, _)), (value, _) in some_dict.items(): # PERF102
pass
for (_, key2), (value1, _) in some_dict.items(): # OK
pass
for ((_, key2), (value1, _)) in some_dict.items(): # OK
pass
for ((_, key2), (_, _)) in some_dict.items(): # PERF102
pass
for (_, _, _, variants), (r_language, _, _, _) in some_dict.items(): # OK
pass
for (_, _, (_, variants)), (_, (_, (r_language, _))) in some_dict.items(): # OK
pass
for key, value in some_dict.items(): # OK
print(key, value)
for _, value in some_dict.items(12): # OK
print(value)
for key in some_dict.keys(): # OK
print(key)
for value in some_dict.values(): # OK
print(value)
for name, (_, _) in (some_function()).items(): # PERF102
pass
for name, (_, _) in (some_function().some_attribute).items(): # PERF102
pass

View file

@ -50,7 +50,8 @@ use crate::rules::{
flake8_print, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return, flake8_self,
flake8_simplify, flake8_slots, flake8_tidy_imports, flake8_type_checking,
flake8_unused_arguments, flake8_use_pathlib, flynt, mccabe, numpy, pandas_vet, pep8_naming,
pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops,
perflint, pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff,
tryceratops,
};
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
@ -1525,6 +1526,9 @@ where
flake8_simplify::rules::key_in_dict_for(self, target, iter);
}
}
if self.enabled(Rule::IncorrectDictIterator) {
perflint::rules::incorrect_dict_iterator(self, target, iter);
}
}
Stmt::Try(ast::StmtTry {
body,

View file

@ -783,6 +783,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
// airflow
(Airflow, "001") => (RuleGroup::Unspecified, rules::airflow::rules::AirflowVariableNameTaskIdMismatch),
// perflint
(Perflint, "102") => (RuleGroup::Unspecified, rules::perflint::rules::IncorrectDictIterator),
// flake8-fixme
(Flake8Fixme, "001") => (RuleGroup::Unspecified, rules::flake8_fixme::rules::LineContainsFixme),
(Flake8Fixme, "002") => (RuleGroup::Unspecified, rules::flake8_fixme::rules::LineContainsTodo),

View file

@ -191,6 +191,9 @@ pub enum Linter {
/// [Airflow](https://pypi.org/project/apache-airflow/)
#[prefix = "AIR"]
Airflow,
/// [Perflint](https://pypi.org/project/perflint/)
#[prefix = "PERF"]
Perflint,
/// Ruff-specific rules
#[prefix = "RUF"]
Ruff,

View file

@ -45,6 +45,7 @@ pub mod mccabe;
pub mod numpy;
pub mod pandas_vet;
pub mod pep8_naming;
pub mod perflint;
pub mod pycodestyle;
pub mod pydocstyle;
pub mod pyflakes;

View file

@ -0,0 +1,26 @@
//! Rules from [perflint](https://pypi.org/project/perflint/).
pub(crate) mod rules;
#[cfg(test)]
mod tests {
use std::path::Path;
use anyhow::Result;
use test_case::test_case;
use crate::assert_messages;
use crate::registry::Rule;
use crate::settings::Settings;
use crate::test::test_path;
#[test_case(Rule::IncorrectDictIterator, Path::new("PERF102.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("perflint").join(path).as_path(),
&Settings::for_rule(rule_code),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}

View file

@ -0,0 +1,170 @@
use std::fmt;
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::Expr;
use rustpython_parser::{ast, lexer, Mode, Tok};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::prelude::Ranged;
use ruff_python_ast::source_code::Locator;
use crate::checkers::ast::Checker;
use crate::registry::AsRule;
/// ## What it does
/// Checks for uses of `dict.items()` that discard either the key or the value
/// when iterating over the dictionary.
///
/// ## Why is this bad?
/// If you only need the keys or values of a dictionary, you should use
/// `dict.keys()` or `dict.values()` respectively, instead of `dict.items()`.
/// These specialized methods are more efficient than `dict.items()`, as they
/// avoid allocating tuples for every item in the dictionary. They also
/// communicate the intent of the code more clearly.
///
/// ## Example
/// ```python
/// some_dict = {"a": 1, "b": 2}
/// for _, val in some_dict.items():
/// print(val)
/// ```
///
/// Use instead:
/// ```python
/// some_dict = {"a": 1, "b": 2}
/// for val in some_dict.values():
/// print(val)
/// ```
#[violation]
pub struct IncorrectDictIterator {
subset: DictSubset,
}
impl AlwaysAutofixableViolation for IncorrectDictIterator {
#[derive_message_formats]
fn message(&self) -> String {
let IncorrectDictIterator { subset } = self;
format!("When using only the {subset} of a dict use the `{subset}()` method")
}
fn autofix_title(&self) -> String {
let IncorrectDictIterator { subset } = self;
format!("Replace `.items()` with `.{subset}()`")
}
}
/// PERF102
pub(crate) fn incorrect_dict_iterator(checker: &mut Checker, target: &Expr, iter: &Expr) {
let Expr::Tuple(ast::ExprTuple {
elts,
..
}) = target
else {
return
};
if elts.len() != 2 {
return;
}
let Expr::Call(ast::ExprCall { func, args, .. }) = iter else {
return;
};
if !args.is_empty() {
return;
}
let Expr::Attribute(ast::ExprAttribute { attr, value, .. }) = func.as_ref() else {
return;
};
if attr != "items" {
return;
}
let unused_key = is_ignored_tuple_or_name(&elts[0]);
let unused_value = is_ignored_tuple_or_name(&elts[1]);
match (unused_key, unused_value) {
(true, true) => {
// Both the key and the value are unused.
}
(false, false) => {
// Neither the key nor the value are unused.
}
(true, false) => {
// The key is unused, so replace with `dict.values()`.
let mut diagnostic = Diagnostic::new(
IncorrectDictIterator {
subset: DictSubset::Values,
},
func.range(),
);
if checker.patch(diagnostic.kind.rule()) {
if let Some(range) = attribute_range(value.end(), checker.locator) {
let replace_attribute = Edit::range_replacement("values".to_string(), range);
let replace_target = Edit::range_replacement(
checker.locator.slice(elts[1].range()).to_string(),
target.range(),
);
diagnostic.set_fix(Fix::suggested_edits(replace_attribute, [replace_target]));
}
}
checker.diagnostics.push(diagnostic);
}
(false, true) => {
// The value is unused, so replace with `dict.keys()`.
let mut diagnostic = Diagnostic::new(
IncorrectDictIterator {
subset: DictSubset::Keys,
},
func.range(),
);
if checker.patch(diagnostic.kind.rule()) {
if let Some(range) = attribute_range(value.end(), checker.locator) {
let replace_attribute = Edit::range_replacement("keys".to_string(), range);
let replace_target = Edit::range_replacement(
checker.locator.slice(elts[0].range()).to_string(),
target.range(),
);
diagnostic.set_fix(Fix::suggested_edits(replace_attribute, [replace_target]));
}
}
checker.diagnostics.push(diagnostic);
}
}
}
#[derive(Debug, PartialEq, Eq)]
enum DictSubset {
Keys,
Values,
}
impl fmt::Display for DictSubset {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
DictSubset::Keys => fmt.write_str("keys"),
DictSubset::Values => fmt.write_str("values"),
}
}
}
/// Returns `true` if the given expression is either an ignored value or a tuple of ignored values.
fn is_ignored_tuple_or_name(expr: &Expr) -> bool {
match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(is_ignored_tuple_or_name),
Expr::Name(ast::ExprName { id, .. }) => id == "_",
_ => false,
}
}
/// Returns the range of the attribute identifier after the given location, if any.
fn attribute_range(at: TextSize, locator: &Locator) -> Option<TextRange> {
lexer::lex_starts_at(locator.after(at), Mode::Expression, at)
.flatten()
.find_map(|(tok, range)| {
if matches!(tok, Tok::Name { .. }) {
Some(range)
} else {
None
}
})
}

View file

@ -0,0 +1,3 @@
pub(crate) use incorrect_dict_iterator::{incorrect_dict_iterator, IncorrectDictIterator};
mod incorrect_dict_iterator;

View file

@ -0,0 +1,167 @@
---
source: crates/ruff/src/rules/perflint/mod.rs
---
PERF102.py:3:17: PERF102 [*] When using only the values of a dict use the `values()` method
|
1 | some_dict = {"a": 12, "b": 32, "c": 44}
2 |
3 | for _, value in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
4 | print(value)
|
= help: Replace `.items()` with `.values()`
Suggested fix
1 1 | some_dict = {"a": 12, "b": 32, "c": 44}
2 2 |
3 |-for _, value in some_dict.items(): # PERF102
3 |+for value in some_dict.values(): # PERF102
4 4 | print(value)
5 5 |
6 6 |
PERF102.py:7:15: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
7 | for key, _ in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
8 | print(key)
|
= help: Replace `.items()` with `.keys()`
Suggested fix
4 4 | print(value)
5 5 |
6 6 |
7 |-for key, _ in some_dict.items(): # PERF102
7 |+for key in some_dict.keys(): # PERF102
8 8 | print(key)
9 9 |
10 10 |
PERF102.py:11:26: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
11 | for weird_arg_name, _ in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
12 | print(weird_arg_name)
|
= help: Replace `.items()` with `.keys()`
Suggested fix
8 8 | print(key)
9 9 |
10 10 |
11 |-for weird_arg_name, _ in some_dict.items(): # PERF102
11 |+for weird_arg_name in some_dict.keys(): # PERF102
12 12 | print(weird_arg_name)
13 13 |
14 14 |
PERF102.py:15:21: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
15 | for name, (_, _) in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
16 | pass
|
= help: Replace `.items()` with `.keys()`
Suggested fix
12 12 | print(weird_arg_name)
13 13 |
14 14 |
15 |-for name, (_, _) in some_dict.items(): # PERF102
15 |+for name in some_dict.keys(): # PERF102
16 16 | pass
17 17 |
18 18 |
PERF102.py:23:26: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
23 | for (key1, _), (_, _) in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
24 | pass
|
= help: Replace `.items()` with `.keys()`
Suggested fix
20 20 | pass
21 21 |
22 22 |
23 |-for (key1, _), (_, _) in some_dict.items(): # PERF102
23 |+for (key1, _) in some_dict.keys(): # PERF102
24 24 | pass
25 25 |
26 26 |
PERF102.py:27:32: PERF102 [*] When using only the values of a dict use the `values()` method
|
27 | for (_, (_, _)), (value, _) in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
28 | pass
|
= help: Replace `.items()` with `.values()`
Suggested fix
24 24 | pass
25 25 |
26 26 |
27 |-for (_, (_, _)), (value, _) in some_dict.items(): # PERF102
27 |+for (value, _) in some_dict.values(): # PERF102
28 28 | pass
29 29 |
30 30 |
PERF102.py:39:28: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
39 | for ((_, key2), (_, _)) in some_dict.items(): # PERF102
| ^^^^^^^^^^^^^^^ PERF102
40 | pass
|
= help: Replace `.items()` with `.keys()`
Suggested fix
36 36 | pass
37 37 |
38 38 |
39 |-for ((_, key2), (_, _)) in some_dict.items(): # PERF102
39 |+for (_, key2) in some_dict.keys(): # PERF102
40 40 | pass
41 41 |
42 42 |
PERF102.py:67:21: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
67 | for name, (_, _) in (some_function()).items(): # PERF102
| ^^^^^^^^^^^^^^^^^^^^^^^ PERF102
68 | pass
|
= help: Replace `.items()` with `.keys()`
Suggested fix
64 64 | print(value)
65 65 |
66 66 |
67 |-for name, (_, _) in (some_function()).items(): # PERF102
67 |+for name in (some_function()).keys(): # PERF102
68 68 | pass
69 69 |
70 70 | for name, (_, _) in (some_function().some_attribute).items(): # PERF102
PERF102.py:70:21: PERF102 [*] When using only the keys of a dict use the `keys()` method
|
68 | pass
69 |
70 | for name, (_, _) in (some_function().some_attribute).items(): # PERF102
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PERF102
71 | pass
|
= help: Replace `.items()` with `.keys()`
Suggested fix
67 67 | for name, (_, _) in (some_function()).items(): # PERF102
68 68 | pass
69 69 |
70 |-for name, (_, _) in (some_function().some_attribute).items(): # PERF102
70 |+for name in (some_function().some_attribute).keys(): # PERF102
71 71 | pass

View file

@ -74,6 +74,7 @@ natively, including:
- [mccabe](https://pypi.org/project/mccabe/)
- [pandas-vet](https://pypi.org/project/pandas-vet/)
- [pep8-naming](https://pypi.org/project/pep8-naming/)
- [perflint](https://pypi.org/project/perflint/) ([#4789](https://github.com/astral-sh/ruff/issues/4789))
- [pydocstyle](https://pypi.org/project/pydocstyle/)
- [pygrep-hooks](https://github.com/pre-commit/pygrep-hooks)
- [pyupgrade](https://pypi.org/project/pyupgrade/)
@ -175,6 +176,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [mccabe](https://pypi.org/project/mccabe/)
- [pandas-vet](https://pypi.org/project/pandas-vet/)
- [pep8-naming](https://pypi.org/project/pep8-naming/)
- [perflint](https://pypi.org/project/perflint/) ([#4789](https://github.com/astral-sh/ruff/issues/4789))
- [pydocstyle](https://pypi.org/project/pydocstyle/)
- [tryceratops](https://pypi.org/project/tryceratops/)

4
ruff.schema.json generated
View file

@ -2061,6 +2061,10 @@
"PD9",
"PD90",
"PD901",
"PERF",
"PERF1",
"PERF10",
"PERF102",
"PGH",
"PGH0",
"PGH00",