flag ambiguous protocol members without type annotations#3012
flag ambiguous protocol members without type annotations#3012knQzx wants to merge 2 commits intofacebook:mainfrom
Conversation
add UnannotatedProtocolMember error kind that fires when a protocol class body assigns a value to a member without an explicit type annotation - matching mypy's behavior of requiring all protocol members to have declared types.
|
Hey @knQzx, nice work on this. Parity with mypy regarding unannotated protocol members is definitely important for consistency across the ecosystem. I've looked through the summary, and the test plan seems solid especially verifying that existing protocol tests still pass. One quick question: for members assigned a value like x = None without an annotation, does the new UnannotatedProtocolMember error provide a clear suggestion to the user to add a type hint? I'm happy to pull this branch and verify it against a few edge cases (like nested Protocols) to make sure there aren't any weird regressions! |
|
thanks, yeah the error points to the unannotated member and suggests adding a type annotation |
|
Appreciate the quick response! Suggesting the type hint directly in the error message is a huge plus for developer experience. Parity with mypy on unannotated protocol members is a major step for making Pyrefly a solid drop-in replacement. Since the existing 34 tests are passing and the diagnostic is clear, this looks great to me. |
stroxler
left a comment
There was a problem hiding this comment.
This looks good to me, thanks! I'll try to get a second review and get it landed this week
|
@stroxler has imported this pull request. If you are a Meta employee, you can view this in D100014044. |
rchen152
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
I thought we were going to have this error code off by default per the discussion in the original ticket? |
summary
UnannotatedProtocolMembererror kind for protocol members assigned a value without an explicit type annotationtest plan
test_protocol_ambiguous_memberto expect the new error on unannotated assignments likex = Noneclass Ok(Protocol)) with annotated members to confirm no false positivesfixes #2925