Occurrence now highlighted for formal parameter of a closure

[scheglov]

Looks like a migration bug, see that only entry declaration in occurrence highlighted.
But not the usage on the next 2 lines.

#### General info

- Dart 3.7.0-edge.7b4b704732bc63853293ac3eeaa453a44bb912ad (main) (Tue Dec 3 15:58:54 2024 +0000) on "macos_arm64"
- on macos / Version 15.3 (Build 24D60)
- locale is en-US

[FMorschel]

I'll take a look at this.

[FMorschel]

@scheglov can you provide either a public file that this occurs or if you can run on VS Code if you could run

and reproduce the error. That should give you a log file with what happened and a stack trace if something went wrong.

I also noticed the version you are using, this may have been solved by Dart-Code/Dart-Code#5369 (the result was the opposite but I had the default vscode behaviour of highlighting every word).

20250205_090004.mp4

Could you try with the latest version? I've tested downloading the latest from https://gsdview.appspot.com/dart-archive/channels/main/raw/latest/sdk/:

#### General info

- Dart 3.8.0-edge.49a9d42cfc08138b3d230296d704ae99c4ae1323 (main) (Wed Feb 5 03:04:50 2025 -0800) on "windows_x64"
- on windows / "Windows 11 Pro" 10.0 (Build 22631)
- locale is pt-BR

Either way, we don't have tests for parameters at pkg\analysis_server\test\lsp\document_highlights_test.dart and I believe we should add them, I'll open a CL later for this.

[FMorschel]

Here is the CL for the tests https://dart-review.googlesource.com/c/sdk/+/408040.

[FMorschel]

@DanTup, can you say if this is possibly related to your fix I mentioned above?

[DanTup]

@DanTup, can you say if this is possibly related to your fix I mentioned above?

If you mean 9820487 then I don't think so. I have made some other recent changes to highlights in https://dart-review.googlesource.com/c/sdk/+/406822 too, but I don't think it would have caused (or fixed) this.

I did try, but was unable to come up with an example that reproduced this (either in stable or bleeding edge). I don't think the code in the screenshot is from the SDK so I couldn't test with it directly. @scheglov are you able to share a code example that triggers this?

Either way, we don't have tests for parameters at pkg\analysis_server\test\lsp\document_highlights_test.dart and I believe we should add them, I'll open a CL later for this.

For some functionality the LSP tests don't cover all cases because there are protocol-agnostic tests - there might be tests in pkg/analysis_server/test/analysis/notification_occurrences_test.dart. Recently, I tend to add new tests to both though, as in some cases the LSP implementation has diverged a little (for example in the case of the CL above, there's a difference because LSP supports occurrences with different lengths, but the legacy protocol does not).

[DanTup]

Oh, I also just remembered that IntelliJ doesn't use the occurrences from the server, so if @scheglov was using IntelliJ here (which I think he might be from the folding icons?), it's possible this issue is in the Dart IntelliJ plugin and not server.

[FMorschel]

I can repro the bug on IntelliJ with this:

void f(void Function(int) _) {}

void g() => f((variable) {
  print(variable);
});

[FMorschel]

@alexander-doroshko in Dart-Code/Dart-Code/4454's comment (the comment mentioned above):

IntelliJ's Dart plugin doesn't use Occurrences from the server. Occurrences highlighting works using IntelliJ's internal (language-agnostic) mechanism, but based on the references (aka navigation) info from the server. So it works only when the reference and the target have the same text.

I think we want to switch to the Occurrences from the server eventually.

I don't suppose this change has happened yet. This was written on Apr 3, 2023.

[scheglov]

I think IntelliJ uses navigation information for this.
And using the example that you constructed previously, I see an issue with navigation.

void f(void Function(int) _) {}

void g() => f((variable) {
  print(variable);
});

Try navigating from variable inside print(variable).
It does not go to variable) {, it goes to the start of the file.

So, apparently the bug is in the navigation.

[DanTup]

So, apparently the bug is in the navigation.

Thanks, I see this too. The issue seems to be that ParameterElementImpl.nameOffset2 is null:

The implementation just looks like:

@override
int? nameOffset2;

Some of the other implementations look like this:

@override
// TODO(scheglov): make it a nullable field
int? get nameOffset2 => nameOffset;

Should we just do the same here? (I'm not sure what the comment means.. probably a reference to nameOffset not being nullable? I wonder if it should be something like nameOffset == -1 ? null : nameOffset until we can remove/update the original fields?

[FMorschel]

So basically the same problem as #60047. I'll see on the CL for it https://dart-review.googlesource.com/c/sdk/+/408140 to fix this too. Thanks!

[scheglov]

The closer we can make it toward the final implementation, the better :-)

We probably will not able to do it with the overrides, because there are actual fields in ParameterElementImpl.

[FMorschel]

After my changes, the bots showed that pkg\analyzer\test\src\summary\top_level_inference_test.dart has failing tests.

I ran locally TopLevelInferenceTest.test_initializer_additive and it seems that we were expecting:

accessors
  synthetic static get vPlusIntInt @-1
    reference: <testLibraryFragment>::@getter::vPlusIntInt
    enclosingElement3: <testLibraryFragment>
    returnType: int
  synthetic static set vPlusIntInt= @-1
    reference: <testLibraryFragment>::@setter::vPlusIntInt
    enclosingElement3: <testLibraryFragment>
    parameters
      requiredPositional _vPlusIntInt @-1
        type: int
    returnType: void

For a local variable vPlusIntInt, but now we are showing <null-name> inside the synthetic static set parameter name. I've not found yet where are we adding this text so I could trace that back. If anyone could help here I'd appreciate it.