Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dartdoc bracketed references don't work for inherited members #4114

Closed
gspencergoog opened this issue Aug 22, 2022 · 4 comments
Closed

Dartdoc bracketed references don't work for inherited members #4114

gspencergoog opened this issue Aug 22, 2022 · 4 comments
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Milestone

Comments

@gspencergoog
Copy link

Description

Bracketed references in dartdoc comments don't get highlighted if the field referred to is a superclass parameter.

For instance, if I have:

/// The callback [TextButton.onPressed] will be called when the button is pressed.

Then the bracketed reference to TextButton.onPressed is not clickable, but:

/// The callback [ButtonStyleButton.onPressed] will be called when the button is pressed.

is clickable. ButtonStyleButton is the superclass for TextButton.

I think this is because the TextButton class inherits its attribute from the parent via the super.onPressed argument in the constructor, but I can't be sure.

Version info

  • Operating System and version:
    • Debian GNU/Linux rodete 5.17.11-1rodete2-amd64
  • VS Code version:
    • Version: 1.70.2
    • Commit: e4503b30fc78200f846c62cf8091b76ff5547662
    • Date: 2022-08-16T05:36:25.715Z
    • Electron: 18.3.5
    • Chromium: 100.0.4896.160
    • Node.js: 16.13.2
    • V8: 10.0.139.17-electron.0
    • OS: Linux x64 5.17.11-1rodete2-amd64
  • Dart extension version: v3.46.1
  • Flutter extension version: v3.46.0
  • Dart/Flutter SDK version:
    • [!] Flutter (Channel menu_bar_iv, 3.1.0-0.0.pre.2427, on Debian GNU/Linux rodete 5.17.11-1rodete2-amd64, locale en_US.UTF-8)
    • Flutter version 3.1.0-0.0.pre.2427 on channel menu_bar_iv at /usr/local/google/home/gspencer/code/flutter
    • ! Upstream repository git@github.com:gspencergoog/flutter.git is not a standard remote.
    • Set environment variable "FLUTTER_GIT_URL" to git@github.com:gspencergoog/flutter.git to dismiss this error.
    • Framework revision 0dab4324ab (2 hours ago), 2022-08-22 13:21:00 -0700
    • Engine revision 0d182ddb0d
    • Dart version 2.19.0 (build 2.19.0-122.0.dev)
    • DevTools version 2.16.0
@DanTup
Copy link
Member

DanTup commented Aug 23, 2022

I don't think it's specific to super-params, as this has the same behaviour:

class A {
  final String myField = '';
}

class B extends A {
}

/// [A.myField] is clickable.
/// [B.myField] is not clickable.
int? a;

@bwilkerson I think this doesn't currently work because in B.myField, myField does not have a staticElement (whereas in A.myField it does). I suspect that's because the comment resolver here only looks in the current class for members:

https://github.com/dart-lang/sdk/blob/571f43b004cc32f50418b037af475c6565648c21/pkg/analyzer/lib/src/dart/resolver/comment_reference_resolver.dart#L71-L75

Should this reference work? And if so, would the correct fix be to walk up prefixElement.allSupertypes to find the first match by name to assign its staticElement?

@DanTup DanTup added in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server labels Aug 23, 2022
@DanTup DanTup added this to the On Deck milestone Aug 23, 2022
@DanTup DanTup changed the title Dartdoc bracketed references don't understand super parameters. Dartdoc bracketed references don't work for inherited members Aug 23, 2022
@bwilkerson
Copy link

Should this reference work?

I don't know what dart doc does with that code, but the analyzer should match it as closely as possible, given the performance constraints it has to operate under.

And if so, would the correct fix be to walk up prefixElement.allSupertypes to find the first match by name to assign its staticElement?

I think we want to walk up the superclass chain first and only look in interfaces when we can't find a member from a supertype. I believe that that's the behavior we use in other features, and it would be good to be consistent. I also think that that's the behavior we'd get from using InheritanceManager3, which is probably the best way to implement such a lookup.

@DanTup
Copy link
Member

DanTup commented Aug 23, 2022

I don't know what dart doc does with that code, but the analyzer should match it as closely as possible, given the performance constraints it has to operate under.

I tested the example above, and it does appear to work:

Screenshot 2022-08-23 at 17 04 43

I also think that that's the behavior we'd get from using InheritanceManager3, which is probably the best way to implement such a lookup.

I'll take a look, thanks!

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Aug 25, 2022
Fixes Dart-Code/Dart-Code#4114.

Change-Id: I47fe2909e4d09366c1e2f432c31076ba690fac7e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256162
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@DanTup DanTup modified the milestones: On Deck, v3.50.0 Aug 25, 2022
@DanTup
Copy link
Member

DanTup commented Aug 25, 2022

Fixed by dart-lang/sdk@857a0d2 (ships in the SDK, not a Dart extension update).

@DanTup DanTup closed this as completed Aug 25, 2022
@DanTup DanTup added the relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available label Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in editor Relates to code editing or language features in lsp/analysis server Something to be fixed in the Dart analysis server is bug relies on sdk changes Something that requires changes in the Dart/Flutter SDK to ship before it will become available
Projects
None yet
Development

No branches or pull requests

3 participants