Let memoize's ignore set match a positional arg by name

ignore={'session'} only worked when session was passed as a keyword.
Call the same function with session passed positionally instead and
diskcache tried to build a cache key that included the raw argument,
which is exactly the footgun described in #240: a caller has to know
whether they always call with keywords, or ignore has to spell out the
positional index too.

memoize() now grabs the wrapped function's parameter names once via
inspect.signature and passes them through to args_to_key, which checks
a positional argument's name against ignore in addition to its index.
Keyword matching was already fine and is untouched. args_to_key's new
arg_names parameter defaults to an empty tuple, so the two other
callers (DjangoCache.memoize, the memoize_stampede recipe) keep their
existing index-only behavior unless they're updated separately.

Added a regression test using an unpicklable value for the ignored
argument, so a caller passing it positionally raises immediately if it
leaks into the key instead of silently caching under a wrong key.

Author: afonsojanu

diskcache/core.py

@@ -5,6 +5,7 @@
 import contextlib as cl
 import errno
 import functools as ft
+import inspect
 import io
 import json
 import os
@@ -384,18 +385,26 @@ class EmptyDirWarning(UserWarning):
     """Warning used by Cache.check for empty directories."""
 
 
-def args_to_key(base, args, kwargs, typed, ignore):
+def args_to_key(base, args, kwargs, typed, ignore, arg_names=()):
     """Create cache key out of function arguments.
 
     :param tuple base: base of key
     :param tuple args: function arguments
     :param dict kwargs: function keyword arguments
     :param bool typed: include types in cache key
     :param set ignore: positional or keyword args to ignore
+    :param tuple arg_names: names of the wrapped function's positional
+        parameters, in order, so a name in `ignore` matches a positional
+        argument the same way it matches a keyword argument (default ())
     :return: cache key tuple
 
     """
-    args = tuple(arg for index, arg in enumerate(args) if index not in ignore)
+    args = tuple(
+        arg
+        for index, arg in enumerate(args)
+        if index not in ignore
+        and not (index < len(arg_names) and arg_names[index] in ignore)
+    )
     key = base + args + (None,)
 
     if kwargs:
@@ -1853,7 +1862,10 @@ def memoize(
         :param float expire: seconds until arguments expire
             (default None, no expiry)
         :param str tag: text to associate with arguments (default None)
-        :param set ignore: positional or keyword args to ignore (default ())
+        :param set ignore: positional or keyword args to ignore (default ()).
+            A parameter's name in this set is matched whether it was passed
+            positionally or by keyword; a positional index is also still
+            matched by position, same as before.
         :return: callable decorator
 
         """
@@ -1865,6 +1877,14 @@ def decorator(func):
             """Decorator created by memoize() for callable `func`."""
             base = (full_name(func),) if name is None else (name,)
 
+            try:
+                arg_names = tuple(inspect.signature(func).parameters)
+            except (TypeError, ValueError):
+                # Some callables (e.g. certain builtins) don't expose a
+                # signature; fall back to matching `ignore` by position only,
+                # same as before this parameter-name lookup was added.
+                arg_names = ()
+
             @ft.wraps(func)
             def wrapper(*args, **kwargs):
                 """Wrapper for callable to cache arguments and return values."""
@@ -1880,7 +1900,7 @@ def wrapper(*args, **kwargs):
 
             def __cache_key__(*args, **kwargs):
                 """Make key for cache given function arguments."""
-                return args_to_key(base, args, kwargs, typed, ignore)
+                return args_to_key(base, args, kwargs, typed, ignore, arg_names)
 
             wrapper.__cache_key__ = __cache_key__
             return wrapper

tests/test_core.py

@@ -1393,6 +1393,20 @@ def test(*args, **kwargs):
     assert cache.stats() == (2, 1)
 
 
+def test_memoize_ignore_by_name_regardless_of_call_style(cache):
+    # Regression test for GH #240: a name in `ignore` should be honored
+    # whether the caller passes that argument positionally or by keyword.
+    # `session` is deliberately something that can't be pickled, so if it
+    # ever leaks into the cache key this raises instead of quietly caching
+    # under two different keys.
+    @cache.memoize(ignore={'session'})
+    def get(entity_id, session):
+        return entity_id
+
+    assert get('a', session=threading.Lock())
+    assert get('b', threading.Lock())
+
+
 def test_memoize_iter(cache):
     @cache.memoize()
     def test(*args, **kwargs):