Merge pull request #109 from yalef/master

Add model checkers based on `astroid` nodes

flake8_django/checker.py

@@ -1,14 +1,13 @@
 import ast
+import os
+import sys
 
-from flake8_django.checkers import (
-    DecoratorChecker,
-    ModelContentOrderChecker,
-    ModelDunderStrMissingChecker,
-    ModelFieldChecker,
-    ModelFormChecker,
-    ModelMetaChecker,
-    RenderChecker,
-)
+import astroid
+
+from flake8_django.checkers import (DecoratorChecker, ModelContentOrderChecker,
+                                    ModelDunderStrMissingChecker,
+                                    ModelFieldChecker, ModelFormChecker,
+                                    ModelMetaChecker, RenderChecker)
 
 __version__ = '1.1.5'
 
@@ -25,12 +24,6 @@ class DjangoStyleFinder(ast.NodeVisitor):
             ModelFieldChecker(),
             RenderChecker(),
         ],
-        'ClassDef': [
-            ModelFormChecker(),
-            ModelDunderStrMissingChecker(),
-            ModelMetaChecker(),
-            ModelContentOrderChecker(),
-        ],
         'FunctionDef': [
             DecoratorChecker(),
         ]
@@ -50,23 +43,68 @@ def capture_issues_visitor(self, visitor, node):
     def visit_Call(self, node):
         self.capture_issues_visitor('Call', node)
 
-    def visit_ClassDef(self, node):
-        self.capture_issues_visitor('ClassDef', node)
-
     def visit_FunctionDef(self, node):
         self.capture_issues_visitor('FunctionDef', node)
 
 
+class AstroidTreeVisitor:
+    """
+    Go through astroid tree and return issues by specified checkers.
+    """
+    checkers = {
+        "ClassDef": (
+            ModelMetaChecker(),
+            ModelFormChecker(),
+            ModelDunderStrMissingChecker(),
+            ModelContentOrderChecker(),
+        )
+    }
+
+    def __init__(self):
+        self.issues = []
+
+    def visit(self, tree):
+        for node in tree.body:
+            self.issues.extend(
+                self.run_checkers(
+                    node=node,
+                    checker_type=node.__class__.__name__,
+                ),
+            )
+
+    def run_checkers(self, node, checker_type):
+        issues = []
+        for checker in self.checkers.get(checker_type, []):
+            checker_issues = checker.run(node)
+            if checker_issues:
+                issues.extend(checker_issues)
+        return issues
+
+
 class DjangoStyleChecker(object):
     """
     Check common Django Style errors
     """
     name = 'flake8-django'
     version = __version__
+    astroid_manager = astroid.MANAGER
 
-    def __init__(self, tree, filename):
+    def __init__(self, tree, filename, lines=[]):
         self.tree = tree
         self.filename = filename
+        self.source_code = ''.join(lines)
+        self.build_astroid_tree()
+
+    def build_astroid_tree(self):
+        sys.path.append(os.getcwd())
+        if not self.filename:
+            self.astroid_tree = self.astroid_manager.ast_from_string(
+                self.source_code,
+            )
+        else:
+            self.astroid_tree = self.astroid_manager.ast_from_file(  # pragma: no cover
+                self.filename,
+            )
 
     @staticmethod
     def add_options(optmanager):
@@ -77,5 +115,8 @@ def run(self):
         parser = DjangoStyleFinder()
         parser.visit(self.tree)
 
-        for issue in parser.issues:
+        astroid_parser = AstroidTreeVisitor()
+        astroid_parser.visit(self.astroid_tree)
+
+        for issue in parser.issues + astroid_parser.issues:
             yield issue.lineno, issue.col, issue.message, DjangoStyleChecker

flake8_django/checkers/base_model_checker.py

@@ -1,52 +1,39 @@
-import ast
+import astroid
 
-from .checker import Checker
+from .checker import AstroidBaseChecker
+from .utils import node_is_subclass
 
 
-class BaseModelChecker(Checker):
+class BaseModelChecker(AstroidBaseChecker):
     """
     Base class for checkers that must lookup for Model like nodes.
     """
 
