diff --git a/README.md b/README.md index fa3a70b665..8d2842f52f 100644 --- a/README.md +++ b/README.md @@ -511,6 +511,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/ | B016 | CannotRaiseLiteral | Cannot raise a literal. Did you intend to return it or raise an Exception? | | | B017 | NoAssertRaisesException | `assertRaises(Exception):` should be considered evil. | | | B018 | UselessExpression | Found useless expression. Either assign it to a variable or remove it. | | +| B019 | CachedInstanceMethod | Use of `functools.lru_cache` or `functools.cache` on methods can lead to memory leaks. | | | B025 | DuplicateTryBlockException | try-except block with duplicate exception `Exception` | | | B026 | StarArgUnpackingAfterKeywordArg | Star-arg unpacking after a keyword argument is strongly discouraged. | | @@ -684,7 +685,7 @@ including: - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/) - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) -- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (20/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (21/32) - [`flake8-2020`](https://pypi.org/project/flake8-2020/) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) @@ -708,7 +709,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/) - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) -- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (20/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (21/32) - [`flake8-2020`](https://pypi.org/project/flake8-2020/) Ruff can also replace [`isort`](https://pypi.org/project/isort/), [`yesqa`](https://github.com/asottile/yesqa), diff --git a/resources/test/fixtures/B019.py b/resources/test/fixtures/B019.py new file mode 100644 index 0000000000..55d32aa1ba --- /dev/null +++ b/resources/test/fixtures/B019.py @@ -0,0 +1,108 @@ +""" +Should emit: +B019 - on lines 73, 77, 81, 85, 89, 93, 97, 101 +""" +import functools +from functools import cache, cached_property, lru_cache + + +def some_other_cache(): + ... + + +@functools.cache +def compute_func(self, y): + ... + + +class Foo: + def __init__(self, x): + self.x = x + + def compute_method(self, y): + ... + + @some_other_cache + def user_cached_instance_method(self, y): + ... + + @classmethod + @functools.cache + def cached_classmethod(cls, y): + ... + + @classmethod + @cache + def other_cached_classmethod(cls, y): + ... + + @classmethod + @functools.lru_cache + def lru_cached_classmethod(cls, y): + ... + + @classmethod + @lru_cache + def other_lru_cached_classmethod(cls, y): + ... + + @staticmethod + @functools.cache + def cached_staticmethod(y): + ... + + @staticmethod + @cache + def other_cached_staticmethod(y): + ... + + @staticmethod + @functools.lru_cache + def lru_cached_staticmethod(y): + ... + + @staticmethod + @lru_cache + def other_lru_cached_staticmethod(y): + ... + + @functools.cached_property + def some_cached_property(self): + ... + + @cached_property + def some_other_cached_property(self): + ... + + # Remaining methods should emit B019 + @functools.cache + def cached_instance_method(self, y): + ... + + @cache + def another_cached_instance_method(self, y): + ... + + @functools.cache() + def called_cached_instance_method(self, y): + ... + + @cache() + def another_called_cached_instance_method(self, y): + ... + + @functools.lru_cache + def lru_cached_instance_method(self, y): + ... + + @lru_cache + def another_lru_cached_instance_method(self, y): + ... + + @functools.lru_cache() + def called_lru_cached_instance_method(self, y): + ... + + @lru_cache() + def another_called_lru_cached_instance_method(self, y): + ... diff --git a/src/check_ast.rs b/src/check_ast.rs index ede664ac3f..0caf1f9b6a 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -346,6 +346,9 @@ where if self.settings.enabled.contains(&CheckCode::B018) { flake8_bugbear::plugins::useless_expression(self, body); } + if self.settings.enabled.contains(&CheckCode::B019) { + flake8_bugbear::plugins::cached_instance_method(self, decorator_list); + } self.check_builtin_shadowing(name, Range::from_located(stmt), true); diff --git a/src/checks.rs b/src/checks.rs index f8e9a8e601..a4451b7a3a 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -93,6 +93,7 @@ pub enum CheckCode { B016, B017, B018, + B019, B025, B026, // flake8-comprehensions @@ -380,6 +381,7 @@ pub enum CheckKind { CannotRaiseLiteral, NoAssertRaisesException, UselessExpression, + CachedInstanceMethod, DuplicateTryBlockException(String), StarArgUnpackingAfterKeywordArg, // flake8-comprehensions @@ -610,6 +612,7 @@ impl CheckCode { CheckCode::B016 => CheckKind::CannotRaiseLiteral, CheckCode::B017 => CheckKind::NoAssertRaisesException, CheckCode::B018 => CheckKind::UselessExpression, + CheckCode::B019 => CheckKind::CachedInstanceMethod, CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()), CheckCode::B026 => CheckKind::StarArgUnpackingAfterKeywordArg, // flake8-comprehensions @@ -841,6 +844,7 @@ impl CheckCode { CheckCode::B016 => CheckCategory::Flake8Bugbear, CheckCode::B017 => CheckCategory::Flake8Bugbear, CheckCode::B018 => CheckCategory::Flake8Bugbear, + CheckCode::B019 => CheckCategory::Flake8Bugbear, CheckCode::B025 => CheckCategory::Flake8Bugbear, CheckCode::B026 => CheckCategory::Flake8Bugbear, CheckCode::C400 => CheckCategory::Flake8Comprehensions, @@ -1036,6 +1040,7 @@ impl CheckKind { CheckKind::CannotRaiseLiteral => &CheckCode::B016, CheckKind::NoAssertRaisesException => &CheckCode::B017, CheckKind::UselessExpression => &CheckCode::B018, + CheckKind::CachedInstanceMethod => &CheckCode::B019, CheckKind::DuplicateTryBlockException(_) => &CheckCode::B025, CheckKind::StarArgUnpackingAfterKeywordArg => &CheckCode::B026, // flake8-comprehensions @@ -1388,6 +1393,9 @@ impl CheckKind { CheckKind::UselessExpression => { "Found useless expression. Either assign it to a variable or remove it.".to_string() } + CheckKind::CachedInstanceMethod => "Use of `functools.lru_cache` or `functools.cache` \ + on methods can lead to memory leaks." + .to_string(), CheckKind::DuplicateTryBlockException(name) => { format!("try-except block with duplicate exception `{name}`") } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index a45d6ed14c..daf0409ce0 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -53,6 +53,7 @@ pub enum CheckCodePrefix { B016, B017, B018, + B019, B02, B025, B026, @@ -368,6 +369,7 @@ impl CheckCodePrefix { CheckCode::B016, CheckCode::B017, CheckCode::B018, + CheckCode::B019, CheckCode::B025, CheckCode::B026, ], @@ -388,6 +390,7 @@ impl CheckCodePrefix { CheckCode::B016, CheckCode::B017, CheckCode::B018, + CheckCode::B019, CheckCode::B025, CheckCode::B026, ], @@ -418,6 +421,7 @@ impl CheckCodePrefix { CheckCode::B016, CheckCode::B017, CheckCode::B018, + CheckCode::B019, ], CheckCodePrefix::B010 => vec![CheckCode::B010], CheckCodePrefix::B011 => vec![CheckCode::B011], @@ -427,6 +431,7 @@ impl CheckCodePrefix { CheckCodePrefix::B016 => vec![CheckCode::B016], CheckCodePrefix::B017 => vec![CheckCode::B017], CheckCodePrefix::B018 => vec![CheckCode::B018], + CheckCodePrefix::B019 => vec![CheckCode::B019], CheckCodePrefix::B02 => vec![CheckCode::B025, CheckCode::B026], CheckCodePrefix::B025 => vec![CheckCode::B025], CheckCodePrefix::B026 => vec![CheckCode::B026], @@ -1134,6 +1139,7 @@ impl CheckCodePrefix { CheckCodePrefix::B016 => PrefixSpecificity::Explicit, CheckCodePrefix::B017 => PrefixSpecificity::Explicit, CheckCodePrefix::B018 => PrefixSpecificity::Explicit, + CheckCodePrefix::B019 => PrefixSpecificity::Explicit, CheckCodePrefix::B02 => PrefixSpecificity::Tens, CheckCodePrefix::B025 => PrefixSpecificity::Explicit, CheckCodePrefix::B026 => PrefixSpecificity::Explicit, diff --git a/src/flake8_bugbear/plugins/cached_instance_method.rs b/src/flake8_bugbear/plugins/cached_instance_method.rs new file mode 100644 index 0000000000..e7b856ebde --- /dev/null +++ b/src/flake8_bugbear/plugins/cached_instance_method.rs @@ -0,0 +1,49 @@ +use rustpython_ast::{Expr, ExprKind}; + +use crate::ast::helpers::{compose_call_path, match_name_or_attr_from_module}; +use crate::ast::types::{Range, ScopeKind}; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; + +fn is_cache_func(checker: &Checker, expr: &Expr) -> bool { + match_name_or_attr_from_module( + expr, + "lru_cache", + "functools", + checker.from_imports.get("functools"), + ) || match_name_or_attr_from_module( + expr, + "cache", + "functools", + checker.from_imports.get("functools"), + ) +} + +/// B019 +pub fn cached_instance_method(checker: &mut Checker, decorator_list: &[Expr]) { + if matches!(checker.current_scope().kind, ScopeKind::Class(_)) { + for decorator in decorator_list { + // TODO(charlie): This should take into account `classmethod-decorators` and + // `staticmethod-decorators`. + if let Some(decorator_path) = compose_call_path(decorator) { + if decorator_path == "classmethod" || decorator_path == "staticmethod" { + return; + } + } + } + for decorator in decorator_list { + if is_cache_func( + checker, + match &decorator.node { + ExprKind::Call { func, .. } => func, + _ => decorator, + }, + ) { + checker.add_check(Check::new( + CheckKind::CachedInstanceMethod, + Range::from_located(decorator), + )); + } + } + } +} diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 2b2781690b..1860767250 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/src/flake8_bugbear/plugins/mod.rs @@ -1,6 +1,7 @@ pub use assert_false::assert_false; pub use assert_raises_exception::assert_raises_exception; pub use assignment_to_os_environ::assignment_to_os_environ; +pub use cached_instance_method::cached_instance_method; pub use cannot_raise_literal::cannot_raise_literal; pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exceptions}; pub use function_call_argument_default::function_call_argument_default; @@ -19,6 +20,7 @@ pub use useless_expression::useless_expression; mod assert_false; mod assert_raises_exception; mod assignment_to_os_environ; +mod cached_instance_method; mod cannot_raise_literal; mod duplicate_exceptions; mod function_call_argument_default; diff --git a/src/linter.rs b/src/linter.rs index 4350b1c47c..2351893f42 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -342,6 +342,7 @@ mod tests { #[test_case(CheckCode::B016, Path::new("B016.py"); "B016")] #[test_case(CheckCode::B017, Path::new("B017.py"); "B017")] #[test_case(CheckCode::B018, Path::new("B018.py"); "B018")] + #[test_case(CheckCode::B019, Path::new("B019.py"); "B019")] #[test_case(CheckCode::B025, Path::new("B025.py"); "B025")] #[test_case(CheckCode::B026, Path::new("B026.py"); "B026")] #[test_case(CheckCode::C400, Path::new("C400.py"); "C400")] diff --git a/src/snapshots/ruff__linter__tests__B019_B019.py.snap b/src/snapshots/ruff__linter__tests__B019_B019.py.snap new file mode 100644 index 0000000000..7980a548ef --- /dev/null +++ b/src/snapshots/ruff__linter__tests__B019_B019.py.snap @@ -0,0 +1,69 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: CachedInstanceMethod + location: + row: 78 + column: 5 + end_location: + row: 78 + column: 20 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 82 + column: 5 + end_location: + row: 82 + column: 10 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 86 + column: 5 + end_location: + row: 86 + column: 22 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 90 + column: 5 + end_location: + row: 90 + column: 12 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 94 + column: 5 + end_location: + row: 94 + column: 24 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 98 + column: 5 + end_location: + row: 98 + column: 14 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 102 + column: 5 + end_location: + row: 102 + column: 26 + fix: ~ +- kind: CachedInstanceMethod + location: + row: 106 + column: 5 + end_location: + row: 106 + column: 16 + fix: ~ +