[M] Change in dart/sdk[main]: [DAS] Fixes occurrences for declaring parameters
FMorschel (Gerrit)
FMorschel has uploaded the change for review![]( "Open in Gerrit")
Commit message
[DAS] Fixes occurrences for declaring parameters
Change information
- M pkg/analysis_server/lib/src/computer/computer_document_highlights.dart
- M pkg/analysis_server/lib/src/domains/analysis/occurrences_dart.dart
- M pkg/analysis_server/lib/src/utilities/extensions/element.dart
- M pkg/analysis_server/test/analysis/notification_occurrences_test.dart
- M pkg/analysis_server/test/lsp/document_highlights_test.dart
Related details
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danny Tuppeny (Gerrit)
Danny Tuppeny added 6 comments![]( "Open in Gerrit")
// For pattern variables in implicit pattern fields (where the field name
// is inferred from the variable name), also include the field element.I think this comment is a bit disconnected from the code it relates to now (the existing blank line probably didn't help), it should probably go above `VariablePattern(` on line 85.
return _targetElements.any(
(element) => element.name == canonicalElement.name,
);Is there a reason we can't keep `_elementName` here (even if we capture it in the .elements()` constructor)? This method is called a lot and I'd expect a string comparison is much faster than iterating over even a small list.
_addOccurrence(field, nameToken);Would we not also need to add `element` here (so that if you start from the parameter name in an invocation, it still matches here?)
Element? get canonical => switch (this) {I wonder if this logic is general enough that it's worth extracting from the occurrences code to here? If it might be useful elsewhere, I think it makes sense, but if the rules for canonicalizing are likely to be feature-specific, it might be confusing to have here.
(if the goal was just to share between LSP + legacy, I wouldn't worry too much - legacy occurrences are completely unused and likely to be removed - https://github.com/dart-lang/sdk/issues/60043)
test_primaryConstructor_declaringPrivateParameter_declaration() async {I'm not sure how this test differs from the one above? (the code is slightly different, but it seems like they're testing the same - should they be combined?)
Related details
- Brian Wilkerson
- FMorschel
- Samuel Rawlins
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |