From 1e5e2a470768549117ac4696ce3e8b4e51d65664 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Ericson?= <de.ericson@gmail.com>
Date: Sat, 4 Oct 2014 20:47:26 -0300
Subject: [PATCH] Fixed #22064 -- Add check for related_name
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Validates that related_name is a valid Python id or ends with a '+' and
it's not a keyword. Without a check it passed silently leading to
unpredictable problems.

Thanks Konrad Świat for the initial work.
---
 django/db/models/fields/related.py            | 21 ++++++
 docs/ref/checks.txt                           |  2 +
 .../test_relative_fields.py                   | 67 +++++++++++++++++++
 3 files changed, 90 insertions(+)

diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py
index f0b6fb5250..dce02aaa45 100644
--- a/django/db/models/fields/related.py
+++ b/django/db/models/fields/related.py
@@ -97,11 +97,32 @@ signals.class_prepared.connect(do_pending_lookups)
 class RelatedField(Field):
     def check(self, **kwargs):
         errors = super(RelatedField, self).check(**kwargs)
+        errors.extend(self._check_related_name_is_valid())
         errors.extend(self._check_relation_model_exists())
         errors.extend(self._check_referencing_to_swapped_model())
         errors.extend(self._check_clashes())
         return errors
 
+    def _check_related_name_is_valid(self):
+        import re
+        import keyword
+        related_name = self.rel.related_name
+
+        is_valid_id = (related_name and re.match('^[a-zA-Z_][a-zA-Z0-9_]*$', related_name)
+                       and not keyword.iskeyword(related_name))
+        if related_name and not (is_valid_id or related_name.endswith('+')):
+            return [
+                checks.Error(
+                    "The name '%s' is invalid related_name for field %s.%s" %
+                    (self.rel.related_name, self.model._meta.object_name,
+                     self.name),
+                    hint="Related name must be a valid Python identifier or end with a '+'",
+                    obj=self,
+                    id='fields.E306',
+                )
+            ]
+        return []
+
     def _check_relation_model_exists(self):
         rel_is_missing = self.rel.to not in apps.get_models()
         rel_is_string = isinstance(self.rel.to, six.string_types)
diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt
index aa3f61117c..841c8f4c48 100644
--- a/docs/ref/checks.txt
+++ b/docs/ref/checks.txt
@@ -117,6 +117,8 @@ Related Fields
   ``<field name>``.
 * **fields.E305**: Field name ``<field name>`` clashes with reverse query name
   for ``<field name>``.
+* **fields.E306**: Related name must be a valid Python identifier or end with
+  a ``'+'``.
 * **fields.E310**: None of the fields ``<field1>``, ``<field2>``, ... on model
   ``<model>`` have a ``unique=True`` constraint.
 * **fields.E311**: ``<model>`` must set ``unique=True`` because it is
diff --git a/tests/invalid_models_tests/test_relative_fields.py b/tests/invalid_models_tests/test_relative_fields.py
index 4b4c5947f3..9781ecc2ba 100644
--- a/tests/invalid_models_tests/test_relative_fields.py
+++ b/tests/invalid_models_tests/test_relative_fields.py
@@ -546,6 +546,73 @@ class RelativeFieldTests(IsolatedModelsTestCase):
             errors = field.check(from_model=Model)
             self.assertEqual(errors, [expected_error])
 
+    def test_related_field_has_invalid_related_name(self):
+        digit = 0
+        illegal_non_alphanumeric = '!'
+        whitespace = '\t'
+
+        invalid_related_names = [
+            '%s_begins_with_digit' % digit,
+            '%s_begins_with_illegal_non_alphanumeric' % illegal_non_alphanumeric,
+            '%s_begins_with_whitespace' % whitespace,
+            'contains_%s_illegal_non_alphanumeric' % illegal_non_alphanumeric,
+            'contains_%s_whitespace' % whitespace,
+            'ends_with_with_illegal_non_alphanumeric_%s' % illegal_non_alphanumeric,
+            'ends_with_whitespace_%s' % whitespace,
+            # Python's keyword
+            'with',
+        ]
+
+        class Parent(models.Model):
+            pass
+
+        for invalid_related_name in invalid_related_names:
+            Child = type(str('Child_%s') % str(invalid_related_name), (models.Model,), {
+                'parent': models.ForeignKey('Parent', related_name=invalid_related_name),
+                '__module__': Parent.__module__,
+            })
+
+            field = Child._meta.get_field('parent')
+            errors = Child.check()
+            expected = [
+                Error(
+                    "The name '%s' is invalid related_name for field Child_%s.parent"
+                    % (invalid_related_name, invalid_related_name),
+                    hint="Related name must be a valid Python identifier or end with a '+'",
+                    obj=field,
+                    id='fields.E306',
+                ),
+            ]
+            self.assertEqual(errors, expected)
+
+    def test_related_field_has_valid_related_name(self):
+        lowercase = 'a'
+        uppercase = 'A'
+        digit = 0
+
+        related_names = [
+            '%s_starts_with_lowercase' % lowercase,
+            '%s_tarts_with_uppercase' % uppercase,
+            '_starts_with_underscore',
+            'contains_%s_digit' % digit,
+            'ends_with_plus+',
+            '_',
+            '_+',
+            '+',
+        ]
+
+        class Parent(models.Model):
+            pass
+
+        for related_name in related_names:
+            Child = type(str('Child_%s') % str(related_name), (models.Model,), {
+                'parent': models.ForeignKey('Parent', related_name=related_name),
+                '__module__': Parent.__module__,
+            })
+
+            errors = Child.check()
+            self.assertFalse(errors)
+
 
 class AccessorClashTests(IsolatedModelsTestCase):