Incorrect `bad-override` when inheriting from classes with existing `Meta` in django

[astronuttt]

Describe the Bug

I don't think this issue is only for django and Meta class, but in my case when I'm using the Meta on a class and then inherit from that class, I get a bad-override error.

example:

from django.db import models


class DateTimeMixin(models.Model):
    """Base abstract class with created_at and updated_at"""

    created_at = models.DateTimeField(auto_now_add=True, editable=False)
    updated_at = models.DateTimeField(auto_now=True)

    class Meta:
        abstract = True


class Invoice(DateTimeMixin):
    class Meta:
        verbose_name = "Invoice"
        verbose_name_plural = "Invoices"

we get a [bad-override] for the Meta:

Class member `Invoice.Meta` overrides parent class `DateTimeMixin` in an inconsistent manner
`Invoice.Meta` has type `type[Invoice.Meta]`, which is not assignable to `type[DateTimeMixin.Meta]`, the type of `DateTimeMixin.Meta` (Pyrefly bad-override)

Sandbox Link

No response

(Only applicable for extension issues) IDE Information

I'm using Zed 0.213.3 with pyrefly 0.42.1

[crimoniv]

Hey! I've hit the same warning, leaving my two cents on this:

Django's documentation on Meta inheritance, mentions two approaches:

• Skip Meta (re)declaration

  "... it sets abstract=False. This means that children of abstract base classes don’t automatically become abstract classes themselves."

• Inherit parent(s) Meta

  "... If the child wants to extend the parent’s Meta class, it can subclass it."

I've rarely found myself subclassing Meta instead of overriding it. Also, the wording "it can subclass it" suggests me that both approaches are right in this specific scenario, at least I've seen no drawback with either in my experience.

The error seems to favor inheritance, though. Does that mean overrides are discouraged?

[stroxler]

There's an off-the shelf answer to this, which is that

• Invoice.Meta must inherit from DateTimeMixin.Meta in order to be considered a subclass; this is almost certainly not going to change because subclassing is a runtime thing and this code is not doing it
• If Invoice.Meta is not a subtype of DateTimeMixin.Meta, then type[DateTimeMixin.Meta] is not assignable to type[DateTimeMixin.Meta], hence the inconsistent override error

So unless we special-case something about Django, Pyrefly is definitely right here in terms of Python static typing semantics. But of course Python static typing and the actual runtime behavior are not the same; Django predates typing, and this code works just fine as runtime.

An open question is whether we should explicitly loosen restrictions for the Meta override in django models. That's probably fairly doable and I suspect that it is the right usability tradeoff here and I'm curious what @rchen152 and @migeed-z think.

[rchen152]

Loosening the override check seems reasonable to me. I think it may be as simple as changing should_check_field_for_override_consistency to also take in the class metadata and return false when the class is a django model and the field name is "Meta": https://github.com/facebook/pyrefly/blob/d668be0912039a2dab14f1d87a3d95a79d85a514/pyrefly/lib/alt/class/class_field.rs#L2065

[migeed-z]

Loosening the override check seems reasonable to me. I think it may be as simple as changing should_check_field_for_override_consistency to also take in the class metadata and return false when the class is a django model and the field name is "Meta":

pyrefly/pyrefly/lib/alt/class/class_field.rs

Line 2065 in d668be0

fn should_check_field_for_override_consistency(&self, field_name: &Name) -> bool {

I agree with this as well

[migeed-z]

@astronuttt the fix is landing. Closing this for now but if any future issues arise, please let us know by reopening this issue or by opening a new issue. Thank you!