From 0fdcf1029cea2d43bd68c5270f48e0f7deab5e47 Mon Sep 17 00:00:00 2001 From: Andreu Vallbona Date: Sat, 8 Jun 2024 20:36:58 +0200 Subject: [PATCH] Fixed #22712 -- Avoided name shadowing of "all" in django.contrib.staticfiles.finders. Co-authored-by: Natalia <124304+nessita@users.noreply.github.com> --- AUTHORS | 1 + django/contrib/staticfiles/finders.py | 84 +++++++++++++++---- .../management/commands/findstatic.py | 2 +- docs/releases/5.2.txt | 3 +- tests/staticfiles_tests/test_finders.py | 82 +++++++++++++++++- 5 files changed, 155 insertions(+), 17 deletions(-) diff --git a/AUTHORS b/AUTHORS index bd86da5b57..ef5054fd11 100644 --- a/AUTHORS +++ b/AUTHORS @@ -82,6 +82,7 @@ answer newbie questions, and generally made Django that much better: Andreas Mock Andreas Pelme Andrés Torres Marroquín + Andreu Vallbona Plazas Andrew Brehaut Andrew Clark Andrew Durdin diff --git a/django/contrib/staticfiles/finders.py b/django/contrib/staticfiles/finders.py index 112a81d279..aca98b2b49 100644 --- a/django/contrib/staticfiles/finders.py +++ b/django/contrib/staticfiles/finders.py @@ -1,5 +1,6 @@ import functools import os +import warnings from django.apps import apps from django.conf import settings @@ -8,6 +9,7 @@ from django.core.checks import Error, Warning from django.core.exceptions import ImproperlyConfigured from django.core.files.storage import FileSystemStorage, Storage, default_storage from django.utils._os import safe_join +from django.utils.deprecation import RemovedInDjango60Warning from django.utils.functional import LazyObject, empty from django.utils.module_loading import import_string @@ -15,6 +17,32 @@ from django.utils.module_loading import import_string searched_locations = [] +# RemovedInDjango60Warning: When the deprecation ends, remove completely. +def _check_deprecated_find_param(class_name="", find_all=False, **kwargs): + method_name = "find" if not class_name else f"{class_name}.find" + if "all" in kwargs: + legacy_all = kwargs.pop("all") + msg = ( + "Passing the `all` argument to find() is deprecated. Use `find_all` " + "instead." + ) + warnings.warn(msg, RemovedInDjango60Warning, stacklevel=2) + + # If both `find_all` and `all` were given, raise TypeError. + if find_all is not False: + raise TypeError( + f"{method_name}() got multiple values for argument 'find_all'" + ) + + find_all = legacy_all + + if kwargs: # any remaining kwargs must be a TypeError + first = list(kwargs.keys()).pop() + raise TypeError(f"{method_name}() got an unexpected keyword argument '{first}'") + + return find_all + + class BaseFinder: """ A base file finder to be used for custom staticfiles finder classes. @@ -26,12 +54,20 @@ class BaseFinder: "configured correctly." ) - def find(self, path, all=False): + # RemovedInDjango60Warning: When the deprecation ends, remove completely. + def _check_deprecated_find_param(self, **kwargs): + return _check_deprecated_find_param( + class_name=self.__class__.__qualname__, **kwargs + ) + + # RemovedInDjango60Warning: When the deprecation ends, replace with: + # def find(self, path, find_all=False): + def find(self, path, find_all=False, **kwargs): """ Given a relative file path, find an absolute file path. - If the ``all`` parameter is False (default) return only the first found - file path; if True, return a list of all found files paths. + If the ``find_all`` parameter is False (default) return only the first + found file path; if True, return a list of all found files paths. """ raise NotImplementedError( "subclasses of BaseFinder must provide a find() method" @@ -113,17 +149,22 @@ class FileSystemFinder(BaseFinder): ) return errors - def find(self, path, all=False): + # RemovedInDjango60Warning: When the deprecation ends, replace with: + # def find(self, path, find_all=False): + def find(self, path, find_all=False, **kwargs): """ Look for files in the extra locations as defined in STATICFILES_DIRS. """ + # RemovedInDjango60Warning. + if kwargs: + find_all = self._check_deprecated_find_param(find_all=find_all, **kwargs) matches = [] for prefix, root in self.locations: if root not in searched_locations: searched_locations.append(root) matched_path = self.find_location(root, path, prefix) if matched_path: - if not all: + if not find_all: return matched_path matches.append(matched_path) return matches @@ -191,10 +232,15 @@ class AppDirectoriesFinder(BaseFinder): for path in utils.get_files(storage, ignore_patterns): yield path, storage - def find(self, path, all=False): + # RemovedInDjango60Warning: When the deprecation ends, replace with: + # def find(self, path, find_all=False): + def find(self, path, find_all=False, **kwargs): """ Look for files in the app directories. """ + # RemovedInDjango60Warning. + if kwargs: + find_all = self._check_deprecated_find_param(find_all=find_all, **kwargs) matches = [] for app in self.apps: app_location = self.storages[app].location @@ -202,7 +248,7 @@ class AppDirectoriesFinder(BaseFinder): searched_locations.append(app_location) match = self.find_in_app(app, path) if match: - if not all: + if not find_all: return match matches.append(match) return matches @@ -241,10 +287,15 @@ class BaseStorageFinder(BaseFinder): self.storage = self.storage() super().__init__(*args, **kwargs) - def find(self, path, all=False): + # RemovedInDjango60Warning: When the deprecation ends, replace with: + # def find(self, path, find_all=False): + def find(self, path, find_all=False, **kwargs): """ Look for files in the default file storage, if it's local. """ + # RemovedInDjango60Warning. + if kwargs: + find_all = self._check_deprecated_find_param(find_all=find_all, **kwargs) try: self.storage.path("") except NotImplementedError: @@ -254,7 +305,7 @@ class BaseStorageFinder(BaseFinder): searched_locations.append(self.storage.location) if self.storage.exists(path): match = self.storage.path(path) - if all: + if find_all: match = [match] return match return [] @@ -285,18 +336,23 @@ class DefaultStorageFinder(BaseStorageFinder): ) -def find(path, all=False): +# RemovedInDjango60Warning: When the deprecation ends, replace with: +# def find(path, find_all=False): +def find(path, find_all=False, **kwargs): """ Find a static file with the given path using all enabled finders. - If ``all`` is ``False`` (default), return the first matching + If ``find_all`` is ``False`` (default), return the first matching absolute path (or ``None`` if no match). Otherwise return a list. """ + # RemovedInDjango60Warning. + if kwargs: + find_all = _check_deprecated_find_param(find_all=find_all, **kwargs) searched_locations[:] = [] matches = [] for finder in get_finders(): - result = finder.find(path, all=all) - if not all and result: + result = finder.find(path, find_all=find_all) + if not find_all and result: return result if not isinstance(result, (list, tuple)): result = [result] @@ -304,7 +360,7 @@ def find(path, all=False): if matches: return matches # No match. - return [] if all else None + return [] if find_all else None def get_finders(): diff --git a/django/contrib/staticfiles/management/commands/findstatic.py b/django/contrib/staticfiles/management/commands/findstatic.py index 97413a64af..1caebf8fa9 100644 --- a/django/contrib/staticfiles/management/commands/findstatic.py +++ b/django/contrib/staticfiles/management/commands/findstatic.py @@ -19,7 +19,7 @@ class Command(LabelCommand): def handle_label(self, path, **options): verbosity = options["verbosity"] - result = finders.find(path, all=options["all"]) + result = finders.find(path, find_all=options["all"]) if verbosity >= 2: searched_locations = ( "\nLooking in the following locations:\n %s" diff --git a/docs/releases/5.2.txt b/docs/releases/5.2.txt index 8b77ecc482..bdc5349368 100644 --- a/docs/releases/5.2.txt +++ b/docs/releases/5.2.txt @@ -279,4 +279,5 @@ Features deprecated in 5.2 Miscellaneous ------------- -* ... +* The ``all`` argument for the ``django.contrib.staticfiles.finders.find()`` + function is deprecated in favor of the ``find_all`` argument. diff --git a/tests/staticfiles_tests/test_finders.py b/tests/staticfiles_tests/test_finders.py index 9f2509d533..28870e6fbe 100644 --- a/tests/staticfiles_tests/test_finders.py +++ b/tests/staticfiles_tests/test_finders.py @@ -4,10 +4,15 @@ from django.conf import settings from django.contrib.staticfiles import finders, storage from django.core.exceptions import ImproperlyConfigured from django.test import SimpleTestCase, override_settings +from django.utils.deprecation import RemovedInDjango60Warning from .cases import StaticFilesTestCase from .settings import TEST_ROOT +DEPRECATION_MSG = ( + "Passing the `all` argument to find() is deprecated. Use `find_all` instead." +) + class TestFinders: """ @@ -25,11 +30,49 @@ class TestFinders: def test_find_all(self): src, dst = self.find_all - found = self.finder.find(src, all=True) + found = self.finder.find(src, find_all=True) found = [os.path.normcase(f) for f in found] dst = [os.path.normcase(d) for d in dst] self.assertEqual(found, dst) + def test_find_all_deprecated_param(self): + src, dst = self.find_all + with self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG): + found = self.finder.find(src, all=True) + found = [os.path.normcase(f) for f in found] + dst = [os.path.normcase(d) for d in dst] + self.assertEqual(found, dst) + + def test_find_all_conflicting_params(self): + src, dst = self.find_all + msg = ( + f"{self.finder.__class__.__qualname__}.find() got multiple values for " + "argument 'find_all'" + ) + with ( + self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG), + self.assertRaisesMessage(TypeError, msg), + ): + self.finder.find(src, find_all=True, all=True) + + def test_find_all_unexpected_params(self): + src, dst = self.find_all + msg = ( + f"{self.finder.__class__.__qualname__}.find() got an unexpected keyword " + "argument 'wrong'" + ) + with ( + self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG), + self.assertRaisesMessage(TypeError, msg), + ): + self.finder.find(src, all=True, wrong=1) + + with self.assertRaisesMessage(TypeError, msg): + self.finder.find(src, find_all=True, wrong=1) + + with self.assertRaisesMessage(TypeError, msg): + self.finder.find(src, wrong=1) + class TestFileSystemFinder(TestFinders, StaticFilesTestCase): """ @@ -114,6 +157,43 @@ class TestMiscFinder(SimpleTestCase): [os.path.join(TEST_ROOT, "project", "documents")], ) + def test_searched_locations_find_all(self): + finders.find("spam", find_all=True) + self.assertEqual( + finders.searched_locations, + [os.path.join(TEST_ROOT, "project", "documents")], + ) + + def test_searched_locations_deprecated_all(self): + with self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG): + finders.find("spam", all=True) + self.assertEqual( + finders.searched_locations, + [os.path.join(TEST_ROOT, "project", "documents")], + ) + + def test_searched_locations_conflicting_params(self): + msg = "find() got multiple values for argument 'find_all'" + with ( + self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG), + self.assertRaisesMessage(TypeError, msg), + ): + finders.find("spam", find_all=True, all=True) + + def test_searched_locations_unexpected_params(self): + msg = "find() got an unexpected keyword argument 'wrong'" + with ( + self.assertWarnsMessage(RemovedInDjango60Warning, DEPRECATION_MSG), + self.assertRaisesMessage(TypeError, msg), + ): + finders.find("spam", all=True, wrong=1) + + with self.assertRaisesMessage(TypeError, msg): + finders.find("spam", find_all=True, wrong=1) + + with self.assertRaisesMessage(TypeError, msg): + finders.find("spam", wrong=1) + @override_settings(MEDIA_ROOT="") def test_location_empty(self): msg = (