Skip to content

Comments

[ty] Fix panic when attempting to provide autocompletions for an instance of a class that assigns attributes to self[0]#18707

Merged
AlexWaygood merged 3 commits intomainfrom
alex/fix-autocomplete-subscript-attrs
Jun 16, 2025
Merged

[ty] Fix panic when attempting to provide autocompletions for an instance of a class that assigns attributes to self[0]#18707
AlexWaygood merged 3 commits intomainfrom
alex/fix-autocomplete-subscript-attrs

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Jun 16, 2025

Summary

Ty currently panics when attempting to provide autocompletions in this scenario:

class Point:
    def orthogonal_direction(self):
        self[0].is_zero


def test_point(p2: Point):
    p2.<CURSOR>

The root cause of this issue is that we're incorrectly registering the [0].is_zero place as an "instance attribute" Place. Places should only be registered as instance attributes if they have the form self.<NAME>; self<SUBSCRIPT>.<NAME> does not constitute an instance attribute.

This panic was uncovered by #18705.

Test Plan

I added the above repro as a test to ty_ide/completions.rs

@AlexWaygood AlexWaygood requested a review from carljm as a code owner June 16, 2025 13:36
@AlexWaygood AlexWaygood added the bug Something isn't working label Jun 16, 2025
@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Jun 16, 2025
@AlexWaygood AlexWaygood mentioned this pull request Jun 16, 2025
Merged
@AlexWaygood AlexWaygood force-pushed the alex/fix-autocomplete-subscript-attrs branch from a0a0fd2 to bfea391 Compare June 16, 2025 13:39
@github-actions
Copy link
Contributor

github-actions bot commented Jun 16, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅

sharkdp
sharkdp approved these changes Jun 16, 2025
View reviewed changes
Copy link
Contributor

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Thank you!

I wondered if matches!(place_expr.sub_segments(), &[PlaceExprSubSegment::Member(_)]) should be a method on PlaceExpr, but it already has PlaceExpr::is_instance_attribute and PlaceExpr::is_instance_attribute_named with subtly different meanings. But maybe that's an argument for doing it (and clarifying the difference in doc comments)?

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jun 16, 2025

Thank you!

I wondered if matches!(place_expr.sub_segments(), &[PlaceExprSubSegment::Member(_)]) should be a method on PlaceExpr, but it already has PlaceExpr::is_instance_attribute and PlaceExpr::is_instance_attribute_named with subtly different meanings. But maybe that's an argument for doing it (and clarifying the difference in doc comments)?

Good call. I added some higher-level abstractions to place.rs, and lots of doc-comments to distinguish semantic-index-internal methods from crate-public API 👍

@AlexWaygood AlexWaygood enabled auto-merge (squash) June 16, 2025 21:51
@AlexWaygood AlexWaygood force-pushed the alex/fix-autocomplete-subscript-attrs branch from daaf192 to 8ee9597 Compare June 16, 2025 21:54
@AlexWaygood AlexWaygood merged commit 2b731d1 into main Jun 16, 2025
31 checks passed
@AlexWaygood AlexWaygood deleted the alex/fix-autocomplete-subscript-attrs branch June 16, 2025 21:58
@sharkdp
Copy link
Contributor

sharkdp commented Jun 17, 2025

Thank you for the update, this is great!

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

Reviewers

@sharkdp sharkdp sharkdp approved these changes

@carljm carljm Awaiting requested review from carljm carljm is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

@MichaReiser MichaReiser Awaiting requested review from MichaReiser MichaReiser is a code owner

Assignees

No one assigned

Labels

bug Something isn't working ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AlexWaygood @sharkdp