-    model_name_lookup = None
+    def is_model(self, node):
+        return node_is_subclass(node, self.model_name_lookups)
 
     @staticmethod
-    def _is_abstract_and_set_to_true(element):
+    def _is_abstract_and_set_to_true(node):
         return (
-            isinstance(element, ast.Assign)
-            and any(target.id == 'abstract' for target in element.targets if isinstance(target, ast.Name))
-            and isinstance(element.value, ast.NameConstant)
-            and element.value.value is True
+            isinstance(node, astroid.Assign)
+            and any(
+                target.name == 'abstract'
+                for target in node.targets
+                if isinstance(target, astroid.AssignName)
+            )
+            and isinstance(node.value, astroid.Const)
+            and node.value.value is True
         )
 
-    def is_abstract_model(self, base):
+    def is_abstract_model(self, node):
         """
-        Return True if AST node has a Meta class with abstract = True.
+        Return True if astroid node has a Meta class with abstract = True.
         """
         # look for "class Meta"
-        for element in base.body:
-            if isinstance(element, ast.ClassDef) and element.name == 'Meta':
+        for element in node.body:
+            if isinstance(element, astroid.ClassDef) and element.name == 'Meta':
                 # look for "abstract = True"
                 for inner_element in element.body:
                     if self._is_abstract_and_set_to_true(inner_element):
                         return True
         return False
-
-    def is_model_name_lookup(self, base):
-        """
-        Return True if class is defined as the respective model name lookup declaration
-        """
-        return (
-            isinstance(base, ast.Name) and
-            base.id == self.model_name_lookup
-        )
-
-    def is_models_name_lookup_attribute(self, base):
-        """
-        Return True if class is defined as the respective model name lookup declaration
-        """
-        return (
-            isinstance(base, ast.Attribute) and
-            isinstance(base.value, ast.Name) and
-            base.value.id == 'models' and base.attr == self.model_name_lookup
-        )

flake8_django/checkers/checker.py

@@ -21,3 +21,12 @@ def run(self, node):
         Method that runs the checks and returns the issues.
         """
         return NotImplementedError  # pragma: no cover
+
+
+class AstroidBaseChecker(object):
+    """
+    Abstract class for astroid nodes checkers.
+    """
+
+    def run(self, node):
+        return NotImplementedError  # pragma: no cover

flake8_django/checkers/model_content_order.py

@@ -1,8 +1,9 @@
-from ast import Assign, ClassDef, FunctionDef
+import astroid
 from functools import partial
 
 from .base_model_checker import BaseModelChecker
 from .issue import Issue
+from .utils import node_is_subclass
 
 
 class DJ12(Issue):
@@ -22,31 +23,39 @@ def __init__(self, elem, elem_type, before):
 
 def is_field_declaration(node):
     """
