[Django] #32868: Add system check or warning for model fields shadowing methods / other attributes

4 views

Skip to first unread message

Django

unread,

Jun 21, 2021, 3:59:35 AM6/21/21

to django-...@googlegroups.com

#32868: Add system check or warning for model fields shadowing methods / other
attributes
------------------------------------------------+------------------------
Reporter: Adam Johnson | Owner: nobody
Type: New feature | Status: new
Component: Core (System checks) | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
It's possible to create a field name that replaces a Model method, for
example:

{{{
from django.db import models

class Book(models.Model):
delete = models.BooleanField(default=False)
}}}

Instances of this model then cannot be deleted with `instance.delete()`.
(They can be with `Model.delete(instance)`, but that's a really rare way
of using a method so I expect most developers would not think of that.)

Django could warn about this, at least for some key names that sound like
feasible field names, e.g. `delete`, `save`, `pk`, `clean`, ...

It's already not possible to create a field called `check` since the
system check `models.E020` warns if the `Model.check()` method has been
replaced, added in #23615. We could see this ticket as an extension of
that, "reserving" more names that Django may depend on.

--
Ticket URL: <https://code.djangoproject.com/ticket/32868>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Jun 22, 2021, 1:06:49 AM6/22/21

to django-...@googlegroups.com

#32868: Add system check or warning for model fields shadowing methods / other
attributes

-------------------------------------+-------------------------------------

Reporter: Adam Johnson | Owner: nobody

Type: New feature | Status: closed
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution: wontfix

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* cc: Carlton Gibson (added)
* status: new => closed
* resolution: => wontfix

Comment:

Thanks for this proposition, however I'm skeptical. I don't think it's
worth additional complexity and potential backward incompatibility. In
most of cases Django will already crash, e.g.
{{{
>>> obj.save()
TypeError: 'bool' object is not callable

>>> obj.delete()
TypeError: 'bool' object is not callable
}}}

The extra check added in #23615 was already a bit controversial (see
[https://github.com/django/django/pull/3362#discussion_r18836742
discussion]). You can raise the idea on the DevelopersMailingList to reach
a wider audience and see what other think.