Fixed #27661 -- Moved FileSystemFinder's ImproperlyConfigured exceptions to system checks.

Thanks Simon Charette, Mariusz Felisiak, Tim Graham, and Adam Johnson
for review.

6 files changed, 133 insertions, 17 deletions

diff --git a/django/contrib/staticfiles/apps.py b/django/contrib/staticfiles/apps.py
index 0911c0113d..65865da0d2 100644
--- a/django/contrib/staticfiles/apps.py
+++ b/django/contrib/staticfiles/apps.py
@@ -1,4 +1,6 @@
 from django.apps import AppConfig
+from django.contrib.staticfiles.checks import check_finders
+from django.core import checks
 from django.utils.translation import ugettext_lazy as _
 
 
@@ -6,3 +8,6 @@ class StaticFilesConfig(AppConfig):
     name = 'django.contrib.staticfiles'
     verbose_name = _("Static Files")
     ignore_patterns = ['CVS', '.*', '*~']
+
+    def ready(self):
+        checks.register(check_finders, 'staticfiles')
diff --git a/django/contrib/staticfiles/checks.py b/django/contrib/staticfiles/checks.py
new file mode 100644
index 0000000000..fb57bf726d
--- /dev/null
+++ b/django/contrib/staticfiles/checks.py
@@ -0,0 +1,14 @@
+from django.contrib.staticfiles.finders import get_finders
+
+
+def check_finders(app_configs=None, **kwargs):
+    """Check all registered staticfiles finders."""
+    errors = []
+    for finder in get_finders():
+        try:
+            finder_errors = finder.check()
+        except NotImplementedError:
+            pass
+        else:
+            errors.extend(finder_errors)
+    return errors
diff --git a/django/contrib/staticfiles/finders.py b/django/contrib/staticfiles/finders.py
index fcb31c8547..76029b33b6 100644
--- a/django/contrib/staticfiles/finders.py
+++ b/django/contrib/staticfiles/finders.py
@@ -5,6 +5,7 @@ from collections import OrderedDict
 from django.apps import apps
 from django.conf import settings
 from django.contrib.staticfiles import utils
