Improved Django settings handling

.travis.yml

@@ -45,7 +45,7 @@ script:
 
 after_success:
   - |
-        if [ -z "$SANITY_CHECK" ]; then
+        if [ -z "$SANITY_CHECK" ] && [ -z "$TOXENV" ]; then
             pip install coveralls
             coveralls
         fi

CHANGELOG.rst

@@ -1,6 +1,17 @@
 Changelog
 =========
 
+Version 2.3.1 (TBD)
+-------------------
+
+- Allowed configuration of the Django settings module to be used via a
+  commandline argument `#286 <https://github.com/PyCAQ/pylint-django/issues/286>`_
+- If Django settings are not specified via a commandline argument or environment
+  variable, an error is issued but defaults are loaded from Django, removing the
+  fatal error behaviour. `#277 <https://github.com/PyCAQ/pylint-django/issues/277>`__
+  and `#243 <https://github.com/PyCAQ/pylint-django/issues/243>`__
+- Fixed tests to work with pylint>2.6
+
 Version 2.3.0 (05 Aug 2020)
 ---------------------------
 

README.rst

@@ -44,10 +44,21 @@ about missing Django!
 Usage
 -----
 
-Ensure ``pylint-django`` is installed and on your path and then execute::
+
+Ensure ``pylint-django`` is installed and on your path. In order to access some
+of the internal Django features to improve pylint inspections, you should also
+provide a Django settings module appropriate to your project. This can be done
+either with an environment variable::
 
     DJANGO_SETTINGS_MODULE=your.app.settings pylint --load-plugins pylint_django [..other options..] <path_to_your_sources>
 
+Alternatively, this can be passed in as a commandline flag::
+
+    pylint --load-plugins pylint_django --django-settings-module=your.app.settings [..other options..] <path_to_your_sources>
+
+If you do not configure Django, default settings will be used but this will not include, for
+example, which applications to include in `INSTALLED_APPS` and so the linting and type inference
+will be less accurate. It is recommended to specify a settings module.
 
 Prospector
 ----------

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,142 @@
+from __future__ import absolute_import
+
+import astroid
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import check_messages
+from pylint.interfaces import IAstroidChecker
+
+from pylint_django.__pkginfo__ import BASE_ID
+from pylint_django.transforms import foreignkey
+
+
+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 __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._raise_warning = False
+
+    def open(self):
+        # 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 (  # pylint: disable=import-outside-toplevel
+            ImproperlyConfigured,
+        )
+
+        try:
+            import django  # pylint: disable=import-outside-toplevel
+
+            django.setup()
+            from django.apps import apps  # noqa pylint: disable=import-outside-toplevel,unused-import
+        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 (  # pylint: disable=import-outside-toplevel
+                    settings,
+                )
+                settings.configure()
+                django.setup()
+            else:
+                # see if we can load the provided settings module
+                try:
+                    from django.conf import (  # pylint: disable=import-outside-toplevel
+                        settings,
+                        Settings,
+                    )
+
+                    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 (  # pylint: disable=import-outside-toplevel
+                        settings,
+                    )
+
+                    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,13 +1,20 @@
-"""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)
 
 

pylint_django/transforms/foreignkey.py

@@ -108,10 +108,10 @@ def infer_key_classes(node, context=None):
                     # 'auth.models', 'User' which works nicely with the `endswith()`
                     # comparison below
                     module_name += '.models'
-                except ImproperlyConfigured:
+                except ImproperlyConfigured as exep:
                     raise RuntimeError("DJANGO_SETTINGS_MODULE required for resolving ForeignKey "
                                        "string references, see Usage section in README at "
-                                       "https://pypi.org/project/pylint-django/!")
+                                       "https://pypi.org/project/pylint-django/!") from exep
 
                 # ensure that module is loaded in astroid_cache, for cases when models is a package
                 if module_name not in MANAGER.astroid_cache:

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):

scripts/build.sh

@@ -69,7 +69,7 @@ echo "..... Trying to install the new tarball inside a virtualenv"
 # note: installs with the optional dependency to verify this is still working
 virtualenv .venv/test-tarball
 source .venv/test-tarball/bin/activate
-pip install --no-binary :all: -f dist/ pylint_django[with_django]
+pip install -f dist/ pylint_django[with_django]
 pip freeze | grep Django
 deactivate
 rm -rf .venv/

tox.ini

@@ -15,7 +15,7 @@ envlist =
 [testenv]
 commands =
     django_not_installed: bash -c \'pylint --load-plugins=pylint_django setup.py | grep django-not-available\'
-    django_is_installed: pylint --load-plugins=pylint_django setup.py
+    django_is_installed: pylint --load-plugins=pylint_django --disable=E5110 setup.py
     flake8: flake8
     pylint: pylint --rcfile=tox.ini -d missing-docstring,too-many-branches,too-many-return-statements,too-many-ancestors,fixme --ignore=tests pylint_django setup
     readme: bash -c \'python setup.py -q sdist && twine check dist/*\'