-
Notifications
You must be signed in to change notification settings - Fork 903
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
## 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: #4789
- Loading branch information
Showing
12 changed files
with
480 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(()) | ||
} | ||
} |
170 changes: 170 additions & 0 deletions
170
crates/ruff/src/rules/perflint/rules/incorrect_dict_iterator.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
}) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
pub(crate) use incorrect_dict_iterator::{incorrect_dict_iterator, IncorrectDictIterator}; | ||
|
||
mod incorrect_dict_iterator; |
Oops, something went wrong.