Repository navigation
allow django models to declare their own id primary key - #5477
dhitalkamal wants to merge 1 commit into
Conversation
pyrefly gives every Django model without its own primary key a made-up `id: int`, including AbstractBaseUser and PermissionsMixin. A user model that declares `id = UUIDField(primary_key=True)` was flagged as an inconsistent override of that made-up field. Django only adds the automatic `id` to the final model when no primary key is declared, so a child's own `id` primary key replaces it rather than overriding it. The override check now skips a parent's `id` when it was made up by pyrefly and the child's `id` is its primary key. An `id` written out in a parent is still checked. Fixes meta#4596
|
Hi @dhitalkamal! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary
If you subclass
AbstractBaseUserandPermissionsMixinand give your user model a UUID primary key calledid, pyrefly reportsbad-override-mutable-attributeon it. This is a really common Django setup, so it shows up a lot.The problem is that pyrefly adds a fake
id: intto every model that doesn't declare its own primary key, and that includes abstract bases likeAbstractBaseUser. So the user'sUUIDFieldgets compared against anintthat doesn't really exist. Django only adds the automaticidto the final model, and only if no primary key is declared.The fix: in the override check, skip the parent's
idif pyrefly made it up and the child'sidis its primary key. If a parent actually writes out anid, it still gets checked.I first thought about skipping abstract models, but django-stubs doesn't include
class Meta: abstract = Trueon these classes, so pyrefly can't tell they're abstract.I used Claude to help track down the cause and draft the fix and this description. I went through the code and ran the tests myself.
Fixes #4596
Test Plan
Added two tests in
pyrefly/lib/test/django/auto_generated_fields.rs:test_custom_id_pk_overrides_synthesized_id: the example from the issue, plus a user-defined abstract base. It failed before the fix with the same errors as in the issue, and passes now.test_non_pk_id_still_checked_against_synthesized_id: anidfield withoutprimary_key=Truestill gets an error, so the fix doesn't let everything through.Commands run:
cargo test -p pyrefly --lib test::django: 91 passedcargo test -p pyrefly --lib -- --skip test::cycles: 9783 passed, 0 failed.test::cycleshits a stack overflow in debug builds onmaintoo, so I skipped it.python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema --no-extension: clean