Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename cache-max-size-none and check functools.cache #6182

Merged
merged 7 commits into from Apr 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions ChangeLog
Expand Up @@ -137,6 +137,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 @@ -172,6 +172,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