Skip to content

Commit

Permalink
Rename cache-max-size-none and check functools.cache (pylint-…
Browse files Browse the repository at this point in the history
…dev#6182)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
  • Loading branch information
2 people authored and Sam M W committed Apr 19, 2022
1 parent 4104287 commit bb55dea
Show file tree
Hide file tree
Showing 12 changed files with 136 additions and 30 deletions.
8 changes: 8 additions & 0 deletions ChangeLog
Expand Up @@ -141,6 +141,14 @@ Release date: TBA

Closes #5936

* The ``cache-max-size-none`` checker has been renamed to ``method-cache-max-size-none``.

Closes #5670

* The ``method-cache-max-size-none`` checker will now also check ``functools.cache``.

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
9 changes: 9 additions & 0 deletions doc/data/messages/m/method-cache-max-size-none/bad.py
@@ -0,0 +1,9 @@
import functools


class Fibonnaci:
@functools.lru_cache(maxsize=None) # [method-cache-max-size-none]
def fibonacci(self, n):
if n in {0, 1}:
return n
return self.fibonacci(n - 1) + self.fibonacci(n - 2)
13 changes: 13 additions & 0 deletions doc/data/messages/m/method-cache-max-size-none/good.py
@@ -0,0 +1,13 @@
import functools


@functools.cache
def cached_fibonacci(n):
if n in {0, 1}:
return n
return cached_fibonacci(n - 1) + cached_fibonacci(n - 2)


class Fibonnaci:
def fibonacci(self, n):
return cached_fibonacci(n)
8 changes: 8 additions & 0 deletions doc/whatsnew/2.14.rst
Expand Up @@ -176,6 +176,14 @@ Other Changes

* The concept of checker priority has been removed.

* The ``cache-max-size-none`` checker has been renamed to ``method-cache-max-size-none``.

Closes #5670

* The ``method-cache-max-size-none`` checker will now also check ``functools.cache``.

Closes #5670

* ``BaseChecker`` classes now require the ``linter`` argument to be passed.

* The ``set_config_directly`` decorator has been removed.
Expand Down
37 changes: 23 additions & 14 deletions pylint/checkers/stdlib.py
Expand Up @@ -420,15 +420,20 @@ class StdlibChecker(DeprecatedMixin, BaseChecker):
"Calls to breakpoint(), sys.breakpointhook() and pdb.set_trace() should be removed "
"from code that is not actively being debugged.",
),
"W1517": (
"'lru_cache(maxsize=None)' will keep all method args alive indefinitely, including 'self'",
"cache-max-size-none",
"By decorating a method with lru_cache the 'self' argument will be linked to "
"the lru_cache function and therefore never garbage collected. Unless your instance "
"W1518": (
"'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self'",
"method-cache-max-size-none",
"By decorating a method with lru_cache or cache the 'self' argument will be linked to "
"the 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 or add a maxsize to the cache."
"The default value for maxsize is 128.",
{"old_names": [("W1516", "lru-cache-decorating-method")]},
{
"old_names": [
("W1516", "lru-cache-decorating-method"),
("W1517", "cache-max-size-none"),
]
},
),
}

Expand Down Expand Up @@ -551,7 +556,7 @@ def visit_boolop(self, node: nodes.BoolOp) -> None:
for value in node.values:
self._check_datetime(value)

@utils.check_messages("cache-max-size-none")
@utils.check_messages("method-cache-max-size-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)
Expand All @@ -563,28 +568,32 @@ def _check_lru_cache_decorators(self, decorators: nodes.Decorators) -> None:
try:
for infered_node in d_node.infer():
q_name = infered_node.qname()
if q_name in NON_INSTANCE_METHODS or q_name not in LRU_CACHE:
if q_name in NON_INSTANCE_METHODS:
return

# Check if there is a maxsize argument set to None in the call
if isinstance(d_node, nodes.Call):
if q_name in LRU_CACHE and isinstance(d_node, nodes.Call):
try:
arg = utils.get_argument_from_call(
d_node, position=0, keyword="maxsize"
)
except utils.NoSuchArgumentError:
return
break

if not isinstance(arg, nodes.Const) or arg.value is not None:
return
break

lru_cache_nodes.append(d_node)
break
lru_cache_nodes.append(d_node)
break

if q_name == "functools.cache":
lru_cache_nodes.append(d_node)
break
except astroid.InferenceError:
pass
for lru_cache_node in lru_cache_nodes:
self.add_message(
"cache-max-size-none",
"method-cache-max-size-none",
node=lru_cache_node,
confidence=interfaces.INFERENCE,
)
Expand Down
2 changes: 1 addition & 1 deletion pylint/message/message_definition_store.py
Expand Up @@ -55,7 +55,7 @@ def register_message(self, message: MessageDefinition) -> None:
# and the arguments are relatively small in size we do not run the
# risk of creating a large memory leak.
# See discussion in: https://github.com/PyCQA/pylint/pull/5673
@functools.lru_cache(maxsize=None) # pylint: disable=cache-max-size-none
@functools.lru_cache(maxsize=None) # pylint: disable=method-cache-max-size-none
def get_message_definitions(self, msgid_or_symbol: str) -> list[MessageDefinition]:
"""Returns the Message definition for either a numeric or symbolic id.
Expand Down
7 changes: 0 additions & 7 deletions tests/functional/c/cache_max_size_none.txt

This file was deleted.

@@ -1,4 +1,4 @@
"""Tests for cache-max-size-none"""
"""Tests for method-cache-max-size-none"""
# pylint: disable=no-self-use, missing-function-docstring, reimported, too-few-public-methods
# pylint: disable=missing-class-docstring, function-redefined

Expand All @@ -22,25 +22,25 @@ def my_func(self, param):
def my_func(self, param):
return param + 1

@lru_cache(None) # [cache-max-size-none]
@lru_cache(None) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

@functools.lru_cache(None) # [cache-max-size-none]
@functools.lru_cache(None) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

@aliased_functools.lru_cache(None) # [cache-max-size-none]
@aliased_functools.lru_cache(None) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

@aliased_cache(None) # [cache-max-size-none]
@aliased_cache(None) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

# Check double decorating to check robustness of checker itself
@aliased_cache(None) # [cache-max-size-none]
@aliased_cache(None) # [cache-max-size-none]
@aliased_cache(None) # [method-cache-max-size-none]
@aliased_cache(None) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

Expand Down Expand Up @@ -70,6 +70,11 @@ def my_func(self, param):
def my_func(self, param):
return param + 1

@lru_cache(typed=True, maxsize=None) # [cache-max-size-none]
@lru_cache(typed=True, maxsize=None) # [method-cache-max-size-none]
def my_func(self, param):
return param + 1


@lru_cache(maxsize=None)
def my_func(param):
return param + 1
7 changes: 7 additions & 0 deletions tests/functional/m/method_cache_max_size_none.txt
@@ -0,0 +1,7 @@
method-cache-max-size-none:25:5:25:20:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:29:5:29:30:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:33:5:33:38:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:37:5:37:24:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:42:5:42:24:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:43:5:43:24:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:73:5:73:40:MyClassWithMethodsAndMaxSize.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
47 changes: 47 additions & 0 deletions tests/functional/m/method_cache_max_size_none_py39.py
@@ -0,0 +1,47 @@
"""Tests for method-cache-max-size-none"""
# 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 # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

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

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

@aliased_cache # [method-cache-max-size-none]
def my_func(self, param):
return param + 1

# Check double decorating to check robustness of checker itself
@functools.lru_cache(maxsize=1)
@aliased_cache # [method-cache-max-size-none]
def my_func(self, param):
return param + 1
2 changes: 2 additions & 0 deletions tests/functional/m/method_cache_max_size_none_py39.rc
@@ -0,0 +1,2 @@
[testoptions]
min_pyver = 3.9
5 changes: 5 additions & 0 deletions tests/functional/m/method_cache_max_size_none_py39.txt
@@ -0,0 +1,5 @@
method-cache-max-size-none:27:5:27:10:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:31:5:31:20:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:35:5:35:28:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:39:5:39:18:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE
method-cache-max-size-none:45:5:45:18:MyClassWithMethods.my_func:'lru_cache(maxsize=None)' or 'cache' will keep all method args alive indefinitely, including 'self':INFERENCE

0 comments on commit bb55dea

Please sign in to comment.