Skip to content

[ty] Return all attribute definitions for goto definition#24332

Open
MichaReiser wants to merge 2 commits intomainfrom
micha/goto-def-all-attribute-members
Open

[ty] Return all attribute definitions for goto definition#24332
MichaReiser wants to merge 2 commits intomainfrom
micha/goto-def-all-attribute-members

Conversation

@MichaReiser
Copy link
Copy Markdown
Member

Summary

Instead of only returning the first definition, return all definitions in the same scope.

This ensures that ty returns all definitions of x in the following example instead of just the first:

class Test:
	x: int
	x: int

test = Test()
test.x

This matches the behavior of standalone variables:

x: int = 1
x: int = 2

x

also shows both definitions of x

Test Plan

Added tests

@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Mar 31, 2026
@MichaReiser MichaReiser requested a review from BurntSushi as a code owner March 31, 2026 14:46
Comment on lines +2622 to +2629
info[rename]: Rename symbol (found 5 locations)
--> main.py:4:13
|
2 | from lib import Foo
3 |
4 | print(Foo().my_property)
| ^^^^^^^^^^^
5 | Foo().my_property = 56
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This already worked before because rename tests if the definitions of the renamed symbol and the current symbol's definitions intersect. But I felt it's still worth adding a test for it.

@MichaReiser MichaReiser assigned sharkdp and unassigned dcreager Mar 31, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 31, 2026

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 86.61%. The percentage of expected errors that received a diagnostic held steady at 81.56%. The number of fully passing files held steady at 70/132.

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 31, 2026

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 31, 2026

ecosystem-analyzer results

Lint rule Added Removed Changed
invalid-await 0 40 0
invalid-return-type 0 1 0
Total 0 41 0

Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.

Full report with detailed diff (timing results)

@MichaReiser MichaReiser changed the title [ty] Return all attribute definitions for goto definition" [ty] Return all attribute definitions for goto definition Mar 31, 2026
@carljm carljm removed their request for review March 31, 2026 14:51
Comment on lines +2074 to +2077
3 | a: str
| -
4 | a: str
| -
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, interesting. It seems a bit strange to me, but as you pointed out, this is now consistent with what we do for non-class scopes. And it seems like the right choice for renaming, and of course for properties.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants