Find References doesn't work when selection covers an entire symbol

[DanTup]

Double-clicking on a method name then pressing Shift+F12 doesn't find anything:

(same goes for putting the caret at the end of the name, which is likely the same offset for this selection).

[DanTup]

@bwilkerson I'm trying to debug this and found that NodeLocator() is locating the argument list here in the case where the caret is in front of it (which seems reasonable and maybe should be handled specially). However, I noticed that when the caret is in the name, we don't locate the name, but rather the whole member declaration. This seemed odd to me so I wanted to check this is correct before making any changes.

For example, if I run this test code:

@soloTest
  void test_danny() async {
    await resolveTestCode('''
void aaa() {}
''');
    result.unit.accept(MyVisitor());
  }

Where MyVisitor looks like this:

class MyVisitor extends UnifyingAstVisitor<void> {
  @override
  void visitNode(AstNode node) {
    print('Visiting (${node.offset}) $node');
    super.visitNode(node);
  }
}

I get the following output:

Visiting (0) void aaa() {}
Visiting (0) void aaa() {}
Visiting (0) void
Visiting (0) void
Visiting (8) () {}
Visiting (8) ()
Visiting (11) {}
Visiting (11) {}

What seems off to me here is that there is no visit of the aaa token the starts at offset 5, but I was expecting one. Does this seem like a bug? (anecdotally, I was sure the behaviour in this issue worked in the past, I feel like I do this all the time, but there are no tests with the caret at the end of the identifier so I may be misremembering 😄).

[DanTup]

Testing the original behaviour up top with stable 2.18.0, it definitely does work, so I think something has changed. I'll try to bisect this tomorrow to figure out what changed (assuming nothing stands out and you might know what it is already).

[bwilkerson]

AstVisitors only visit AstNodes. It used to be the case that the names in declarations were represented as SimpleIdentifier nodes. But we decided that the names aren't expressions, so we replaced all of the references to the nodes with references to the tokens. Your code isn't visiting aaa because it isn't a node anymore. Therefore, the closest node that NodeLocator can return is the declaration that holds onto the token.

[DanTup]

Aah, you warned me that was coming, makes sense now.

It's possible this affects more than just Navigation then - previously the offset between name and args (foo^() would have located the name node but now it locates the argument list. Anything that doesn't handle this specifically might fail in this case (when double-clicking a symbol to select it, VS Code uses the end as the offset for requests like this).

I'll fix navigation and check other functionality for similar issues. Thanks!

Edit: I wrote "argument" many times above, but while testing realised that (as you stated) this change only affected declarations/parameters (identifiers in front of argument lists are still tokens and don't seem affected).

[bwilkerson]

It's possible this affects more than just Navigation then ...

Yes, lots of code might potentially be broken. We relied heavily on test cases to catch breakage, but tests can't provide complete coverage.

[DanTup]

Fixed by dart-lang/sdk@d83cb4e.