+from django.core.checks import Error
 from django.core.exceptions import ImproperlyConfigured
 from django.core.files.storage import (
     FileSystemStorage, Storage, default_storage,
@@ -21,6 +22,11 @@ class BaseFinder:
     """
     A base file finder to be used for custom staticfiles finder classes.
     """
+    def check(self, **kwargs):
+        raise NotImplementedError(
+            'subclasses may provide a check() method to verify the finder is '
+            'configured correctly.'
+        )
 
     def find(self, path, all=False):
         """
@@ -52,19 +58,11 @@ class FileSystemFinder(BaseFinder):
         self.locations = []
         # Maps dir paths to an appropriate storage instance
         self.storages = OrderedDict()
-        if not isinstance(settings.STATICFILES_DIRS, (list, tuple)):
-            raise ImproperlyConfigured(
-                "Your STATICFILES_DIRS setting is not a tuple or list; "
-                "perhaps you forgot a trailing comma?")
         for root in settings.STATICFILES_DIRS:
             if isinstance(root, (list, tuple)):
                 prefix, root = root
             else:
                 prefix = ''
-            if settings.STATIC_ROOT and os.path.abspath(settings.STATIC_ROOT) == os.path.abspath(root):
-                raise ImproperlyConfigured(
-                    "The STATICFILES_DIRS setting should "
-                    "not contain the STATIC_ROOT setting")
             if (prefix, root) not in self.locations:
                 self.locations.append((prefix, root))
         for prefix, root in self.locations:
@@ -73,6 +71,25 @@ class FileSystemFinder(BaseFinder):
             self.storages[root] = filesystem_storage
         super().__init__(*args, **kwargs)
 
+    def check(self, **kwargs):
+        errors = []
+        if not isinstance(settings.STATICFILES_DIRS, (list, tuple)):
+            errors.append(Error(
+                'The STATICFILES_DIRS setting is not a tuple or list.',
+                hint='Perhaps you forgot a trailing comma?',
+                id='staticfiles.E001',
+            ))
+        for root in settings.STATICFILES_DIRS:
+            if isinstance(root, (list, tuple)):
+                _, root = root
+            if settings.STATIC_ROOT and os.path.abspath(settings.STATIC_ROOT) == os.path.abspath(root):
+                errors.append(Error(
+                    'The STATICFILES_DIRS setting should not contain the '
+                    'STATIC_ROOT setting.',
+                    id='staticfiles.E002',
+                ))
+        return errors
+
     def find(self, path, all=False):
         """
         Looks for files in the extra locations
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index 518ed507af..35c0bd6987 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -83,6 +83,7 @@ Django's system checks are organized using the following tags:
 * ``models``: Checks of model, field, and manager definitions.
 * ``security``: Checks security related configuration.
 * ``signals``: Checks on signal declarations and handler registrations.
+* ``staticfiles``: Checks :mod:`django.contrib.staticfiles` configuration.
 * ``templates``: Checks template related configuration.
 * ``urls``: Checks URL configuration.
 
@@ -675,3 +676,14 @@ The following checks are performed on any model using a
   ``<field name>``.
 * **sites.E002**: ``CurrentSiteManager`` cannot use ``<field>`` as it is not a
   foreign key or a many-to-many field.
+
+``staticfiles``
+---------------
+
+The following checks verify that :mod:`django.contrib.staticfiles` is correctly
+configured:
+
+* **staticfiles.E001**: The :setting:`STATICFILES_DIRS` setting is not a tuple
+  or list.
+* **staticfiles.E002**: The :setting:`STATICFILES_DIRS` setting should not
+  contain the :setting:`STATIC_ROOT` setting.
diff --git a/tests/staticfiles_tests/test_checks.py b/tests/staticfiles_tests/test_checks.py
new file mode 100644
index 0000000000..0fe432b5c7
--- /dev/null
+++ b/tests/staticfiles_tests/test_checks.py
@@ -0,0 +1,77 @@
+from unittest import mock
+
+from django.conf import settings
+from django.contrib.staticfiles.checks import check_finders
+from django.contrib.staticfiles.finders import BaseFinder
+from django.core.checks import Error
+from django.test import SimpleTestCase, override_settings
+
+
+class FindersCheckTests(SimpleTestCase):
+
+    def test_base_finder_check_not_implemented(self):
+        finder = BaseFinder()
+        msg = 'subclasses may provide a check() method to verify the finder is configured correctly.'
+        with self.assertRaisesMessage(NotImplementedError, msg):
+            finder.check()
+
+    def test_check_finders(self):
+        """check_finders() concatenates all errors."""
+        error1 = Error('1')
+        error2 = Error('2')
+        error3 = Error('3')
+
+        def get_finders():
+            class Finder1(BaseFinder):
+                def check(self, **kwargs):
+                    return [error1]
+
+            class Finder2(BaseFinder):
+                def check(self, **kwargs):
+                    return []
+
+            class Finder3(BaseFinder):
+                def check(self, **kwargs):
+                    return [error2, error3]
+
+            class Finder4(BaseFinder):
+                pass
+
+            return [Finder1(), Finder2(), Finder3(), Finder4()]
+
+        with mock.patch('django.contrib.staticfiles.checks.get_finders', get_finders):
+            errors = check_finders(None)
+            self.assertEqual(errors, [error1, error2, error3])
+
+    def test_no_errors_with_test_settings(self):
+        self.assertEqual(check_finders(None), [])
+
+    @override_settings(STATICFILES_DIRS='a string')
+    def test_dirs_not_tuple_or_list(self):
+        self.assertEqual(check_finders(None), [
+            Error(
+                'The STATICFILES_DIRS setting is not a tuple or list.',
+                hint='Perhaps you forgot a trailing comma?',
+                id='staticfiles.E001',
+            )
+        ])
+
+    @override_settings(STATICFILES_DIRS=['/fake/path', settings.STATIC_ROOT])
+    def test_dirs_contains_static_root(self):
+        self.assertEqual(check_finders(None), [
+            Error(
+                'The STATICFILES_DIRS setting should not contain the '
+                'STATIC_ROOT setting.',
+                id='staticfiles.E002',
+            )
+        ])
+
+    @override_settings(STATICFILES_DIRS=[('prefix', settings.STATIC_ROOT)])
+    def test_dirs_contains_static_root_in_tuple(self):
+        self.assertEqual(check_finders(None), [
+            Error(
+                'The STATICFILES_DIRS setting should not contain the '
+                'STATIC_ROOT setting.',
+                id='staticfiles.E002',
+            )
+        ])
diff --git a/tests/staticfiles_tests/test_finders.py b/tests/staticfiles_tests/test_finders.py
index 0b661cb103..20d3060092 100644
--- a/tests/staticfiles_tests/test_finders.py
+++ b/tests/staticfiles_tests/test_finders.py
@@ -103,15 +103,6 @@ class TestMiscFinder(SimpleTestCase):
             [os.path.join(TEST_ROOT, 'project', 'documents')]
         )
 
-    @override_settings(STATICFILES_DIRS='a string')
-    def test_non_tuple_raises_exception(self):
-        """
-        We can't determine if STATICFILES_DIRS is set correctly just by
-        looking at the type, but we can determine if it's definitely wrong.
-        """
-        with self.assertRaises(ImproperlyConfigured):
-            finders.FileSystemFinder()
-
     @override_settings(MEDIA_ROOT='')
     def test_location_empty(self):
         with self.assertRaises(ImproperlyConfigured):