[#369] Hints for querysets and models

pylint_django/checkers/__init__.py

@@ -4,7 +4,10 @@
 from pylint_django.checkers.foreign_key_strings import ForeignKeyStringsChecker
 from pylint_django.checkers.forms import FormChecker
 from pylint_django.checkers.json_response import JsonResponseChecker
+from pylint_django.checkers.modelfilter_forloop import ModelFilterForLoopChecker
 from pylint_django.checkers.models import ModelChecker
+from pylint_django.checkers.modelsave_forloop import ModelSaveForLoopChecker
+from pylint_django.checkers.queryset_iterator import QuerysetIteratorForLoopChecker
 
 
 def register_checkers(linter):
@@ -15,3 +18,6 @@ def register_checkers(linter):
     linter.register_checker(FormChecker(linter))
     linter.register_checker(AuthUserChecker(linter))
     linter.register_checker(ForeignKeyStringsChecker(linter))
+    linter.register_checker(ModelSaveForLoopChecker(linter))
+    linter.register_checker(ModelFilterForLoopChecker(linter))
+    linter.register_checker(QuerysetIteratorForLoopChecker(linter))

pylint_django/checkers/modelfilter_forloop.py

@@ -0,0 +1,48 @@
+from astroid.nodes import Attribute, Call, Expr
+from pylint import checkers, interfaces
+from pylint.checkers import utils
+
+from pylint_django.__pkginfo__ import BASE_ID
+
+
+class ModelFilterForLoopChecker(checkers.BaseChecker):
+    """
+    Checks for usage of "Model.manager.filter() inside of for loops
+    """
+
+    __implements__ = interfaces.IAstroidChecker
+
+    name = "model-filter-forloop-checker"
+
+    msgs = {
+        f"R{BASE_ID}06": (
+            "Consider using '__in' queries",
+            "consider-using-in-queries",
+            "Using 'Model.manager.filter()' or 'Model.manager.get() inside a "
+            "for loop may negatively impact performance. Consider using a "
+            "single query with as '__in' filter instead, outside of the loop.",
+        ),
+    }
+
+    @utils.check_messages("consider-using-in-queries")
+    def visit_for(self, node):
+        """
+        Checks for a Queryset.filter() inside of a for loop
+
+        May create false positives
+        """
+        for subnode in node.body:
+            for child in subnode.get_children():
+                if isinstance(child, Expr):
+                    for subchild in child.get_children():
+                        if isinstance(subchild, Call):
+                            self._check_forloop_filter(subchild)
+                if isinstance(child, Call):
+                    self._check_forloop_filter(child)
+
+    def _check_forloop_filter(self, node):
+        if isinstance(node.func, Attribute):
+            # Ensure it's an attribute, to avoid clashing with the
+            # filter() python builtin, may still clash with the dict .get()
+            if node.func.attrname in ("filter", "get"):
+                self.add_message(f"R{BASE_ID}06", node=node)

pylint_django/checkers/modelsave_forloop.py

@@ -0,0 +1,60 @@
+from astroid.nodes import Attribute, Call, Expr
+from pylint import checkers, interfaces
+from pylint.checkers import utils
+
+from pylint_django.__pkginfo__ import BASE_ID
+
+# from pylint_django.augmentations import is_manager_attribute
+
+
+class ModelSaveForLoopChecker(checkers.BaseChecker):
+    """
+    Checks for usage of Model.manager.create() or Model.save() inside of for
+    loops
+    """
+
+    __implements__ = interfaces.IAstroidChecker
+
+    name = "model-save-forloop-checker"
+
+    msgs = {
+        f"R{BASE_ID}04": (
+            "Consider using 'Model.bulk_create()'",
+            "consider-using-bulk-create",
+            "Using 'Model.manager.create()' inside a for loop may negatively "
+            "impact performance. Consider using 'Model.manager.bulk_create()' "
+            "instead.",
+        ),
+        f"R{BASE_ID}05": (
+            "Consider using 'Model.bulk_*()",
+            "consider-using-bulk-create-save",
+            "Using 'Model.save()' inside a for loop may negatively impact "
+            "performance. Consider using 'Model.manager.bulk_update()' or "
+            "'Model.manager.bulk_create()' instead.",
+        ),
+    }
+
+    @utils.check_messages("consider-using-bulk-create")
+    @utils.check_messages("consider-using-bulk-create-save")
+    def visit_for(self, node):
+        """
+        Checks for a Model.create() inside of a for loop
+
+        May create false positives
+        """
+        for subnode in node.body:
+            for child in subnode.get_children():
+                if isinstance(child, Expr):
+                    for subchild in child.get_children():
+                        if isinstance(subchild, Call):
+                            self._check_forloop_create_save(subchild)
+                if isinstance(child, Call):
+                    self._check_forloop_create_save(child)
+
+    def _check_forloop_create_save(self, node):
+        if isinstance(node.func, Attribute):
+            # Ensure node.func is an attribute to avoid crashes
+            if node.func.attrname == "create":
+                self.add_message(f"R{BASE_ID}04", node=node)
+            if node.func.attrname == "save":
+                self.add_message(f"R{BASE_ID}05", node=node)

pylint_django/checkers/queryset_iterator.py

@@ -0,0 +1,43 @@
+from astroid.nodes import Attribute, Call
+from pylint import checkers, interfaces
+from pylint.checkers import utils
+
+from pylint_django.__pkginfo__ import BASE_ID
+
+# from pylint_django.augmentations import is_manager_attribute
+
+
+class QuerysetIteratorForLoopChecker(checkers.BaseChecker):
+    """
+    Checks for usage of "QuerySet.all()" in the head of a for loop,
+    eventually suggesting the usage of ".iterator()"
+    """
+
+    __implements__ = interfaces.IAstroidChecker
+
+    name = "queryset-iterator-forloop-checker"
+
+    msgs = {
+        f"R{BASE_ID}07": (
+            "Consider using 'Queryset.iterator()'",
+            "consider-using-queryset-iterator",
+            "Using 'QuerySet.all()' may load all results in memory "
+            "if you need to iterate over the data only once and don't need "
+            "caching, 'QuerySet.iterator()' may be a more performant option.",
+        ),
+    }
+
+    @utils.check_messages("consider-using-queryset-iterator")
+    def visit_for(self, node):
+        """
+        Checks for a QuerySet.all() inside of a for loop
+
+        May create false positives
+        """
+        for subnode in node.get_children():
+            if isinstance(subnode, Call):
+                if isinstance(subnode.func, Attribute):
+                    # Ensure it's an attribute, to avoid clashing with the
+                    # all() python builtin
+                    if subnode.func.attrname == "all":
+                        self.add_message(f"R{BASE_ID}07", node=subnode)

pylint_django/tests/input/func_queryset_hints.py

@@ -0,0 +1,41 @@
+"""
+Checks that using save() or create() inside a for loop
+raises complaints from PyLint
+"""
+# pylint: disable=missing-docstring,too-few-public-methods
+
+from django.db import models
+
+
+class Book(models.Model):
+    name = models.CharField(max_length=100)
+
+    class Meta:
+        app_label = "test_app"
+
+
+def for_create():
+    for i in range(10):
+        Book.objects.create(name=str(i))  # [consider-using-bulk-create]
+
+
+def for_nested_if_create():
+    for i in range(10):
+        if i % 2 == 0:
+            Book.objects.create(name=str(i))  # [consider-using-bulk-create]
+
+
+def assigned_for_create():
+    for i in range(10):
+        _ = Book.objects.create(name=str(i))  # [consider-using-bulk-create]
+
+
+def for_filter():
+    for i in range(10):
+        _ = Book.objects.filter(name=str(i))  # [consider-using-in-queries]
+
+
+def for_save():
+    obj = Book(name="Test Book")
+    for _ in range(10):
+        obj.save()  # [consider-using-bulk-create-save]

pylint_django/tests/input/func_queryset_hints.txt

@@ -0,0 +1,5 @@
+consider-using-bulk-create:19:8:19:40:for_create:Consider using 'Model.bulk_create()':UNDEFINED
+consider-using-bulk-create:25:12:25:44:for_nested_if_create:Consider using 'Model.bulk_create()':UNDEFINED
+consider-using-bulk-create:30:12:30:44:assigned_for_create:Consider using 'Model.bulk_create()':UNDEFINED
+consider-using-in-queries:35:12:35:44:for_filter:Consider using '__in' queries:UNDEFINED
+consider-using-bulk-create-save:41:8:41:18:for_save:Consider using 'Model.bulk_*():UNDEFINED