[ty] Ignore and reject annotations on non-name targets#25324
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 89.70%. The percentage of expected errors that received a diagnostic held steady at 86.99%. The number of fully passing files held steady at 91/134. |
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
unused-type-ignore-comment |
0 | 4 | 0 |
| Total | 0 | 4 | 0 |
Raw diff:
pyppeteer (https://github.com/pyppeteer/pyppeteer)
- pyppeteer/connection.py:99:53 warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive
- pyppeteer/connection.py:100:40 warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive
- pyppeteer/connection.py:228:53 warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive
- pyppeteer/connection.py:229:40 warning[unused-type-ignore-comment] Unused blanket `type: ignore` directive91aa62f to
0e6abf3
Compare
| def __init__(self) -> None: | ||
| this = self | ||
| # error: [invalid-type-form] | ||
| # error: [unresolved-attribute] |
There was a problem hiding this comment.
These cases are a bit weird, but Mypy and Pyright also reject this.
| def set_attribute(value: str): | ||
| # error: [invalid-type-form] | ||
| # error: [unresolved-attribute] | ||
| self.x: str = value |
There was a problem hiding this comment.
Pyright rejects this, but Mypy accepts it as defining C.x... Maybe we should accept this?
There was a problem hiding this comment.
Pyrefly also supports this. I think it's fine to wait for a user request. It seems easy to work around with a class-level annotation.
0e6abf3 to
07146db
Compare
|
@sharkdp will be so happy to finally have a PR to review ;) |
| self.x: "int" | "str" = 42 | ||
|
|
||
| d = {} | ||
| # error: [invalid-type-form] |
There was a problem hiding this comment.
It seems slightly wrong to me to use invalid-type-form as the rule code. The "type form" itself is not invalid, it's in an invalid position. On the other hand, the rule description (https://docs.astral.sh/ty/reference/rules/#invalid-type-form) could be interpreted in a way that would include this case.
I think I'm okay with reusing the code, especially since this seems like a mistake that does not come up very frequently.
|
|
||
| @classmethod | ||
| def initialize(cls): | ||
| cls.class_attr: int = 1 # fine |
There was a problem hiding this comment.
to match the description above, can we also include
| cls.class_attr: int = 1 # fine | |
| cls.class_attr: str = None. # snapshot: invalid-assignment | |
| cls.class_attr2: int = 1 # fine |
| def set_attribute(value: str): | ||
| # error: [invalid-type-form] | ||
| # error: [unresolved-attribute] | ||
| self.x: str = value |
There was a problem hiding this comment.
Pyrefly also supports this. I think it's fine to wait for a user request. It seems easy to work around with a class-level annotation.
07146db to
6abf7b3
Compare
## Summary Like Mypy and Pyright, we now ignore annotations on non-name (i.e., subscript and attribute) targets, and emit a diagnostic for the invalid annotation -- with the exception of annotated assignments on `self` and class attributes (i.e., receivers). Closes astral-sh/ty#509.
) ## Summary We now emit `invalid-type-form` for an invalid annotation on a non-name target (#25324)... But it turns out that we still allow that annotation to participate in downstream operations, since we create an annotated-assignment definition for it during the indexing pass. For example: ```python def accepts_inner(inner: dict[str, int]): ... def _(x: dict[str, object]): x["inner"]: dict[str, float | str] = {"a": 1, "b": "a"} # error: [invalid-type-form] x["inner"]["c"] = 1.0 # error: [invalid-assignment] because we infer from the RHS, not the annotation x["inner"] = {"inner": {"a": 1}} accepts_inner(**x["inner"]) # ok ``` The rejected annotation should not widen the dictionary assigned to `x["inner"]`, nor should it constrain the later replacement. (The same issue applies to rejected attribute annotations, like `obj.inner: T`.) We now represent each syntactically indexed declaration outcome as either `Declared(...)` or `Rejected`. Declaration consumers only use `Declared(...)`.
Summary
Like Mypy and Pyright, we now ignore annotations on non-name (i.e., subscript and attribute) targets, and emit a diagnostic for the invalid annotation -- with the exception of annotated assignments on
selfand class attributes (i.e., receivers).Closes astral-sh/ty#509.