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

Fix AttrsInstance protocol check with cache #14551

Merged
merged 2 commits into from
Jan 30, 2023

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Jan 29, 2023

Use correct fullname for __attrs_attrs__ ClassVar to fix issue with warm cache.
Closes #14099

@github-actions

This comment has been minimized.

Comment on lines 68 to 74
[file m.py.2]
import attr
from c import Entry

def func(e: Entry) -> int:
attr.asdict(e)
return 2
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix does work when tested locally. However, I couldn't reproduce it with this test case although I though I should be able to. Maybe the cache isn't serialized for it?

Open to ideas how to fix the test case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to reproduce with types-attrs. Maybe try revendoring the attr fixture from typeshed from before python/typeshed#5585 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we already use a custom stub for the test from test-data/unit/lib-stub/attr. That shouldn't be the issue, I think.

Rather the problem only happens if c.py is deserialized from cache and I'm not sure that is happing here. Might be that just both c.py and m.py.2 get analyzed in the second run in which case the error won't show up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hauntsaninja Got it working 🎉 Moving AttrsInstance from lib-stub to the test file did the job. Without the change in plugins/attrs.py, the test case is now failing.

@cdce8p cdce8p mentioned this pull request Jan 29, 2023
17 tasks
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@hauntsaninja hauntsaninja merged commit 8e9f89a into python:master Jan 30, 2023
@cdce8p cdce8p deleted the fix-attrs-cache branch January 30, 2023 09:05
cdce8p added a commit to cdce8p/mypy that referenced this pull request Jan 30, 2023
…14551)

Use correct fullname for `__attrs_attrs__` ClassVar to fix issue with
warm cache.
Closes python#14099

(cherry picked from commit 8e9f89a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

attrs plugin fails to type check against AttrsInstance protocol with warm cache
2 participants