Relax Y011, Y014 and Y015 to allow default values in many contexts #326
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The typeshed_primer diff won't be very helpful here, since we've already switched off Y011 and Y015 in typeshed's I think these are all "true positives". I'll file a PR to fix them over at typeshed, if others agree. |
This comment has been minimized.
This comment has been minimized.
pyi.py
Outdated
| if not is_special_assignment: | ||
| self._check_for_type_aliases(node, target, assignment) | ||
| if isinstance( | ||
| assignment, (ast.Dict, ast.List, ast.Tuple, ast.Set, ast.JoinedStr) |
There was a problem hiding this comment.
Why doesn't this use _is_valid_stub_default? There are a lot of other expression nodes.
There was a problem hiding this comment.
Yup. But for assignments, we want to allow much more than just the things we allow in default values for parameters. All of the following are valid in a stub:
T = TypeVar("T") # ast.Call
def foo(): ...
bar = foo # ast.Name
class Spam:
def eggs(self): ...
ham = Spam.eggs # ast.Attribute
baz = Literal["foo"] # ast.Subscript — this should trigger Y026 but not Y015There was a problem hiding this comment.
I see, but this still allows a bunch of nodes that have little conceivable use, like Lambda and SetComp. Might be better to go with an allowlist instead of a blocklist.
There was a problem hiding this comment.
Hmm, fair enough, I'll rework.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AlexWaygood
commented
Jan 16, 2023
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
JelleZijlstra
approved these changes
Jan 16, 2023
pyi.py
Outdated
| def _is_valid_assignment_value(self, node: ast.expr) -> bool: | ||
| return ( | ||
| isinstance(node, (ast.Call, ast.Name, ast.Attribute, ast.Subscript)) | ||
| or (isinstance(node, ast.BinOp) and isinstance(node.op, ast.BitOr)) |
There was a problem hiding this comment.
For extra paranoia could check that the operands to | are themselves valid, but fine to omit that.
There was a problem hiding this comment.
Oh good idea, I'll do that
|
|
||
| is_special_assignment = isinstance( | ||
| node_target, ast.Name | ||
| ) and _is_assignment_which_must_have_a_value( |
There was a problem hiding this comment.
this is not Black's finest hour
This comment has been minimized.
This comment has been minimized.
|
This change has no effect on typeshed. 🤖🎉 |
JelleZijlstra
approved these changes
Jan 16, 2023
JelleZijlstra
pushed a commit
that referenced
this pull request
Jan 28, 2023
…class namespaces where the assignments don't have type annotations (#339) Following #326, we now allow constructs such as these in a stub file: ```python x = 1 y = "foo" z = b"bar" ``` It's good that we now allow variables to have default values. However, the above constructs are problematic in a stub, as we're leaving it up to the type checker to make inferences about the types of these variables, which can have unpredictable consequences. For example, the `x` variable could be inferred to be of type `int`, `Final[int]`, `Literal[1]`, or `Final[Literal[1]]`. In a class namespace, the problem is even worse -- the `eggs` variable in the following snippet could reasonably be inferred as being of type `str`, `ClassVar[str]`, `Final[str]`, `Literal["EGGS"]`, `ClassVar[Literal["EGGS"]]`, or `Final[Literal["EGGS"]]`: ```python class Whatever: eggs = "EGGS" ``` This PR introduces Y052, enforcing type annotations for assignments to simple constants.
56 tasks
sbdchd
added a commit
to sbdchd/ruff
that referenced
this pull request
Feb 26, 2023
Implement PYI011 and PYI014 with the latest changes: PyCQA/flake8-pyi#326 PyCQA/flake8-pyi#316 rel: astral-sh#848 rel: https://github.com/PyCQA/flake8-pyi/blob/4212bec43dbc4020a59b90e2957c9488575e57ba/pyi.py#L718
charliermarsh
pushed a commit
to astral-sh/ruff
that referenced
this pull request
Feb 26, 2023
Implement PYI011 and PYI014 with the latest changes: PyCQA/flake8-pyi#326 PyCQA/flake8-pyi#316 rel: #848 rel: https://github.com/PyCQA/flake8-pyi/blob/4212bec43dbc4020a59b90e2957c9488575e57ba/pyi.py#L718
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #316, among other things.