Fixed #17673 -- Forbid field shadowing.

Thanks Anssi Kääriäinen for the suggestion.

3 files changed, 131 insertions, 57 deletions

diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py
index 8515cc8070..9820aea969 100644
--- a/tests/invalid_models_tests/test_models.py
+++ b/tests/invalid_models_tests/test_models.py
@@ -191,48 +191,158 @@ class UniqueTogetherTests(IsolatedModelsTestCase):
         self.assertEqual(errors, expected)
 
 
-class OtherModelTests(IsolatedModelsTestCase):
+class FieldNamesTests(IsolatedModelsTestCase):
 
-    def test_unique_primary_key(self):
+    def test_ending_with_underscore(self):
         class Model(models.Model):
-            id = models.IntegerField(primary_key=False)
+            field_ = models.CharField(max_length=10)
+            m2m_ = models.ManyToManyField('self')
 
         errors = Model.check()
         expected = [
             Error(
-                ('You cannot use "id" as a field name, because each model '
-                 'automatically gets an "id" field if none of the fields '
-                 'have primary_key=True.'),
-                hint='Remove or rename "id" field or add primary_key=True to a field.',
-                obj=Model,
-                id='E005',
+                'Field names must not end with underscores.',
+                hint=None,
+                obj=Model._meta.get_field('field_'),
+                id='E001',
             ),
             Error(
-                'Field "id" has column name "id" that is already used.',
+                'Field names must not end with underscores.',
                 hint=None,
-                obj=Model,
+                obj=Model._meta.get_field('m2m_'),
+                id='E001',
+            ),
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_including_separator(self):
+        class Model(models.Model):
+            some__field = models.IntegerField()
+
+        errors = Model.check()
+        expected = [
+            Error(
+                'Field names must not contain "__".',
+                hint=None,
+                obj=Model._meta.get_field('some__field'),
+                id='E052',
             )
         ]
         self.assertEqual(errors, expected)
 
-    def test_field_names_ending_with_underscore(self):
+    def test_pk(self):
         class Model(models.Model):
-            field_ = models.CharField(max_length=10)
-            m2m_ = models.ManyToManyField('self')
+            pk = models.IntegerField()
 
         errors = Model.check()
         expected = [
             Error(
-                'Field names must not end with underscores.',
+                'Cannot use "pk" as a field name since it is a reserved name.',
                 hint=None,
-                obj=Model._meta.get_field('field_'),
-                id='E001',
+                obj=Model._meta.get_field('pk'),
+                id='E051',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+
+class ShadowingFieldsTests(IsolatedModelsTestCase):
+
+    def test_multiinheritance_clash(self):
+        class Mother(models.Model):
+            clash = models.IntegerField()
+
+        class Father(models.Model):
+            clash = models.IntegerField()
+
+        class Child(Mother, Father):
+            # Here we have two clashed: id (automatic field) and clash, because
+            # both parents define these fields.
+            pass
+
+        errors = Child.check()
+        expected = [
+            Error(
+                ('The field "id" from parent model '
+                 'invalid_models_tests.mother clashes with the field "id" '
+                 'from parent model invalid_models_tests.father.'),
+                hint=None,
+                obj=Child,
+                id='E053',
             ),
             Error(
-                'Field names must not end with underscores.',
+                ('The field "clash" from parent model '
+                 'invalid_models_tests.mother clashes with the field "clash" '
+                 'from parent model invalid_models_tests.father.'),
                 hint=None,
-                obj=Model._meta.get_field('m2m_'),
-                id='E001',
+                obj=Child,
+                id='E053',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_inheritance_clash(self):
+        class Parent(models.Model):
+            f_id = models.IntegerField()
+
+        class Target(models.Model):
+            # This field doesn't result in a clash.
+            f_id = models.IntegerField()
+
+        class Child(Parent):
+            # This field clashes with parent "f_id" field.
+            f = models.ForeignKey(Target)
+
+        errors = Child.check()
+        expected = [
+            Error(
+                ('The field clashes with the field "f_id" '
+                 'from model invalid_models_tests.parent.'),
+                hint=None,
+                obj=Child._meta.get_field('f'),
+                id='E054',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+    def test_id_clash(self):
+        class Target(models.Model):
+            pass
+
+        class Model(models.Model):
+            fk = models.ForeignKey(Target)
+            fk_id = models.IntegerField()
+
+        errors = Model.check()
+        expected = [
+            Error(
+                ('The field clashes with the field "fk" from model '
+                 'invalid_models_tests.model.'),
+                hint=None,
+                obj=Model._meta.get_field('fk_id'),
+                id='E054',
+            )
+        ]
+        self.assertEqual(errors, expected)
+
+
+class OtherModelTests(IsolatedModelsTestCase):
+
+    def test_unique_primary_key(self):
+        invalid_id = models.IntegerField(primary_key=False)
+
+        class Model(models.Model):
+            id = invalid_id
+
+        errors = Model.check()
+        expected = [
+            Error(
+                ('You cannot use "id" as a field name, because each model '
+                 'automatically gets an "id" field if none of the fields '
+                 'have primary_key=True.'),
+                hint='Remove or rename "id" field or add primary_key=True to a field.',
+                obj=Model,
+                id='E005',
             ),
         ]
         self.assertEqual(errors, expected)
diff --git a/tests/model_inheritance/models.py b/tests/model_inheritance/models.py
index 7f5702da59..3245c71232 100644
--- a/tests/model_inheritance/models.py
+++ b/tests/model_inheritance/models.py
@@ -45,10 +45,6 @@ class Student(CommonInfo):
         pass
 
 
-class StudentWorker(Student, Worker):
-    pass
-
-
 #
 # Abstract base classes with related models
 #
diff --git a/tests/model_inheritance/tests.py b/tests/model_inheritance/tests.py
index dab3088a41..ebcfe1154e 100644
--- a/tests/model_inheritance/tests.py
+++ b/tests/model_inheritance/tests.py
@@ -10,7 +10,7 @@ from django.utils import six
 
 from .models import (
     Chef, CommonInfo, ItalianRestaurant, ParkingLot, Place, Post,
-    Restaurant, Student, StudentWorker, Supplier, Worker, MixinModel,
+    Restaurant, Student, Supplier, Worker, MixinModel,
     Title, Base, SubBase)
 
 
@@ -48,38 +48,6 @@ class ModelInheritanceTests(TestCase):
         # doesn't exist as a model).
         self.assertRaises(AttributeError, lambda: CommonInfo.objects.all())
 
-        # A StudentWorker which does not exist is both a Student and Worker
-        # which does not exist.
-        self.assertRaises(
-            Student.DoesNotExist,
-            StudentWorker.objects.get, pk=12321321
-        )
-        self.assertRaises(
-            Worker.DoesNotExist,
-            StudentWorker.objects.get, pk=12321321
-        )
-
-        # MultipleObjectsReturned is also inherited.
-        # This is written out "long form", rather than using __init__/create()
-        # because of a bug with diamond inheritance (#10808)
-        sw1 = StudentWorker()
-        sw1.name = "Wilma"
-        sw1.age = 35
-        sw1.save()
-        sw2 = StudentWorker()
-        sw2.name = "Betty"
-        sw2.age = 24
-        sw2.save()
-
-        self.assertRaises(
-            Student.MultipleObjectsReturned,
-            StudentWorker.objects.get, pk__lt=sw2.pk + 100
-        )
-        self.assertRaises(
-            Worker.MultipleObjectsReturned,
-            StudentWorker.objects.get, pk__lt=sw2.pk + 100
-        )
-
     def test_multiple_table(self):
         post = Post.objects.create(title="Lorem Ipsum")
         # The Post model has distinct accessors for the Comment and Link models.