-    Verifies that the code is of the form: `field = models.CharField()`, matching by the `models` string.
+    Verify that node has Field value.
     """
     try:
-        return node.value.func.value.id == 'models'
+        for inferred_value in node.value.func.inferred():
+            if node_is_subclass(
+                inferred_value,
+                [".Field", "django.db.models.fields.Field"],
+            ):
+                return True
+        return False
     except AttributeError:
         return False
 
 
 def is_manager_declaration(node):
-    return isinstance(node, Assign) and getattr(node.targets[0], 'id', None) == 'objects'
+    return (
+        isinstance(node, astroid.Assign)
+        and getattr(node.targets[0], 'name', None) == 'objects'
+    )
 
 
 def is_meta_declaration(node):
-    return isinstance(node, ClassDef) and node.name == 'Meta'
+    return isinstance(node, astroid.ClassDef) and node.name == 'Meta'
 
 
 def is_method(node, method_name=None):
     if method_name is None:
-        return isinstance(node, FunctionDef)
-    return isinstance(node, FunctionDef) and node.name == method_name
+        return isinstance(node, astroid.FunctionDef)
+    return isinstance(node, astroid.FunctionDef) and node.name == method_name
 
 
 class ModelContentOrderChecker(BaseModelChecker):
-    model_name_lookup = 'Model'
-
+    model_name_lookups = ['.Model', 'django.db.models.base.Model']
     FIELD_DECLARATION = 'field declaration'
     MANAGER_DECLARATION = 'manager declaration'
     META_CLASS = 'Meta class'
@@ -75,10 +84,7 @@ class ModelContentOrderChecker(BaseModelChecker):
     ]
 
     def checker_applies(self, node):
-        for base in node.bases:
-            if self.is_model_name_lookup(base) or self.is_models_name_lookup_attribute(base):
-                return True
-        return False
+        return self.is_model(node)
 
     def run(self, node):
         if not self.checker_applies(node):

flake8_django/checkers/model_dunder_str.py

@@ -1,4 +1,4 @@
-import ast
+import astroid
 
 from .base_model_checker import BaseModelChecker
 from .issue import Issue
@@ -10,18 +10,14 @@ class DJ08(Issue):
 
 
 class ModelDunderStrMissingChecker(BaseModelChecker):
-    model_name_lookup = 'Model'
+    model_name_lookups = ['.Model', 'django.db.models.base.Model']
 
     def checker_applies(self, node):
-        for base in node.bases:
-            if self.is_model_name_lookup(base) or self.is_models_name_lookup_attribute(base):
-                if not self.is_abstract_model(node):
-                    return True
-        return False
+        return self.is_model(node) and not self.is_abstract_model(node)
 
     def is_dunder_str_method(self, element):
         return (
-            isinstance(element, ast.FunctionDef) and
+            isinstance(element, astroid.FunctionDef) and
             element.name == '__str__'
         )
 

flake8_django/checkers/model_form.py

@@ -1,4 +1,4 @@
-import ast
+import astroid
 
 from .base_model_checker import BaseModelChecker
 from .issue import Issue
@@ -15,26 +15,25 @@ class DJ07(Issue):
 
 
 class ModelFormChecker(BaseModelChecker):
-    model_name_lookup = 'ModelForm'
+    model_name_lookups = ['.ModelForm', 'django.forms.models.ModelForm']
 
     def checker_applies(self, node):
-        for base in node.bases:
-            is_model_form = self.is_model_name_lookup(base) or self.is_models_name_lookup_attribute(base)
-            if is_model_form:
-                return True
-        return False
+        return self.is_model(node)
 
     def is_string_dunder_all(self, element):
         """
-        Return True if element is ast.Str or ast.Bytes and equals "__all__"
+        Return True if element is astroid.Const, astroid.List or astroid.Tuple  and equals "__all__"
         """
-        if not isinstance(element.value, (ast.Str, ast.Bytes)):
-            return False
-
-        node_value = element.value.s
-        if isinstance(node_value, bytes):
-            node_value = node_value.decode()
-        return node_value == '__all__'
+        if isinstance(element.value, (astroid.List, astroid.Tuple)):
+            return any(
+                iter_item.value == '__all__'
+                for iter_item in element.value.itered()
+            )
+        else:
+            node_value = element.value.value
+            if isinstance(node_value, bytes):
+                node_value = node_value.decode()
+            return node_value == '__all__'
 
     def run(self, node):
         """
@@ -45,20 +44,20 @@ def run(self, node):
 
         issues = []
         for body in node.body:
-            if not isinstance(body, ast.ClassDef):
+            if not isinstance(body, astroid.ClassDef):
                 continue
             for element in body.body:
-                if not isinstance(element, ast.Assign):
+                if not isinstance(element, astroid.Assign):
                     continue
                 for target in element.targets:
-                    if target.id == 'fields' and self.is_string_dunder_all(element):
+                    if target.name == 'fields' and self.is_string_dunder_all(element):
                         issues.append(
                             DJ07(
                                 lineno=node.lineno,
                                 col=node.col_offset,
                             )
                         )
-                    elif target.id == 'exclude':
+                    elif target.name == 'exclude':
                         issues.append(
                             DJ06(
                                 lineno=node.lineno,