Moved the Django foreignkey field into a separate checker to enable the open() pre-configuration. This allows the django settings module to be set from a command-line option or .pylintrc option as well as an environment variable (refs #286). This commit also uses default Django settings if none are specified, raising an error to the user to inform them that it is better to set it explicitly, but avoiding the need to raise a Fatal or RuntimeError if Django is not configured (refs #277 and #243)

Author: carlio

pylint_django/checkers/__init__.py

@@ -4,6 +4,7 @@
 from pylint_django.checkers.json_response import JsonResponseChecker
 from pylint_django.checkers.forms import FormChecker
 from pylint_django.checkers.auth_user import AuthUserChecker
+from pylint_django.checkers.foreign_key_strings import ForeignKeyStringsChecker
 
 
 def register_checkers(linter):
@@ -13,3 +14,4 @@ def register_checkers(linter):
     linter.register_checker(JsonResponseChecker(linter))
     linter.register_checker(FormChecker(linter))
     linter.register_checker(AuthUserChecker(linter))
+    linter.register_checker(ForeignKeyStringsChecker(linter))

pylint_django/checkers/django_installed.py

@@ -20,8 +20,11 @@ class DjangoInstalledChecker(BaseChecker):
     }
 
     @check_messages('django-not-available')
-    def close(self):
+    def open(self):
         try:
             __import__('django')
         except ImportError:
-            self.add_message('F%s01' % BASE_ID)
+            # mild hack: this error is added before any modules have been inspected
+            # so we need to set some kind of value to prevent the linter failing to it
+            self.linter.set_current_module('project global')
+            self.add_message('django-not-available')

pylint_django/checkers/foreign_key_strings.py

@@ -0,0 +1,117 @@
+from __future__ import absolute_import
+from pylint.checkers import BaseChecker
+from pylint.interfaces import IAstroidChecker
+from pylint.checkers.utils import check_messages
+from pylint_django.__pkginfo__ import BASE_ID
+from pylint_django.transforms import foreignkey
+import astroid
+
+
+class ForeignKeyStringsChecker(BaseChecker):
+    """
+    Adds transforms to be able to do type inference for model ForeignKeyField
+    properties which use a string to name the foreign relationship. This uses
+    Django's model name resolution and this checker wraps the setup to ensure
+    Django is able to configure itself before attempting to use the lookups.
+    """
+
+    _LONG_MESSAGE = """Finding foreign-key relationships from strings in pylint-django requires configuring Django.
+This can be done via the DJANGO_SETTINGS_MODULE environment variable or the pylint option django-settings-module, eg:
+
+    `pylint --load-plugins=pylint_django --django-settings-module=myproject.settings`
+
+. This can also be set as an option in a .pylintrc configuration file.
+
+Some basic default settings were used, however this will lead to less accurate linting.
+Consider passing in an explicit Django configuration file to match your project to improve accuracy."""
+
+    __implements__ = (IAstroidChecker,)
+
+    name = "Django foreign keys referenced by strings"
+
+    options = (
+        (
+            "django-settings-module",
+            {
+                "default": None,
+                "type": "string",
+                "metavar": "<django settings module>",
+                "help": "A module containing Django settings to be used while linting.",
+            },
+        ),
+    )
+
+    msgs = {"E%s10" % BASE_ID: ("Django was not configured. For more information run pylint --load-plugins=pylint_django --help-msg=django-not-configured", "django-not-configured", _LONG_MESSAGE),
+    "F%s10" % BASE_ID: (
+        'Provided Django settings %s could not be loaded',
+        'django-settings-module-not-found',
+        'The provided Django settings module %s was not found on the path'
+    )}
+
+    def open(self):
+        self._raise_warning = False
+        # This is a bit of a hacky workaround. pylint-django does not *require* that
+        # Django is configured explicitly, and will use some basic defaults in that
+        # case. However, as this is a WARNING not a FATAL, the error must be raised
+        # with an AST node - only F and R messages are scope exempt (see
+        # https://github.com/PyCQA/pylint/blob/master/pylint/constants.py#L24)
+
+        # However, testing to see if Django is configured happens in `open()`
+        # before any modules are inspected, as Django needs to be configured with
+        # defaults before the foreignkey checker can work properly. At this point,
+        # there are no nodes.
+
+        # Therefore, during the initialisation in `open()`, if django was configured
+        # using defaults by pylint-django, it cannot raise the warning yet and
+        # must wait until some module is inspected to be able to raise... so that
+        # state is stashed in this property.
+
+        from django.core.exceptions import (
+            ImproperlyConfigured,
+        )  # pylint: disable=import-outside-toplevel
+
+        try:
+            import django  # pylint: disable=import-outside-toplevel
+
+            django.setup()
+            from django.apps import apps  # pylint: disable=import-outside-toplevel
+        except ImproperlyConfigured:
+            # this means that Django wasn't able to configure itself using some defaults
+            # provided (likely in a DJANGO_SETTINGS_MODULE environment variable)
+            # so see if the user has specified a pylint option
+            if self.config.django_settings_module is None:
+                # we will warn the user that they haven't actually configured Django themselves
+                self._raise_warning = True
+                # but use django defaults then...
+                from django.conf import settings  # pylint: disable=import-outside-toplevel
+                settings.configure()
+                django.setup()
+            else:
+                # see if we can load the provided settings module
+                try:
+                    from django.conf import settings, Settings  # pylint: disable=import-outside-toplevel
+                    settings.configure(Settings(self.config.django_settings_module))
+                    django.setup()
+                except ImportError:
+                    # we could not find the provided settings module...
+                    # at least here it is a fatal error so we can just raise this immediately
+                    self.add_message('django-settings-module-not-found', args=self.config.django_settings_module)
+                    # however we'll trundle on with basic settings
+                    from django.conf import settings  # pylint: disable=import-outside-toplevel
+                    settings.configure()
+                    django.setup()
+
+        # now we can add the trasforms speciifc to this checker
+        foreignkey.add_transform(astroid.MANAGER)
+
+        # TODO: this is a bit messy having so many inline imports but in order to avoid
+        # duplicating the django_installed checker, it'll do for now. In the future, merging
+        # those two checkers together might make sense.
+
+
+    @check_messages("django-not-configured")
+    def visit_module(self, node):
+        if self._raise_warning:
+            # just add it to the first node we see... which isn't nice but not sure what else to do
+            self.add_message("django-not-configured", node=node)
+            self._raise_warning = False  # only raise it once...

pylint_django/checkers/migrations.py

@@ -156,7 +156,7 @@ def func(apps, schema_editor):
     return is_migrations_module(node.parent)
 
 
-def load_configuration(linter):
+def load_configuration(linter):  # TODO this is redundant and can be  removed
     # don't blacklist migrations for this checker
     new_black_list = list(linter.config.black_list)
     if 'migrations' in new_black_list:

pylint_django/transforms/__init__.py

@@ -1,16 +1,24 @@
-"""Transforms."""
+"""
+These transforms replace the Django types with adapted versions to provide
+additional typing and method inference to pylint. All of these transforms
+are considered "global" to pylint-django, in that all checks and improvements
+requre them to be loaded. Additional transforms specific to checkers are loaded
+by the checker rather than here.
+
+For example, the ForeignKeyStringsChecker loads the foreignkey.py transforms
+itself as it may be disabled independently of the rest of pylint-django
+"""
 import os
 import re
 
 import astroid
 
-from pylint_django.transforms import foreignkey, fields
+from pylint_django.transforms import fields
 
-
-foreignkey.add_transform(astroid.MANAGER)
 fields.add_transforms(astroid.MANAGER)
 
 
+
 def _add_transform(package_name):
     def fake_module_builder():
         """

pylint_django/utils.py

@@ -8,7 +8,7 @@
 
 from pylint_django.compat import Uninferable
 
-PY3 = sys.version_info >= (3, 0)
+PY3 = sys.version_info >= (3, 0)  # TODO: pylint_django doesn't support Py2 any more
 
 
 def node_is_subclass(cls, *subclass_names):