Go to Definition on destruction with the same name "does nothing"

[FMorschel]

If you have a code like the following:

class Class {
  int myField = 0;
}

void foo() {
  var instance = Class();
  if (instance case Class(:var myField)) {
    print(myField);
    print(myField);
  }
}

Using "Go to Definition" or CTRL + Click on myField at line 7 (inside if), it doesn't go to the definition in the class. Since it is also a variable definition itself, it doesn't think there is anything more to it.

By default, VS Code has a fallback at a definition (if you call that command again at that location) to show references, so it would show a list containing lines 8 and 9.

I'd like a way of getting back to the class definition for myField. In this case, this means that "Go to Definition" would take us to the definition and not "do nothing" to fall back to VS Code default.

CC @DanTup @bwilkerson @scheglov

[FMorschel]

https://dart-review.googlesource.com/c/sdk/+/404926

[bwilkerson]

While I agree that it would be useful to navigate to the implied getter that matches the variable name (myField), I am not convinced that we want to do so by navigating from the pattern variable to the field. The getter and the pattern variable are two distinct declarations, and I'm not sure it's a good idea to conflate them. (Although I will note that the 'occurrences' highlighting already conflates them when you click on the pattern variable's declaration.)

For example, consider

class Class {
  int myField = 0;
}

void foo() {
  var instance = Class();
  if (instance case Class(myField : var x)) {
    print(x);
    print(x);
  }
}

I wouldn't want to be able to navigate from the declaration of x to the declaration of myField, but semantically that's equivalent to what's being asked for here.

Interestingly, we made the decision to navigate to between the ( and : when going to references of the field, not to the pattern variable.

The support in this area is a bit inconsistent, but I don't know that we want to make the inconsistency worse.

[FMorschel]

If you test the CL, the code you suggested works as you'd expect. When you go to declaration of x it does so and the same for myField (going to the class member).

But if the declaration has the same name (as my example), if you ask for the definition inside the if (any print) it goes to the line of the if (where it binds the variable). And only then, if you ask again for the definition it takes you to the class member.

I'd say it is worse to need the full myField: var myField (which we can't add differently than manually or renaming twice - that can be a bit slow considering the shadowing analysis) for the ability to navigate than to add this functionality for the short version. I always catch myself trying to do that.

20250121_164513.mp4

[DanTup]

case Class(myField : var x)

I wouldn't want to be able to navigate from the declaration of x to the declaration of myField, but semantically that's equivalent to what's being asked for here.

To me, these don't feel equivalent. In the case of Class(:var myField), myField is also a "reference" (for some definition of the word) to the getter. You can't just change that name to myField2 because then it wouldn't match a getter. In my mind, myField in this case is both a declaration of myField and a "reference" to the getter (even if that's not technically accurate.. although I think I would probably want something to show up in Find References on the getter for similar reasons).

Whether we should support Go-to-Def like this though, I'm less sure - on one hand, it's the only way (unless it's also in surrounding code) to get to the getter to see what you're using (and the same goes for the hover - I'd love that to show both the getter dartdocs and some indication it's a pattern/declaration). On the other hand, if you are someone that uses the Go-to-Definition shortcut (F12) to Find References when you know something is a declaration (because it's easier to press than Shift+F12), you might invoke it here expecting references to it, and this would break that.

I don't know how many users use F12 to get references on a declaration (personally I always use the specific shortcuts and not the fallbacks) but it is functionality that exists and might be used, so I don't think making this change is completely free of potentially negative consequences/confusion.

I wonder if a simple compromise would be that hovering over myField here included something in the hover indicating it declares a new variable(?) for the value of [Class.getter] where that reference is a clickable link?

[bwilkerson]

I'd say it is worse to need the full myField: var myField (which we can't add differently than manually or renaming twice - that can be a bit slow considering the shadowing analysis) for the ability to navigate than to add this functionality for the short version.

I agree. I'm just trying to convince myself that this is either (a) consistent with the semantics of the language (which I think it isn't), (b) unlikely to create confusion in the user's mind about what the semantics are, or (c) the only reasonable choice to make given the semantics of the language and the limitations of the IDE support.

To me, these don't feel equivalent. In the case of Class(:var myField), myField is also a "reference" (for some definition of the word) to the getter.

The specification for patterns says:

As a convenience, the identifier can be omitted and inferred from pattern.

Maybe I'm reading that wrong, but that seems to me to imply that the name of the getter being referenced is semantically distinct from the name of the variable.

Nevertheless, I see where you're both coming from. Similar to the case of a default constructor, there's something in the semantics of the code that isn't explicit in the syntax of the code. I think we need to look at all such cases and see whether we can at least be consistent in the way we handle them.

I wonder if a simple compromise would be that hovering over myField here included something in the hover indicating it declares a new variable ...

I think that extending the hover text to make it more clear what the code is doing would be great, independent of the question of whether there's a way to link to the getter.