Detect unnecessary params in `lru_cache` (#664)

This commit is contained in:
Chammika Mannakkara 2022-11-10 00:02:48 +09:00 committed by GitHub
parent 6c17670aa5
commit ff0f5968fa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 210 additions and 8 deletions

View File

@ -432,6 +432,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI.
| U008 | SuperCallWithParameters | Use `super()` instead of `super(__class__, self)` | 🛠 |
| U009 | PEP3120UnnecessaryCodingComment | utf-8 encoding declaration is unnecessary | 🛠 |
| U010 | UnnecessaryFutureImport | Unnessary __future__ import `...` for target Python version | |
| U011 | UnnecessaryLRUCacheParams | Unnessary parameters to functools.lru_cache | 🛠 |
### pep8-naming
@ -654,7 +655,7 @@ including:
- [`flake8-annotations`](https://pypi.org/project/flake8-annotations/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (17/32)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (12/34)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34)
- [`autoflake`](https://pypi.org/project/autoflake/) (1/7)
Beyond rule-set parity, Ruff suffers from the following limitations vis-à-vis Flake8:
@ -679,7 +680,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (17/32)
Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa),
and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (12/34).
and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34).
If you're looking to use Ruff, but rely on an unsupported Flake8 plugin, free to file an Issue.

53
resources/test/fixtures/U011_0.py vendored Normal file
View File

@ -0,0 +1,53 @@
import functools
from functools import lru_cache
@lru_cache()
def fixme1():
pass
@other_deco_after
@functools.lru_cache()
def fixme2():
pass
@lru_cache(maxsize=None)
def fixme3():
pass
@functools.lru_cache(maxsize=None)
@other_deco_before
def fixme4():
pass
@lru_cache
def correct1():
pass
@functools.lru_cache
def correct2():
pass
@functoools.lru_cache(maxsize=64)
def correct3():
pass
def user_func():
pass
@lru_cache(user_func)
def correct4():
pass
@lru_cache(user_func, maxsize=None)
def correct5():
pass

15
resources/test/fixtures/U011_1.py vendored Normal file
View File

@ -0,0 +1,15 @@
import functools
def lru_cache(maxsize=None):
pass
@lru_cache()
def dont_fixme():
pass
@lru_cache(maxsize=None)
def dont_fixme():
pass

View File

@ -37,10 +37,11 @@ use crate::{
};
const GLOBAL_SCOPE_INDEX: usize = 0;
const TRACK_FROM_IMPORTS: [&str; 9] = [
const TRACK_FROM_IMPORTS: [&str; 10] = [
"collections",
"collections.abc",
"contextlib",
"functools",
"re",
"typing",
"typing.io",
@ -62,6 +63,8 @@ pub struct Checker<'a> {
// Edit tracking.
// TODO(charlie): Instead of exposing deletions, wrap in a public API.
pub(crate) deletions: BTreeSet<usize>,
// Import tracking.
pub(crate) from_imports: BTreeMap<&'a str, BTreeSet<&'a str>>,
// Retain all scopes and parent nodes, along with a stack of indexes to track which are active
// at various points in time.
pub(crate) parents: Vec<&'a Stmt>,
@ -84,7 +87,6 @@ pub struct Checker<'a> {
futures_allowed: bool,
annotations_future_enabled: bool,
except_handlers: Vec<Vec<String>>,
from_imports: BTreeMap<&'a str, BTreeSet<&'a str>>,
}
impl<'a> Checker<'a> {
@ -102,6 +104,7 @@ impl<'a> Checker<'a> {
checks: Default::default(),
definitions: Default::default(),
deletions: Default::default(),
from_imports: Default::default(),
parents: Default::default(),
parent_stack: Default::default(),
scopes: Default::default(),
@ -124,7 +127,6 @@ impl<'a> Checker<'a> {
futures_allowed: true,
annotations_future_enabled: Default::default(),
except_handlers: Default::default(),
from_imports: Default::default(),
}
}
@ -332,6 +334,12 @@ where
}
}
if self.settings.enabled.contains(&CheckCode::U011)
&& self.settings.target_version >= PythonVersion::Py38
{
pyupgrade::plugins::unnecessary_lru_cache_params(self, decorator_list);
}
if self.settings.enabled.contains(&CheckCode::B018) {
flake8_bugbear::plugins::useless_expression(self, body);
}

View File

@ -140,6 +140,7 @@ pub enum CheckCode {
U008,
U009,
U010,
U011,
// pydocstyle
D100,
D101,
@ -403,6 +404,7 @@ pub enum CheckKind {
SuperCallWithParameters,
PEP3120UnnecessaryCodingComment,
UnnecessaryFutureImport(String),
UnnecessaryLRUCacheParams,
// pydocstyle
BlankLineAfterLastSection(String),
BlankLineAfterSection(String),
@ -631,6 +633,7 @@ impl CheckCode {
CheckCode::U008 => CheckKind::SuperCallWithParameters,
CheckCode::U009 => CheckKind::PEP3120UnnecessaryCodingComment,
CheckCode::U010 => CheckKind::UnnecessaryFutureImport("...".to_string()),
CheckCode::U011 => CheckKind::UnnecessaryLRUCacheParams,
// pydocstyle
CheckCode::D100 => CheckKind::PublicModule,
CheckCode::D101 => CheckKind::PublicClass,
@ -824,6 +827,7 @@ impl CheckCode {
CheckCode::U008 => CheckCategory::Pyupgrade,
CheckCode::U009 => CheckCategory::Pyupgrade,
CheckCode::U010 => CheckCategory::Pyupgrade,
CheckCode::U011 => CheckCategory::Pyupgrade,
CheckCode::D100 => CheckCategory::Pydocstyle,
CheckCode::D101 => CheckCategory::Pydocstyle,
CheckCode::D102 => CheckCategory::Pydocstyle,
@ -1009,6 +1013,7 @@ impl CheckKind {
CheckKind::SuperCallWithParameters => &CheckCode::U008,
CheckKind::PEP3120UnnecessaryCodingComment => &CheckCode::U009,
CheckKind::UnnecessaryFutureImport(_) => &CheckCode::U010,
CheckKind::UnnecessaryLRUCacheParams => &CheckCode::U011,
// pydocstyle
CheckKind::BlankLineAfterLastSection(_) => &CheckCode::D413,
CheckKind::BlankLineAfterSection(_) => &CheckCode::D410,
@ -1452,6 +1457,9 @@ impl CheckKind {
CheckKind::UnnecessaryFutureImport(name) => {
format!("Unnessary __future__ import `{name}` for target Python version")
}
CheckKind::UnnecessaryLRUCacheParams => {
"Unnessary parameters to functools.lru_cache".to_string()
}
// pydocstyle
CheckKind::FitsOnOneLine => "One-line docstring should fit on one line".to_string(),
CheckKind::BlankLineAfterSummary => {
@ -1686,6 +1694,7 @@ impl CheckKind {
| CheckKind::DeprecatedUnittestAlias(_, _)
| CheckKind::DoNotAssertFalse
| CheckKind::DuplicateHandlerException(_)
| CheckKind::IsLiteral
| CheckKind::NewLineAfterLastParagraph
| CheckKind::NewLineAfterSectionName(_)
| CheckKind::NoBlankLineAfterFunction(_)
@ -1697,6 +1706,7 @@ impl CheckKind {
| CheckKind::NoUnderIndentation
| CheckKind::OneBlankLineAfterClass(_)
| CheckKind::OneBlankLineBeforeClass(_)
| CheckKind::PEP3120UnnecessaryCodingComment
| CheckKind::PPrintFound
| CheckKind::PrintFound
| CheckKind::RaiseNotImplemented
@ -1713,6 +1723,7 @@ impl CheckKind {
| CheckKind::UnnecessaryGeneratorDict
| CheckKind::UnnecessaryGeneratorList
| CheckKind::UnnecessaryGeneratorSet
| CheckKind::UnnecessaryLRUCacheParams
| CheckKind::UnnecessaryListCall
| CheckKind::UnnecessaryListComprehensionSet
| CheckKind::UnnecessaryListComprehensionDict
@ -1727,8 +1738,6 @@ impl CheckKind {
| CheckKind::UsePEP604Annotation
| CheckKind::UselessMetaclassType
| CheckKind::UselessObjectInheritance(_)
| CheckKind::PEP3120UnnecessaryCodingComment
| CheckKind::IsLiteral
)
}
}

View File

@ -256,6 +256,7 @@ pub enum CheckCodePrefix {
U009,
U01,
U010,
U011,
W,
W2,
W29,
@ -958,6 +959,7 @@ impl CheckCodePrefix {
CheckCode::U008,
CheckCode::U009,
CheckCode::U010,
CheckCode::U011,
],
CheckCodePrefix::U0 => vec![
CheckCode::U001,
@ -970,6 +972,7 @@ impl CheckCodePrefix {
CheckCode::U008,
CheckCode::U009,
CheckCode::U010,
CheckCode::U011,
],
CheckCodePrefix::U00 => vec![
CheckCode::U001,
@ -991,8 +994,9 @@ impl CheckCodePrefix {
CheckCodePrefix::U007 => vec![CheckCode::U007],
CheckCodePrefix::U008 => vec![CheckCode::U008],
CheckCodePrefix::U009 => vec![CheckCode::U009],
CheckCodePrefix::U01 => vec![CheckCode::U010],
CheckCodePrefix::U01 => vec![CheckCode::U010, CheckCode::U011],
CheckCodePrefix::U010 => vec![CheckCode::U010],
CheckCodePrefix::U011 => vec![CheckCode::U011],
CheckCodePrefix::W => vec![CheckCode::W292, CheckCode::W605],
CheckCodePrefix::W2 => vec![CheckCode::W292],
CheckCodePrefix::W29 => vec![CheckCode::W292],
@ -1256,6 +1260,7 @@ impl CheckCodePrefix {
CheckCodePrefix::U009 => PrefixSpecificity::Explicit,
CheckCodePrefix::U01 => PrefixSpecificity::Tens,
CheckCodePrefix::U010 => PrefixSpecificity::Explicit,
CheckCodePrefix::U011 => PrefixSpecificity::Explicit,
CheckCodePrefix::W => PrefixSpecificity::Category,
CheckCodePrefix::W2 => PrefixSpecificity::Hundreds,
CheckCodePrefix::W29 => PrefixSpecificity::Tens,

View File

@ -446,6 +446,8 @@ mod tests {
#[test_case(CheckCode::U009, Path::new("U009_2.py"); "U009_2")]
#[test_case(CheckCode::U009, Path::new("U009_3.py"); "U009_3")]
#[test_case(CheckCode::U010, Path::new("U010.py"); "U010")]
#[test_case(CheckCode::U011, Path::new("U011_0.py"); "U011_0")]
#[test_case(CheckCode::U011, Path::new("U011_1.py"); "U011_1")]
#[test_case(CheckCode::W292, Path::new("W292_0.py"); "W292_0")]
#[test_case(CheckCode::W292, Path::new("W292_1.py"); "W292_1")]
#[test_case(CheckCode::W292, Path::new("W292_2.py"); "W292_2")]

View File

@ -1,3 +1,6 @@
use std::collections::BTreeSet;
use rustpython_ast::{Constant, KeywordData};
use rustpython_parser::ast::{ArgData, Expr, ExprKind, Stmt, StmtKind};
use crate::ast::helpers;
@ -196,3 +199,50 @@ pub fn unnecessary_future_import(
}
None
}
/// U011
pub fn unnecessary_lru_cache_params(
decorator_list: &[Expr],
target_version: PythonVersion,
imports: Option<&BTreeSet<&str>>,
) -> Option<Check> {
for expr in decorator_list.iter() {
if let ExprKind::Call {
func,
args,
keywords,
} = &expr.node
{
if args.is_empty()
&& helpers::match_name_or_attr_from_module(func, "lru_cache", "functools", imports)
{
// Ex) `functools.lru_cache()`
if keywords.is_empty() {
return Some(Check::new(
CheckKind::UnnecessaryLRUCacheParams,
Range::from_located(expr),
));
}
// Ex) `functools.lru_cache(maxsize=None)`
if target_version >= PythonVersion::Py39 && keywords.len() == 1 {
let KeywordData { arg, value } = &keywords[0].node;
if arg.as_ref().map(|arg| arg == "maxsize").unwrap_or_default()
&& matches!(
value.node,
ExprKind::Constant {
value: Constant::None,
kind: None,
}
)
{
return Some(Check::new(
CheckKind::UnnecessaryLRUCacheParams,
Range::from_located(expr),
));
}
}
}
}
}
None
}

View File

@ -3,6 +3,7 @@ pub use super_call_with_parameters::super_call_with_parameters;
pub use type_of_primitive::type_of_primitive;
pub use unnecessary_abspath::unnecessary_abspath;
pub use unnecessary_future_import::unnecessary_future_import;
pub use unnecessary_lru_cache_params::unnecessary_lru_cache_params;
pub use use_pep585_annotation::use_pep585_annotation;
pub use use_pep604_annotation::use_pep604_annotation;
pub use useless_metaclass_type::useless_metaclass_type;
@ -13,6 +14,7 @@ mod super_call_with_parameters;
mod type_of_primitive;
mod unnecessary_abspath;
mod unnecessary_future_import;
mod unnecessary_lru_cache_params;
mod use_pep585_annotation;
mod use_pep604_annotation;
mod useless_metaclass_type;

View File

@ -0,0 +1,14 @@
use rustpython_parser::ast::Expr;
use crate::check_ast::Checker;
use crate::pyupgrade::checks;
pub fn unnecessary_lru_cache_params(checker: &mut Checker, decorator_list: &[Expr]) {
if let Some(check) = checks::unnecessary_lru_cache_params(
decorator_list,
checker.settings.target_version,
checker.from_imports.get("functools"),
) {
checker.add_check(check);
}
}

View File

@ -0,0 +1,37 @@
---
source: src/linter.rs
expression: checks
---
- kind: UnnecessaryLRUCacheParams
location:
row: 5
column: 1
end_location:
row: 5
column: 12
fix: ~
- kind: UnnecessaryLRUCacheParams
location:
row: 11
column: 1
end_location:
row: 11
column: 22
fix: ~
- kind: UnnecessaryLRUCacheParams
location:
row: 16
column: 1
end_location:
row: 16
column: 24
fix: ~
- kind: UnnecessaryLRUCacheParams
location:
row: 21
column: 1
end_location:
row: 21
column: 34
fix: ~

View File

@ -0,0 +1,6 @@
---
source: src/linter.rs
expression: checks
---
[]