Skip to content

Commit

Permalink
Add functools-cache-decorating-method checker
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielNoord committed Apr 4, 2022
1 parent 7a9234f commit 6fdba83
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 0 deletions.
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ Release date: TBA

Closes #5936

* Added ``functools-cache-decorating-method`` checker with checks for the use of ``functools.cache``
on class methods. This is unrecommended as it creates memory leaks by never letting the instance
getting garbage collected.

Closes #5670

* ``potential-index-error``: Emitted when the index of a list or tuple exceeds its length.
This checker is currently quite conservative to avoid false positives. We welcome
suggestions for improvements.
Expand Down
6 changes: 6 additions & 0 deletions doc/whatsnew/2.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ New checkers

Closes #4525

* Added ``functools-cache-decorating-method`` checker with checks for the use of ``functools.cache``
on class methods. This is unrecommended as it creates memory leaks by never letting the instance
getting garbage collected.

Closes #5670

* Added new message called ``duplicate-value`` which identifies duplicate values inside sets.

Closes #5880
Expand Down
30 changes: 30 additions & 0 deletions pylint/checkers/stdlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,14 @@ class StdlibChecker(DeprecatedMixin, BaseChecker):
"will never need to be garbage collected (singleton) it is recommended to refactor "
"code to avoid this pattern or add a maxsize to the cache.",
),
"W1517": (
"'cache' will keep all method args alive indefinitely, including 'self'",
"functools-cache-decorating-method",
"By decorating a method with cache the 'self' argument will be linked to "
"the cache function and therefore never garbage collected. Unless your instance "
"will never need to be garbage collected (singleton) it is recommended to refactor "
"code to avoid this pattern.",
),
}

def __init__(self, linter: Optional["PyLinter"] = None) -> None:
Expand Down Expand Up @@ -565,6 +573,7 @@ def visit_boolop(self, node: nodes.BoolOp) -> None:
def visit_functiondef(self, node: nodes.FunctionDef) -> None:
if node.decorators and isinstance(node.parent, nodes.ClassDef):
self._check_lru_cache_decorators(node.decorators)
self._check_cache_decorators(node.decorators)

def _check_lru_cache_decorators(self, decorators: nodes.Decorators) -> None:
"""Check if instance methods are decorated with functools.lru_cache."""
Expand Down Expand Up @@ -599,6 +608,27 @@ def _check_lru_cache_decorators(self, decorators: nodes.Decorators) -> None:
confidence=interfaces.INFERENCE,
)

def _check_cache_decorators(self, decorators: nodes.Decorators) -> None:
"""Check if instance methods are decorated with functools.cache."""
cache_nodes: List[nodes.NodeNG] = []
for d_node in decorators.nodes:
try:
for infered_node in d_node.infer():
q_name = infered_node.qname()
if q_name in NON_INSTANCE_METHODS or q_name != "functools.cache":
return

cache_nodes.append(d_node)
break
except astroid.InferenceError:
pass
for cache_node in cache_nodes:
self.add_message(
"functools-cache-decorating-method",
node=cache_node,
confidence=interfaces.INFERENCE,
)

def _check_redundant_assert(self, node, infer):
if (
isinstance(infer, astroid.BoundMethod)
Expand Down
47 changes: 47 additions & 0 deletions tests/functional/f/functools_cache_decorating_method.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""Tests for functools-cache-decorating-method"""
# pylint: disable=no-self-use, missing-function-docstring, reimported, too-few-public-methods
# pylint: disable=missing-class-docstring, function-redefined

import functools
import functools as aliased_functools
from functools import cache
from functools import cache as aliased_cache


@cache
def my_func(param):
return param + 1


class MyClassWithMethods:
@cache
@staticmethod
def my_func(param):
return param + 1

@cache
@classmethod
def my_func(cls, param):
return param + 1

@cache # [functools-cache-decorating-method]
def my_func(self, param):
return param + 1

@functools.cache # [functools-cache-decorating-method]
def my_func(self, param):
return param + 1

@aliased_functools.cache # [functools-cache-decorating-method]
def my_func(self, param):
return param + 1

@aliased_cache # [functools-cache-decorating-method]
def my_func(self, param):
return param + 1

# Check double decorating to check robustness of checker itself
@aliased_cache # [functools-cache-decorating-method]
@aliased_cache # [functools-cache-decorating-method]
def my_func(self, param):
return param + 1
2 changes: 2 additions & 0 deletions tests/functional/f/functools_cache_decorating_method.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[testoptions]
min_pyver = 3.9
6 changes: 6 additions & 0 deletions tests/functional/f/functools_cache_decorating_method.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
functools-cache-decorating-method:27:5:27:10:MyClassWithMethods.my_func:'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
functools-cache-decorating-method:31:5:31:20:MyClassWithMethods.my_func:'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
functools-cache-decorating-method:35:5:35:28:MyClassWithMethods.my_func:'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
functools-cache-decorating-method:39:5:39:18:MyClassWithMethods.my_func:'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
functools-cache-decorating-method:44:5:44:18:MyClassWithMethods.my_func:'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
functools-cache-decorating-method:45:5:45:18:MyClassWithMethods.my_func:'cache' will keep all method args alive indefinitely, including 'self':INFERENCE

0 comments on commit 6fdba83

Please sign in to comment.