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.

Sign up for GitHub

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

Jump to bottom

[doc_imports] [this] won't resolve to the current element in new analyzer comment resolver #3761

Closed
kallentu opened this issue Apr 29, 2024 · 3 comments · Fixed by #3765
Closed

[doc_imports] [this] won't resolve to the current element in new analyzer comment resolver #3761

kallentu opened this issue Apr 29, 2024 · 3 comments · Fixed by #3765
Assignees
@srawlins
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

Background

Example of a [this] reference:

Resolution today

Dartdoc adds this as a reference child to look through. When it looks for a reference for the current ModelElement, it will find itself and create a link to that element.

Resolution tomorrow

AFAIK the comment reference resolver in the analyzer doesn’t handle a reference named this.

We need to make sure that we add this into the ModelNode data that we are building up in Dartdoc, or alter the analyzer reference resolver in some way (not sure about this one).

But make sure we handle this, otherwise there will be a regression once we move to solely use what the analyzer provides.
@kallentu
Copy link
Member Author

Related linter issue: dart-lang/sdk#58155

@srawlins
Copy link
Member

We think this is acceptable. I can remove support for [this] in dartdoc, so that the change is incremental, and we can analyze the ongoing gap between the current universal reference support, and analyzer resolution.

@kallentu
Copy link
Member Author

That sounds good. There's other ways to reference the current element anyways.

@srawlins srawlins self-assigned this Apr 30, 2024
srawlins added a commit to srawlins/dartdoc that referenced this issue Apr 30, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
9f465cf 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
@srawlins srawlins mentioned this issue Apr 30, 2024
Merged
1 task
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 3, 2024
@srawlins
front_end: replace unsupported [this] comment references with this …
c4d00e2 
…[Type].

Work towards dart-lang/dartdoc#3761

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

Change-Id: I49478bb39f135d87acff4047767743862c6b8551
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365305
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 3, 2024
@srawlins
SDK: replace unsupported [this] comment references with this.
6a25f12 
Work towards dart-lang/dartdoc#3761

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

CoreLibraryReviewExempt: Comments only.
Change-Id: I6cc85115186527820bdd38dcfda8f61e35ae3fe9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365204
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
srawlins added a commit to srawlins/dartdoc that referenced this issue May 3, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
71ac075 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
srawlins added a commit to srawlins/dartdoc that referenced this issue May 14, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
4eada5b 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 16, 2024
@srawlins
observatory: replace unsupported [this] comment references with `th…
8b53d26 
…is`.

Work towards dart-lang/dartdoc#3761

Sibling CL to https://dart-review.googlesource.com/c/sdk/+/365204

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

TEST=Nope
Change-Id: Ie584a8338d4b203c4dce737769dbf2bd9094a42c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366881
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 16, 2024
@srawlins
js_interop: replace unsupported [this] comment references with this.
020a59e 
Work towards dart-lang/dartdoc#3761

Sibling CL to https://dart-review.googlesource.com/c/sdk/+/365204

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

CoreLibraryReviewExempt: Comments only.
Cq-Include-Trybots: luci.dart.try:flutter-analyze-try,analyzer-win-release-try,pkg-win-release-try
Change-Id: I38dd476e40e4508fb6bf88c28415ff261bae5f43
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366880
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Auto-Submit: Samuel Rawlins <srawlins@google.com>
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 16, 2024
@srawlins
compiler: replace unsupported [this] comment references with this.
2e53ead 
Work towards dart-lang/dartdoc#3761

Sibling CL to https://dart-review.googlesource.com/c/sdk/+/365204

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

Change-Id: I0a30a34c9f58bd6c98c6b83e607a973c9ba51ed7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366900
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label May 17, 2024
srawlins added a commit to srawlins/dartdoc that referenced this issue May 17, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
8cd25bd 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
@bwilkerson bwilkerson added the P2 A bug or feature request we're likely to work on label May 22, 2024
srawlins added a commit to srawlins/dartdoc that referenced this issue May 29, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
d2a7bee 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Jun 5, 2024
@srawlins
macros: replace unsupported [this] comment references with this.
82a3006 
Work towards dart-lang/dartdoc#3761

Sibling CL to https://dart-review.googlesource.com/c/sdk/+/365204

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

Change-Id: I872c215a574dc3d04f0708387408d22fdfd14c14
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366882
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
srawlins added a commit to srawlins/dartdoc that referenced this issue Jun 7, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
acd0f8f 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
@srawlins srawlins mentioned this issue Jun 17, 2024
Merged
9 tasks
srawlins added a commit to flutter/flutter that referenced this issue Jun 17, 2024
@srawlins
Fix doc comment references to 'this' (#150379)
cf375f0 
Work towards dart-lang/dartdoc#3761

Sibling CL to https://dart-review.googlesource.com/c/sdk/+/365204

The analyzer has never recognized `[this]` as a valid doc comment
reference (and the `comment_references` lint rule has similarly
reported such reference attempts). dartdoc has its own algorithms
for resolving comment references, which we are dismantling in favor
of a single resolution, provided by the analyzer.

We've also decided against adding support in the analyzer (see
https://github.com/dart-lang/linter/issues/2079), so these
reference attempts should be re-written.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
srawlins added a commit to srawlins/dartdoc that referenced this issue Jun 18, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
ce78520 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
srawlins added a commit to srawlins/dartdoc that referenced this issue Jun 18, 2024
@srawlins
Stop supporting '[this]' as a comment reference.
f34551d 
The analyzer does not support it and does not intend to, so this change aligns
with that effort. It makes analyzing the gap between analyzer and dartdoc
easier as well.

Fixes dart-lang#3761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srawlins srawlins

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
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stop supporting '[this]' as a comment reference. srawlins/dartdoc
3 participants
@srawlins @bwilkerson @kallentu