Implement B019

astral-sh · Nov 12, 2022 · adafe58 · adafe58
1 parent a21fe71
commit adafe58
Show file tree

Hide file tree

Showing 9 changed files with 234 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,11 @@ 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
@@ -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
@@ -0,0 +1,32 @@
+use rustpython_ast::Expr;
+
+use crate::ast::helpers::compose_call_path;
+use crate::ast::types::Range;
+use crate::ast::types::ScopeKind;
+use crate::check_ast::Checker;
+use crate::checks::{Check, CheckKind};
+
+const CACHE_FUNCTIONS: [&str; 4] = [
+    "functools.lru_cache",
+    "functools.cache",
+    "lru_cache",
+    "cache",
+];
+
+pub fn cached_instance_method(checker: &mut Checker, decorator_list: &[Expr]) {
+    if matches!(checker.current_scope().kind, ScopeKind::Class(_)) {
+        for decorator in decorator_list {
+            if let Some(decorator_path) = compose_call_path(decorator) {
+                if decorator_path == "classmethod" || decorator_path == "staticmethod" {
+                    return;
+                }
+                if CACHE_FUNCTIONS.contains(&decorator_path.as_str()) {
+                    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
@@ -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
@@ -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
@@ -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: ~
+