Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: related fields raise [attr-defined] #1872

Closed
WhyNotHugo opened this issue Dec 12, 2023 · 3 comments · Fixed by #1877
Closed

Regression: related fields raise [attr-defined] #1872

WhyNotHugo opened this issue Dec 12, 2023 · 3 comments · Fixed by #1877
Labels
bug Something isn't working regression Behavior has changed for worse with a release

Comments

@WhyNotHugo
Copy link
Contributor

Bug report

With django-stubs==4.2.7, reports [attr-defined] for related fields of models. This works fine with django-stubs==4.2.6.

What's wrong

Related fields are reported as undefined.

How is that should be

Related fields should raise no error.

System information

  • OS: Alpine Linux Edge
  • python version: 3.10 or 3.11
  • django version: 4.2.8
  • mypy version: mypy 1.6.1 (compiled: no)
  • django-stubs version: 4.2.7
  • django-stubs-ext version: 4.2.7

Reproduction scenario

This can easiest be reproduced with this repository:

https://github.com/WhyNotHugo/django-afip.git

Currently, main is 3fcc7d4e3d3d0b8b201f9bc7ab35f52c003a852c. Relevant dependencies can be installed with pip install -e '.[dev]'.

> mypy .
WARNING:root:No DATABASE_URL environment variable set, and so no databases setup
django_afip/pdf.py:59: error: "Receipt" has no attribute "validation"; maybe "validate"?  [attr-defined]
django_afip/pdf.py:195: error: "Receipt" has no attribute "entries"  [attr-defined]
django_afip/models.py:1250: error: "Receipt" has no attribute "validation"; maybe "validate"?  [attr-defined]
django_afip/models.py:1308: error: "Receipt" has no attribute "validation"; maybe "validate"?  [attr-defined]
django_afip/admin.py:269: error: "Receipt" has no attribute "validation"; maybe "validate"?  [attr-defined]
Found 5 errors in 3 files (checked 61 source files)
> pip install "django-stubs[compatible-mypy]==4.2.6"
WARNING:root:No DATABASE_URL environment variable set, and so no databases setup
Success: no issues found in 61 source files

As can be seen, this was not a problem with 4.2.6.

@WhyNotHugo WhyNotHugo added the bug Something isn't working label Dec 12, 2023
WhyNotHugo added a commit to WhyNotHugo/django-afip that referenced this issue Dec 12, 2023
Works around regression in latest release.

See: typeddjango/django-stubs#1872
@intgr intgr added the regression Behavior has changed for worse with a release label Dec 12, 2023
@flaeppe
Copy link
Member

flaeppe commented Dec 12, 2023

Thanks for reporting.

I ran git bisect and managed to pin point the issue being introduced in 53cdbe4 (#1805)

@flaeppe
Copy link
Member

flaeppe commented Dec 12, 2023

@WhyNotHugo I think I found the issue. It's a bit of a hidden one, this: #1802

I found the following ManyToManyField on your Receipt model: https://github.com/WhyNotHugo/django-afip/blob/2ba5a097d00c1f9de174678d14bd0d1eac696377/django_afip/models.py#L1198-L1203

If you apply the patch in #1802 (comment) ("Receipt"-> "afip.Receipt" I believe) you should get successful runs again.

Although there's still improvements we want from this report. The not so obvious issue is that our plugin processing of the model bails hard and almost invisibly on 1 of the model attributes, which skips any remaining correct or incorrect ones.

We should be a bit more graceful and attempt to process all attributes individually and bail if we encountered any we couldn't resolve appropriately. That would also have been a resolution to this issue, essentially some form of "decoupling" within mypy_django_plugin.transformers.models.AddReverseLookups.run_with_model_cls, which can be seen was changed in #1805

@WhyNotHugo
Copy link
Contributor Author

I think it is safe to close this issue in favour of the ones that you've mentioned.

Thanks for bisecting this and thanks for the detailed explanation on the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression Behavior has changed for worse with a release
Development

Successfully merging a pull request may close this issue.

3 participants