Refs #24227 -- Replaced M2M isinstance checks by field.many_to_many

Thanks Markus Holtermann, Collin Anderson and Tim Graham for the reviews.

django/contrib/admin/checks.py

@@ -104,8 +104,8 @@ class BaseModelAdminChecks(object):
             return refer_to_missing_field(field=field_name, option=label,
                                           model=model, obj=obj, id='admin.E002')
         else:
-            if not isinstance(field, (models.ForeignKey, models.ManyToManyField)):
-                return must_be('a ForeignKey or ManyToManyField',
+            if not field.many_to_many and not isinstance(field, models.ForeignKey):
+                return must_be('a foreign key or a many-to-many field',
                                option=label, obj=obj, id='admin.E003')
             else:
                 return []
@@ -218,11 +218,10 @@ class BaseModelAdminChecks(object):
                 # be an extra field on the form.
                 return []
             else:
-                if (isinstance(field, models.ManyToManyField) and
-                        not field.remote_field.through._meta.auto_created):
+                if field.many_to_many and not field.remote_field.through._meta.auto_created:
                     return [
                         checks.Error(
-                            "The value of '%s' cannot include the ManyToManyField '%s', "
+                            "The value of '%s' cannot include the many-to-many field '%s' "
                             "because that field manually specifies a relationship model."
                             % (label, field_name),
                             obj=obj.__class__,
@@ -295,8 +294,8 @@ class BaseModelAdminChecks(object):
             return refer_to_missing_field(field=field_name, option=label,
                                           model=model, obj=obj, id='admin.E019')
         else:
-            if not isinstance(field, models.ManyToManyField):
-                return must_be('a ManyToManyField', option=label, obj=obj, id='admin.E020')
+            if not field.many_to_many:
+                return must_be('a many-to-many field', option=label, obj=obj, id='admin.E020')
             else:
                 return []
 
@@ -389,23 +388,17 @@ class BaseModelAdminChecks(object):
         is a name of existing field and the field is one of the allowed types.
         """
 
-        forbidden_field_types = (
-            models.DateTimeField,
-            models.ForeignKey,
-            models.ManyToManyField
-        )
-
         try:
             field = model._meta.get_field(field_name)
         except FieldDoesNotExist:
             return refer_to_missing_field(field=field_name, option=label,
                                           model=model, obj=obj, id='admin.E027')
         else:
-            if isinstance(field, forbidden_field_types):
+            if field.many_to_many or isinstance(field, (models.DateTimeField, models.ForeignKey)):
                 return [
                     checks.Error(
                         "The value of '%s' refers to '%s', which must not be a DateTimeField, "
-                        "ForeignKey or ManyToManyField." % (label, field_name),
+                        "a foreign key, or a many-to-many field." % (label, field_name),
                         obj=obj.__class__,
                         id='admin.E028',
                     )
@@ -629,10 +622,10 @@ class ModelAdminChecks(BaseModelAdminChecks):
                         id='admin.E108',
                     )
                 ]
-            elif isinstance(field, models.ManyToManyField):
+            elif getattr(field, 'many_to_many', False):
                 return [
                     checks.Error(
-                        "The value of '%s' must not be a ManyToManyField." % label,
+                        "The value of '%s' must not be a many-to-many field." % label,
                         obj=obj.__class__,
                         id='admin.E109',
                     )

django/contrib/admin/options.py

@@ -133,8 +133,8 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
         if db_field.choices:
             return self.formfield_for_choice_field(db_field, request, **kwargs)
 
-        # ForeignKey or ManyToManyFields
-        if isinstance(db_field, (models.ForeignKey, models.ManyToManyField)):
+        # Foreign key or many-to-many fields
+        if db_field.many_to_many or isinstance(db_field, models.ForeignKey):
             # Combine the field kwargs with any options for formfield_overrides.
             # Make sure the passed in **kwargs override anything in
             # formfield_overrides because **kwargs is more specific, and should
@@ -145,7 +145,7 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
             # Get the correct formfield.
             if isinstance(db_field, models.ForeignKey):
                 formfield = self.formfield_for_foreignkey(db_field, request, **kwargs)
-            elif isinstance(db_field, models.ManyToManyField):
+            elif db_field.many_to_many:
                 formfield = self.formfield_for_manytomany(db_field, request, **kwargs)
 
             # For non-raw_id fields, wrap the widget with a wrapper that adds
@@ -1385,7 +1385,7 @@ class ModelAdmin(BaseModelAdmin):
             except FieldDoesNotExist:
                 continue
             # We have to special-case M2Ms as a list of comma-separated PKs.
-            if isinstance(f, models.ManyToManyField):
+            if f.many_to_many:
                 initial[k] = initial[k].split(",")
         return initial
 

django/contrib/sites/managers.py

@@ -34,10 +34,10 @@ class CurrentSiteManager(models.Manager):
                 )
             ]
 
-        if not isinstance(field, (models.ForeignKey, models.ManyToManyField)):
+        if not field.many_to_many and not isinstance(field, (models.ForeignKey)):
             return [
                 checks.Error(
-                    "CurrentSiteManager cannot use '%s.%s' as it is not a ForeignKey or ManyToManyField." % (
+                    "CurrentSiteManager cannot use '%s.%s' as it is not a foreign key or a many-to-many field." % (
                         self.model._meta.object_name, field_name
                     ),
                     obj=self,

django/db/migrations/autodetector.py

@@ -799,8 +799,7 @@ class MigrationAutodetector(object):
         # You can't just add NOT NULL fields with no default or fields
         # which don't allow empty strings as default.
         preserve_default = True
-        if (not field.null and not field.has_default() and
-                not isinstance(field, models.ManyToManyField) and
+        if (not field.null and not field.has_default() and not field.many_to_many and
                 not (field.blank and field.empty_strings_allowed)):
             field = field.clone()
             field.default = self.questioner.ask_not_null_addition(field_name, model_name)
@@ -861,19 +860,13 @@ class MigrationAutodetector(object):
             old_field_dec = self.deep_deconstruct(old_field)
             new_field_dec = self.deep_deconstruct(new_field)
             if old_field_dec != new_field_dec:
-                both_m2m = (
-                    isinstance(old_field, models.ManyToManyField) and
-                    isinstance(new_field, models.ManyToManyField)
-                )
-                neither_m2m = (
-                    not isinstance(old_field, models.ManyToManyField) and
-                    not isinstance(new_field, models.ManyToManyField)
-                )
+                both_m2m = old_field.many_to_many and new_field.many_to_many
+                neither_m2m = not old_field.many_to_many and not new_field.many_to_many
                 if both_m2m or neither_m2m:
                     # Either both fields are m2m or neither is
                     preserve_default = True
                     if (old_field.null and not new_field.null and not new_field.has_default() and
-                            not isinstance(new_field, models.ManyToManyField)):
+                            not new_field.many_to_many):
                         field = new_field.clone()
                         new_default = self.questioner.ask_not_null_alteration(field_name, model_name)
                         if new_default is not models.NOT_PROVIDED:

django/forms/models.py

@@ -79,8 +79,6 @@ def model_to_dict(instance, fields=None, exclude=None):
     fields will be excluded from the returned dict, even if they are listed in
     the ``fields`` argument.
     """
-    # avoid a circular import
-    from django.db.models.fields.related import ManyToManyField
     opts = instance._meta
     data = {}
     for f in chain(opts.concrete_fields, opts.virtual_fields, opts.many_to_many):
@@ -90,7 +88,7 @@ def model_to_dict(instance, fields=None, exclude=None):
             continue
         if exclude and f.name in exclude:
             continue
-        if isinstance(f, ManyToManyField):
+        if f.many_to_many:
             # If the object doesn't have a primary key yet, just use an empty
             # list for its m2m fields. Calling f.value_from_object will raise
             # an exception.

docs/ref/checks.txt

@@ -271,8 +271,8 @@ with the admin site:
 * **admin.E001**: The value of ``raw_id_fields`` must be a list or tuple.
 * **admin.E002**: The value of ``raw_id_fields[n]`` refers to ``<field name>``,
   which is not an attribute of ``<model>``.
-* **admin.E003**: The value of ``raw_id_fields[n]`` must be a ``ForeignKey`` or
-  ``ManyToManyField``.
+* **admin.E003**: The value of ``raw_id_fields[n]`` must be a foreign key or
+  a many-to-many field.
 * **admin.E004**: The value of ``fields`` must be a list or tuple.
 * **admin.E005**: Both ``fieldsets`` and ``fields`` are specified.
 * **admin.E006**: The value of ``fields`` contains duplicate field(s).
@@ -284,7 +284,7 @@ with the admin site:
   ``fields``.
 * **admin.E012**: There are duplicate field(s) in ``fieldsets[n][1]``.
 * **admin.E013**: ``fields[n]/fieldsets[n][m]`` cannot include the
-  ``ManyToManyField`` ``<field name>``, because that field manually specifies a
+  many-to-many field ``<field name>``, because that field manually specifies a
   relationship model.
 * **admin.E014**: The value of ``exclude`` must be a list or tuple.
 * **admin.E015**: The value of ``exclude`` contains duplicate field(s).
@@ -294,7 +294,7 @@ with the admin site:
 * **admin.E019**: The value of ``filter_vertical[n]/filter_vertical[n]`` refers
   to ``<field name>``, which is not an attribute of ``<model>``.
 * **admin.E020**: The value of ``filter_vertical[n]/filter_vertical[n]`` must
-  be a ``ManyToManyField``.
+  be a many-to-many field.
 * **admin.E021**: The value of ``radio_fields`` must be a dictionary.
 * **admin.E022**: The value of ``radio_fields`` refers to ``<field name>``,
   which is not an attribute of ``<model>``.
@@ -308,8 +308,8 @@ with the admin site:
 * **admin.E027**: The value of ``prepopulated_fields`` refers to
   ``<field name>``, which is not an attribute of ``<model>``.
 * **admin.E028**: The value of ``prepopulated_fields`` refers to
-  ``<field name>``, which must not be a ``DateTimeField``, ``ForeignKey`` or
-  ``ManyToManyField``.
+  ``<field name>``, which must not be a ``DateTimeField``, a foreign key or a
+  many-to-many field.
 * **admin.E029**: The value of ``prepopulated_fields[<field name>]`` must be a
   list or tuple.
 * **admin.E030**: The value of ``prepopulated_fields`` refers to
@@ -343,7 +343,7 @@ with the admin site:
   which is not a callable, an attribute of ``<ModelAdmin class>``, or an
   attribute or method on ``<model>``.
 * **admin.E109**: The value of ``list_display[n]`` must not be a
-  ``ManyToManyField``.
+  many-to-many field.
 * **admin.E110**: The value of ``list_display_links`` must be a list, a tuple,
   or ``None``.
 * **admin.E111**: The value of ``list_display_links[n]`` refers to ``<label>``,
@@ -562,7 +562,7 @@ The following checks are performed on any model using a
 * **sites.E001**: ``CurrentSiteManager`` could not find a field named
   ``<field name>``.
 * **sites.E002**: ``CurrentSiteManager`` cannot use ``<field>`` as it is not a
-  ``ForeignKey`` or ``ManyToManyField``.
+  foreign key or a many-to-many field.
 
 Database
 --------

tests/admin_checks/tests.py

@@ -583,7 +583,7 @@ class SystemChecksTestCase(SimpleTestCase):
         errors = BookAdmin(Book, AdminSite()).check()
         expected = [
             checks.Error(
-                "The value of 'fields' cannot include the ManyToManyField 'authors', "
+                "The value of 'fields' cannot include the many-to-many field 'authors' "
                 "because that field manually specifies a relationship model.",
                 obj=BookAdmin,
                 id='admin.E013',
@@ -601,8 +601,8 @@ class SystemChecksTestCase(SimpleTestCase):
         errors = FieldsetBookAdmin(Book, AdminSite()).check()
         expected = [
             checks.Error(
-                "The value of 'fieldsets[1][1][\"fields\"]' cannot include the ManyToManyField "
-                "'authors', because that field manually specifies a relationship model.",
+                "The value of 'fieldsets[1][1][\"fields\"]' cannot include the many-to-many field "
+                "'authors' because that field manually specifies a relationship model.",
                 obj=FieldsetBookAdmin,
                 id='admin.E013',
             )

tests/model_meta/tests.py

@@ -33,8 +33,7 @@ class OptionsBaseTests(SimpleTestCase):
             model = None
 
         field = relation if direct else relation.field
-        m2m = isinstance(field, related.ManyToManyField)
-        return relation, model, direct, m2m
+        return relation, model, direct, bool(field.many_to_many)  # many_to_many can be None
 
 
 class GetFieldsTests(OptionsBaseTests):
@@ -69,7 +68,7 @@ class DataTests(OptionsBaseTests):
 
     def test_local_fields(self):
         def is_data_field(f):
-            return isinstance(f, Field) and not isinstance(f, related.ManyToManyField)
+            return isinstance(f, Field) and not f.many_to_many
 
         for model, expected_result in TEST_RESULTS['local_fields'].items():
             fields = model._meta.local_fields

tests/modeladmin/tests.py

@@ -633,7 +633,7 @@ class RawIdCheckTests(CheckTestCase):
 
         self.assertIsInvalid(
             ValidationTestModelAdmin, ValidationTestModel,
-            "The value of 'raw_id_fields[0]' must be a ForeignKey or ManyToManyField.",
+            "The value of 'raw_id_fields[0]' must be a foreign key or a many-to-many field.",
             'admin.E003')
 
     def test_valid_case(self):
@@ -826,7 +826,7 @@ class FilterVerticalCheckTests(CheckTestCase):
 
         self.assertIsInvalid(
             ValidationTestModelAdmin, ValidationTestModel,
-            "The value of 'filter_vertical[0]' must be a ManyToManyField.",
+            "The value of 'filter_vertical[0]' must be a many-to-many field.",
             'admin.E020')
 
     def test_valid_case(self):
@@ -863,7 +863,7 @@ class FilterHorizontalCheckTests(CheckTestCase):
 
         self.assertIsInvalid(
             ValidationTestModelAdmin, ValidationTestModel,
-            "The value of 'filter_horizontal[0]' must be a ManyToManyField.",
+            "The value of 'filter_horizontal[0]' must be a many-to-many field.",
             'admin.E020')
 
     def test_valid_case(self):
@@ -960,7 +960,7 @@ class PrepopulatedFieldsCheckTests(CheckTestCase):
         self.assertIsInvalid(
             ValidationTestModelAdmin, ValidationTestModel,
             ("The value of 'prepopulated_fields' refers to 'users', which must not be "
-             "a DateTimeField, ForeignKey or ManyToManyField."),
+             "a DateTimeField, a foreign key, or a many-to-many field."),
             'admin.E028')
 
     def test_valid_case(self):
@@ -998,7 +998,7 @@ class ListDisplayTests(CheckTestCase):
 
         self.assertIsInvalid(
             ValidationTestModelAdmin, ValidationTestModel,
-            "The value of 'list_display[0]' must not be a ManyToManyField.",
+            "The value of 'list_display[0]' must not be a many-to-many field.",
             'admin.E109')
 
     def test_valid_case(self):

tests/sites_framework/tests.py

@@ -62,7 +62,7 @@ class CurrentSiteManagerChecksTests(SimpleTestCase):
         expected = [
             checks.Error(
                 "CurrentSiteManager cannot use 'ConfusedArticle.site' as it is "
-                "not a ForeignKey or ManyToManyField.",
+                "not a foreign key or a many-to-many field.",
                 obj=ConfusedArticle.on_site,
                 id='sites.E002',
             )