Enforce annotation when inferred item in list, dict or set is object type #9051
Conversation
|
Thanks to the original creator of #6792 ! :) I haven't looked into the flag to disable this check as discussed in the issue as of now. Any suggestions as to where the code for it should go and/or how should I go about it? |
Maybe #6562 serves as an example of how to add a flag? Also, can you rename the PR to describe the new feature, and include "Fixes #3816." in the description? |
This was helpful, thanks! I added the flag and a corresponding test for it. |
This PR does the following: - Fixes failing tests from PR 6792 - Adds suugested annotation note - Adds annotation checker for sets - Adds tests
|
Hey there! Sorry I didn't keep track of this. Is this still needed? Should I rebase it? |
|
|
||
| rvalue_type = get_proper_type(rvalue_type) | ||
| if isinstance(rvalue_type, Instance): | ||
| if isinstance(rvalue, ListExpr): |
There was a problem hiding this comment.
I think it should be possible to reduce most of the duplicate between the three cases with an appropriate helper function.
There was a problem hiding this comment.
I tried this. The left over redundancy is due to the fact that the Dict case is handled a little differently from the List and Set case. Let me know if you think more could be done here.
|
Yeah, I'd appreciate that! I've left what I think is my main comment, so if you address that and rebase it, we should be able to get this merged. Thanks. |
|
Not saying we shouldn't do this, but since this has the potential to be a disruptive change, I ran mypy_primer on it: |
Yeah, I would definitely not take this if it wasn't behind a flag. But that does raise a question about whether it is worth adding a flag for this. @JukkaL @ilevkivskyi thoughts? |
mypy/options.py
Outdated
| @@ -136,6 +136,9 @@ def __init__(self) -> None: | |||
| # Apply strict None checking | |||
| self.strict_optional = True | |||
|
|
|||
| # Testing new flag | |||
There was a problem hiding this comment.
This comment doesn't make sense; I'd drop it and move this up with other related things
There was a problem hiding this comment.
Yes I must have left it there while debugging or something. I removed it.
mypy/main.py
Outdated
| forbidden_inference_group = parser.add_argument_group( | ||
| title='Handling forbidden inference related type checks', | ||
| description="Warn when infered type is List[object], Set[Object] or Dict[*, Object].") | ||
| add_invertible_flag('--suppress-need-type-annotation-errors', default=True, |
There was a problem hiding this comment.
Please make this more consistent with other options. How about warn-implicit-object-collection as the name for when warning is enabled.
There was a problem hiding this comment.
Changed now. I was unsure of the naming so I had left it to whatever first popped in my mind
af4b82e to
c710899
Compare
I did not get a chance to look into this. But I will once I set up
Maybe |
|
Closing due to inactivity. |
Fixes #3816