[ty] Support declaration-only attributes #19048
Conversation
iyakushev
requested review from
carljm,
AlexWaygood,
sharkdp and
dcreager
as code owners
|
|
Thank you for working on this! I am planning to take a closer look tomorrow. It looks like this leads to a lot new false positives on ecosystem projects (see the auto-generated mypy_primer comment). We should probably try to find out what causes them (+ fix them and write a few more test cases here). |
Yes. Feel free to add some new sections in https://github.com/astral-sh/ruff/blob/main/crates/ty_python_semantic/resources/mdtest/attributes.md
That particular part will change with #18955 where we basically decide not to track possible unboundness (and with your PR: undeclaredness) anymore for implicit instance attributes. You could maybe try rebasing your branch on top of that already. |
iyakushev
force-pushed
the
support-decl-only
branch
from
b5033c4 to
6dac224
Compare
|
@sharkdp Please take a look when available. It seems to me that most of the errors (false positives) in the original implementation come from class instance access to its attributes via class C:
def __init__(self):
self.foo: list[int] = []
def bar(self):
print(self.foo) # Previously possibly-unbound I've added a few mdtest to validate this (and marked them with TODO). There's definitely something wrong; either its beyond the original scope of the issue, or I'm misusing something. Either way, your guidance would be very valuable 😃 |
I don't understand? Non of the new MD tests seems to show some kind of possibly-unbound false positive?
I think we should switch that and check declarations first. If we have a declaration, we should prefer it over any bindings. |
Sorry for the ambiguity in my previous message. I've tried to replicate the warnings I saw in I've moved the declaration check above bindings and fixed another potential issue with Class obj/instance access to attributes. In my previous attempt instance attributes were leaking into class objects even if they were declared only in Could we, perhaps, run |
CodSpeed WallTime Performance ReportMerging #19048 will not alter performanceComparing Summary
|
iyakushev
changed the title
iyakushev
changed the title
|
Thank you for taking time to review this! |
sharkdp
force-pushed
the
support-decl-only
branch
from
884b766 to
a6206f1
Compare
…c_tokens * 'main' of https://github.com/astral-sh/ruff: (27 commits) [ty] First cut at semantic token provider (astral-sh#19108) [`flake8-simplify`] Make example error out-of-the-box (`SIM116`) (astral-sh#19111) [`flake8-use-pathlib`] Make example error out-of-the-box (`PTH210`) (astral-sh#19189) [`flake8-use-pathlib`] Add autofixes for `PTH203`, `PTH204`, `PTH205` (astral-sh#18922) [`flake8-type-checking`] Fix syntax error introduced by fix (`TC008`) (astral-sh#19150) [`flake8-pyi`] Make example error out-of-the-box (`PYI007`, `PYI008`) (astral-sh#19103) Update Rust crate indicatif to 0.18.0 (astral-sh#19165) [ty] Add separate CI job for memory usage stats (astral-sh#19134) [ty] Add documentation for server traits (astral-sh#19137) Rename to `SessionSnapshot`, move unwind assertion closer (astral-sh#19177) [`flake8-type-checking`] Make example error out-of-the-box (`TC001`) (astral-sh#19151) [ty] Bare `ClassVar` annotations (astral-sh#15768) [ty] Re-enable multithreaded pydantic benchmark (astral-sh#19176) [ty] Implement equivalence for protocols with method members (astral-sh#18659) [ty] Use RHS inferred type for bare `Final` symbols (astral-sh#19142) [ty] Support declaration-only attributes (astral-sh#19048) [ty] Sync vendored typeshed stubs (astral-sh#19174) Update dependency pyodide to ^0.28.0 (astral-sh#19164) Update NPM Development dependencies (astral-sh#19170) Update taiki-e/install-action action to v2.56.7 (astral-sh#19169) ...
Summary
Following ty issue #698 this PR adds support for declarations.
I'd say the implementation is rather naive, so I would use it as a starting point for a gradual improvement. I tried to check declarations before bindings and it seemed to resolve the issue.
closes #698
Test Plan
Tested against mdtest (specifically attributes).