adding warnings for __isnull (#3033)

mypy_django_plugin/transformers/orm_lookups.py

@@ -1,12 +1,24 @@
+from django.db.models.constants import LOOKUP_SEP
 from mypy.plugin import MethodContext
-from mypy.types import AnyType, Instance, ProperType, TypeOfAny, get_proper_type
+from mypy.types import AnyType, Instance, LiteralType, ProperType, TypeOfAny, get_proper_type
 from mypy.types import Type as MypyType
 
 from mypy_django_plugin.django.context import DjangoContext
 from mypy_django_plugin.exceptions import UnregisteredModelError
 from mypy_django_plugin.lib import fullnames, helpers
 
 
+def _extract_literal_bool(provided_type: ProperType) -> bool | None:
+    literal: LiteralType | None = None
+    if isinstance(provided_type, LiteralType):
+        literal = provided_type
+    elif isinstance(provided_type, Instance) and provided_type.last_known_value is not None:
+        literal = provided_type.last_known_value
+    if literal is not None and isinstance(literal.value, bool):
+        return literal.value
+    return None
+
+
 def typecheck_queryset_filter(ctx: MethodContext, django_context: DjangoContext) -> MypyType:
     django_model = helpers.get_model_info_from_qs_ctx(ctx, django_context)
     if django_model is None:
@@ -21,6 +33,43 @@ def typecheck_queryset_filter(ctx: MethodContext, django_context: DjangoContext)
         if lookup_kwarg is None:
             continue
         provided_type = get_proper_type(provided_type)
+
+        lookup_path, _, lookup_name = lookup_kwarg.rpartition(LOOKUP_SEP)
+
+        if lookup_name == "isnull" and isinstance(provided_type, LiteralType):
+            isnull_value = _extract_literal_bool(provided_type)
+
+            if isnull_value is not None and lookup_path:
+                field = None
+
+                try:
+                    real_model_cls = django_context.get_model_class_by_fullname(django_model.cls.fullname)
+                    if real_model_cls is not None:
+                        path_parts = lookup_path.split(LOOKUP_SEP)
+                        current_model = real_model_cls
+
+                        for part in path_parts:
+                            field = current_model._meta.get_field(part)
+
+                            if hasattr(field, "related_model") and field.related_model is not None:
+                                current_model = field.related_model
+                except Exception:
+                    field = None
+
+                if field is not None and getattr(field, "null", None) is False:
+                    if isnull_value is True:
+                        ctx.api.fail(
+                            f'Field "{field.name}" does not allow NULL;'
+                            f'using "__isnull=True" will always return an empty queryset.',
+                            ctx.context,
+                        )
+                    elif isnull_value is False:
+                        ctx.api.fail(
+                            f'Field "{field.name}" does not allow NULL;'
+                            f'using "__isnull=False" is a no-op and can be removed',
+                            ctx.context,
+                        )
+
         if isinstance(provided_type, Instance) and provided_type.type.has_base(
             fullnames.COMBINABLE_EXPRESSION_FULLNAME
         ):

tests/typecheck/db/models/test_filter_isnull.yml

@@ -0,0 +1,192 @@
+- case: filter_isnull_true_on_non_nullable_field_is_an_error
+  main: |
+    from myapp.models import MyModel
+    MyModel.objects.filter(name__isnull=True)  # E: Field "name" does not allow NULL; using "__isnull=True" will always return an empty queryset.  [misc]
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            name = models.CharField(max_length=100)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_false_on_non_nullable_field_is_a_noop_error
+  main: |
+    from myapp.models import MyModel
+    MyModel.objects.filter(name__isnull=False)  # E: Field "name" does not allow NULL; using "__isnull=False" is a no-op and can be removed.  [misc]
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            name = models.CharField(max_length=100)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_true_on_nullable_field_is_allowed
+  main: |
+    from myapp.models import MyModel
+    MyModel.objects.filter(name__isnull=True)
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            name = models.CharField(max_length=100, null=True)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_false_on_nullable_field_is_allowed
+  main: |
+    from myapp.models import MyModel
+    MyModel.objects.filter(name__isnull=False)
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            name = models.CharField(max_length=100, null=True)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_with_plain_bool_variable_does_not_raise
+  # A runtime bool variable is not a LiteralType — its value cannot be
+  # statically determined, so no error should be emitted.
+  main: |
+    from myapp.models import MyModel
+    flag: bool
+    MyModel.objects.filter(name__isnull=flag)
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            name = models.CharField(max_length=100)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_true_on_related_field_traversal_non_nullable
+  # Traversing a FK: the resolved leaf field "name" is non-nullable → error.
+  main: |
+    from myapp.models import Order
+    Order.objects.filter(customer__name__isnull=True)  # E: Field "name" does not allow NULL; using "__isnull=True" will always return an empty queryset.  [misc]
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class Customer(models.Model):
+            name = models.CharField(max_length=100)
+
+            class Meta:
+                app_label = "myapp"
+
+        class Order(models.Model):
+            customer = models.ForeignKey(Customer, on_delete=models.CASCADE)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_true_on_nullable_fk_field_itself_is_allowed
+  # __isnull directly on a nullable FK ("has no related object") must not be flagged.
+  main: |
+    from myapp.models import Order
+    Order.objects.filter(customer__isnull=True)
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class Customer(models.Model):
+            name = models.CharField(max_length=100)
+
+            class Meta:
+                app_label = "myapp"
+
+        class Order(models.Model):
+            customer = models.ForeignKey(Customer, null=True, on_delete=models.SET_NULL)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_mixed_kwargs_only_non_nullable_field_errors
+  # Only the non-nullable field produces an error; the nullable one is silent.
+  main: |
+    from myapp.models import MyModel
+    MyModel.objects.filter(
+        name__isnull=True,  # E: Field "name" does not allow NULL; using "__isnull=True" will always return an empty queryset.  [misc]
+        bio__isnull=True,
+    )
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            name = models.CharField(max_length=100)
+            bio = models.TextField(null=True)
+
+            class Meta:
+                app_label = "myapp"
+
+
+- case: filter_isnull_on_integer_field_non_nullable
+  # The check is field-type-agnostic: IntegerField with null=False also triggers.
+  main: |
+    from myapp.models import MyModel
+    MyModel.objects.filter(age__isnull=True)  # E: Field "age" does not allow NULL; using "__isnull=True" will always return an empty queryset.  [misc]
+  installed_apps:
+    - myapp
+  files:
+    - path: myapp/__init__.py
+    - path: myapp/models.py
+      content: |
+        from django.db import models
+
+        class MyModel(models.Model):
+            age = models.IntegerField()
+
+            class Meta:
+                app_label = "myapp"