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

[Wildcard Variables] Dartdoc Support #3769

Open
Tracked by #55673
kallentu opened this issue May 10, 2024 · 8 comments · Fixed by #3837
Open
Tracked by #55673

[Wildcard Variables] Dartdoc Support #3769

kallentu opened this issue May 10, 2024 · 8 comments · Fixed by #3837
Assignees
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@kallentu
Copy link
Member

This issue will track the work needed in Dartdoc for the wildcard variables feature.

There's probably no additional work needed for Dartdoc.
However, we should test that Dartdoc isn't broken by changes to how we represent wildcard variables in the analyzer.

@kallentu
Copy link
Member Author

cc @srawlins

This issue tracks if we need anything for Dartdoc. We probably won't, but we can cross that bridge later when we have the analyzer work done :)

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label May 17, 2024
@bwilkerson bwilkerson added the P2 A bug or feature request we're likely to work on label May 22, 2024
@bwilkerson
Copy link
Member

If Dartdoc is still doing its own resolution of identifier references in doc comments, then a reference of the form [_] will need to be resolved differently. From dartdoc's perspective, this feature will only effect parameters, and that might be the only place that will be impacted by the change.

@srawlins
Copy link
Member

From dartdoc's perspective, this feature will only effect parameters, and that might be the only place that will be impacted by the change.

I think also type parameters, import prefixes, and maybe record type positional fields. In particular field formal parameters and super parameters should be tested.

@srawlins
Copy link
Member

srawlins commented Sep 3, 2024

@kallentu I closed this too soon (I had not properly enabled wildcard variables in tests). I have a question:

In dartdoc's comment resolution code, as you know, we use what analyzer gave us via it's own doc-scope resolution stuff. So now, even with the wildcard variables feature enabled, for this code:

import 'dart:async' as _;

/// Text [_].
int x = 0;

I see that [_] has been resolved to be a comment reference with a PrefixElement element.

I'm not sure how the idea of "non-binding" is implemented in the analyzer, but do you think that [_] should not resolve to that PrefixElement? It's not really a dartdoc question, as this would apply to effects in the IDE, like go-to-definition, etc.

@kallentu
Copy link
Member Author

kallentu commented Sep 3, 2024

On first glance, I would assume that /// Text [_]. wouldn't refer to any element in that example. You can't access the _ prefix.

@pq How do go-to-definitions work for import prefixes? Does it link to something?

@srawlins
Copy link
Member

srawlins commented Sep 3, 2024

Oh good point, since a prefix can be shared amongst several imports. It looks like go-to-definition currently jumps to the last import with that prefix.

Maybe rename? Which seems to work even if there are multiple imports with the same prefix.

srawlins added a commit to srawlins/dartdoc that referenced this issue Sep 3, 2024
An unrelated fix, dart-lang#3865, revealed that
the wildcard-variables experiment was not actually enabled in tests; the SDK
constraint used in test packages needs to be '^3.6.0'. With that fix, there are
some questions about how much dartdoc should be doing to keep wildcards
"non-binding" and how much the analyzer should be doing. So that is raised in
dart-lang#3769 (comment).
@pq
Copy link
Member

pq commented Sep 3, 2024

How do go-to-definitions work for import prefixes? Does it link to something?

If a prefix is bound, they work as you'd expect and jump you to where the prefix is declared.

For a wildcard, _ is not bound so it won't jump you anywhere

image

@srawlins
Copy link
Member

srawlins commented Sep 3, 2024

Indeed, if I add this test case to pkg/analysis_server/test/analysis/get_navigation_test.dart:

  Future<void> test_x() async {
    addTestFile('''
import 'dart:async' as _;
/// Returns a [_].
String f() {
}''');
    await waitForTasksFinished();
    await _getNavigation(search: '[_', length: 1);
    expect(regions, hasLength(1));
    assertHasRegion('_]');
  }

it fails expecting one region (there are zero). Good. So then back to the question of [_] having an element or anything, if I add the following test to pkg/analyzer/test/src/dart/resolution/comment_test.dart:

  test_x() async {
    await assertNoErrorsInCode('''
// ignore: unused_import
import 'dart:async' as _;

/// [_]
void f() {}
''');

    assertResolvedNodeText(findNode.commentReference('_]'), r'''
CommentReference
  expression: SimpleIdentifier
    token: _
    staticElement: <null>
    staticType: null
''');
  }

I get a passing result. I'm thinking dartdoc just doesn't have access to the latest analyzer code that puts a null static element here. So I'll leave this ticket open until that is published.

srawlins added a commit that referenced this issue Sep 4, 2024
An unrelated fix, #3865, revealed that
the wildcard-variables experiment was not actually enabled in tests; the SDK
constraint used in test packages needs to be '^3.6.0'. With that fix, there are
some questions about how much dartdoc should be doing to keep wildcards
"non-binding" and how much the analyzer should be doing. So that is raised in
#3769 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants