diff --git a/.travis.yml b/.travis.yml index 81337545..47203e1c 100644 --- a/.travis.yml +++ b/.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 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 264cd962..653271b5 100644 --- a/CHANGELOG.rst +++ b/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 `_ +- 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 `__ + and `#243 `__ +- Fixed tests to work with pylint>2.6 + Version 2.3.0 (05 Aug 2020) --------------------------- diff --git a/README.rst b/README.rst index f339aa77..8df40a61 100644 --- a/README.rst +++ b/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..] +Alternatively, this can be passed in as a commandline flag:: + + pylint --load-plugins pylint_django --django-settings-module=your.app.settings [..other options..] + +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 ---------- diff --git a/pylint_django/checkers/__init__.py b/pylint_django/checkers/__init__.py index d036e7b5..b0a5bb00 100644 --- a/pylint_django/checkers/__init__.py +++ b/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)) diff --git a/pylint_django/checkers/django_installed.py b/pylint_django/checkers/django_installed.py index 1a51c70c..de79e4d6 100644 --- a/pylint_django/checkers/django_installed.py +++ b/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('pylint_django') + self.add_message('django-not-available') diff --git a/pylint_django/checkers/foreign_key_strings.py b/pylint_django/checkers/foreign_key_strings.py new file mode 100644 index 00000000..02606cce --- /dev/null +++ b/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": "", + "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 specifc 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... diff --git a/pylint_django/checkers/migrations.py b/pylint_django/checkers/migrations.py index 4014d412..392ed324 100644 --- a/pylint_django/checkers/migrations.py +++ b/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: diff --git a/pylint_django/transforms/__init__.py b/pylint_django/transforms/__init__.py index cf602b0f..03930d3d 100644 --- a/pylint_django/transforms/__init__.py +++ b/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) diff --git a/pylint_django/transforms/foreignkey.py b/pylint_django/transforms/foreignkey.py index fdc02eca..d16b47c7 100644 --- a/pylint_django/transforms/foreignkey.py +++ b/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: diff --git a/pylint_django/utils.py b/pylint_django/utils.py index f91cf5aa..883ef122 100644 --- a/pylint_django/utils.py +++ b/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): diff --git a/scripts/build.sh b/scripts/build.sh index 0d261070..00ba5b91 100755 --- a/scripts/build.sh +++ b/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/ diff --git a/tox.ini b/tox.ini index ba833345..b5207d30 100644 --- a/tox.ini +++ b/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/*\'