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

Start using docImport resolution when resolving references #3805

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Jun 29, 2024

Currently dartdoc uses a two-step strategy to resolve doc comment references:

First, for each reference in square brackets, dartdoc asks analyzer's Scope.lookup API for what a reference should resolve to. (Note: that API does not support resolution that includes the "doc import" scope which is introduced by @docImport.) Second, if Scope.lookup could not resolve the reference, dartdoc uses it's own lookup system which searches outward for identifiers (the "universal scope" system).

In this change, we introduce a third step: between the existing two steps, we check the ModelNode.commentData references. These references do take the "doc import" scope into account. So here is the new process:

  1. Check analyzer's Scope.lookup API. If that returns null,
  2. Check analyzer's earlier resolution, which includes the "doc import" scope. If that returns null,
  3. Resolve the reference with the "universal scope" system.

This should not change any existing successful resolutions into different, unintended resolutions. Introducing the second step should only start resolving references that can be resolved specifically because of an introduced @docImport.

The only secondary change here which is needed to facilitate the @docImport change is to declare that extension members are "contained" by their enclosing extension. I don't know why this wasn't the case before. It doesn't seem intentional, as the tests continue to pass. I think it was an oversight, or a misunderstanding of extension members.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@srawlins
Copy link
Member Author

CC @dart-lang/analyzer-team

if (resultElement == null) return null;
}

if (this case ModelElement(:var modelNode?) when resultElement == null) {
Copy link
Contributor

@scheglov scheglov Jun 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever, maybe too much :-P
Match a pattern, and then check a condition that does not use the pattern.

Ideally, should be a separate if enclosing the pattern matching one, but one liner is tempting :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I definitely thought this was a little weird :P Specifically because it seems like the when resultElement == null should be first. But we cannot write if (resultElement == null && this case ModelElement(:var modelNode?)). I can split it up in a follow-up.

@srawlins srawlins merged commit e7f3694 into dart-lang:main Jul 1, 2024
9 checks passed
@srawlins srawlins deleted the doc-imports-fallback branch July 1, 2024 15:09
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 1, 2024
…, convert, crypto, csslib, dartdoc, fixnum, html, http, lints, logging, markdown, matcher, mime, mockito, path, pool, pub_semver, shelf, source_map_stack_trace, sse, stack_trace, stream_channel, string_scanner, term_glyph, test, test_descriptor, test_process, test_reflective_loader, tools, typed_data, vector_math, watcher, web, web_socket_channel, webdriver, webkit_inspection_protocol, yaml, yaml_edit

Revisions updated by `dart tools/rev_sdk_deps.dart`.

async (https://github.com/dart-lang/async/compare/d7c0cd5..c0d81f8):
  c0d81f8  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/async#280)

boolean_selector (https://github.com/dart-lang/boolean_selector/compare/62f82f6..c5468f4):
  c5468f4  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/boolean_selector#63)

browser_launcher (https://github.com/dart-lang/browser_launcher/compare/7348cea..6012690):
  6012690  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/browser_launcher#61)

cli_util (https://github.com/dart-lang/cli_util/compare/c37d5e1..6419270):
  6419270  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/cli_util#105)

clock (https://github.com/dart-lang/clock/compare/7cbf08e..ad428ea):
  ad428ea  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/clock#66)

convert (https://github.com/dart-lang/convert/compare/0c9eab7..9035caf):
  9035caf  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/convert#108)

crypto (https://github.com/dart-lang/crypto/compare/813e35e..1216790):
  1216790  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/crypto#175)

csslib (https://github.com/dart-lang/csslib/compare/b70fef2..192d720):
  192d720  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/csslib#205)

dartdoc (https://github.com/dart-lang/dartdoc/compare/7e5da60..e7f3694):
  e7f36946  2024-07-01  Sam Rawlins  Start using docImport resolution when resolving references (dart-lang/dartdoc#3805)
  9d756dc3  2024-06-30  Sam Rawlins  Bump to 8.0.10 (dart-lang/dartdoc#3806)
  4259b0b1  2024-06-28  Sam Rawlins  Parse and remove doc-imports from comment text (dart-lang/dartdoc#3803)
  31833c34  2024-06-28  Sam Rawlins  Correctly show extension type ctors and hide enum ctors (dart-lang/dartdoc#3804)
  2a39376a  2024-06-27  Sam Rawlins  Add the 8.0.9+1 changelog entry (dart-lang/dartdoc#3801)

fixnum (https://github.com/dart-lang/fixnum/compare/a8157d8..6c19e60):
  6c19e60  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/fixnum#132)
  57b41c2  2024-06-27  Kevin Moore  update lints (dart-archive/fixnum#131)

html (https://github.com/dart-lang/html/compare/f6c2c71..0da420c):
  0da420c  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/html#250)

http (https://github.com/dart-lang/http/compare/bf96551..8d89385):
  8d89385  2024-06-28  Brian Quinlan  Make it more clear to using use runWithClient in the Dart SDK. (dart-lang/http#1250)
  321362a  2024-06-27  Brian Quinlan  Document that runWithClient should not be used with flutter (dart-lang/http#1249)

lints (https://github.com/dart-lang/lints/compare/baaaa56..f6b5d36):
  f6b5d36  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart from 1.6.4 to 1.6.5 (dart-lang/lints#194)
  09a9c88  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.6 to 4.1.7 (dart-lang/lints#193)

logging (https://github.com/dart-lang/logging/compare/240ec33..6c3fb37):
  6c3fb37  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.6 to 4.1.7 (dart-archive/logging#168)

markdown (https://github.com/dart-lang/markdown/compare/3d8d7a8..6242437):
  6242437  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/markdown#617)

matcher (https://github.com/dart-lang/matcher/compare/0abd405..d6d573d):
  d6d573d  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/matcher#250)

mime (https://github.com/dart-lang/mime/compare/fd7010b..11fec7d):
  11fec7d  2024-07-01  Kevin Moore  blast_repo fixes (dart-archive/mime#131)
  ffdcde3  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.6 to 4.1.7 (dart-archive/mime#129)

mockito (https://github.com/dart-lang/mockito/compare/9deddcf..a7fdf71):
  a7fdf71  2024-06-27  Sam Rawlins  Use curly braces in if statement, in accordance with upcoming lint rule change.

path (https://github.com/dart-lang/path/compare/04807b6..e969f42):
  e969f42  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/path#169)

pool (https://github.com/dart-lang/pool/compare/832c5ab..924fb04):
  924fb04  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-lang/pool#90)

pub_semver (https://github.com/dart-lang/pub_semver/compare/dfcad38..d9e5ee6):
  d9e5ee6  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/pub_semver#106)

shelf (https://github.com/dart-lang/shelf/compare/2536c15..9f2dffe):
  9f2dffe  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/shelf#438)

source_map_stack_trace (https://github.com/dart-lang/source_map_stack_trace/compare/96a8213..741b6ce):
  741b6ce  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-archive/source_map_stack_trace#56)

sse (https://github.com/dart-lang/sse/compare/7dcde16..52d042f):
  52d042f  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/sse#112)

stack_trace (https://github.com/dart-lang/stack_trace/compare/ab09060..4fd3e2a):
  4fd3e2a  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/stack_trace#156)

stream_channel (https://github.com/dart-lang/stream_channel/compare/dc620d2..28a6533):
  28a6533  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/stream_channel#109)

string_scanner (https://github.com/dart-lang/string_scanner/compare/e1cab8f..0de03b5):
  0de03b5  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/string_scanner#77)

term_glyph (https://github.com/dart-lang/term_glyph/compare/6c2a977..38a158f):
  38a158f  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/term_glyph#55)

test (https://github.com/dart-lang/test/compare/329c6df..3256c23):
  3256c23c  2024-07-01  dependabot[bot]  Bump the github-actions group with 3 updates (dart-lang/test#2247)
  4da62ac6  2024-06-27  Nate Bosch  Add package config URI into VM test entrypoints (dart-lang/test#2245)

test_descriptor (https://github.com/dart-lang/test_descriptor/compare/2f19400..90743bc):
  90743bc  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/test_descriptor#69)

test_process (https://github.com/dart-lang/test_process/compare/a7ca20b..6223572):
  6223572  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/test_process#61)

test_reflective_loader (https://github.com/dart-lang/test_reflective_loader/compare/816942e..6e64886):
  6e64886  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/test_reflective_loader#64)

tools (https://github.com/dart-lang/tools/compare/4c60686..43a8582):
  43a8582  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/tools#283)

typed_data (https://github.com/dart-lang/typed_data/compare/8529929..365468a):
  365468a  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-archive/typed_data#91)

vector_math (https://github.com/google/vector_math.dart/compare/a4304d1..2cfbe2c):
  2cfbe2c  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (google/vector_math.dart#329)

watcher (https://github.com/dart-lang/watcher/compare/f312f1f..0484625):
  0484625  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/watcher#170)

web (https://github.com/dart-lang/web/compare/2ae509e..e4c4d81):
  e4c4d81  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/web#269)

web_socket_channel (https://github.com/dart-lang/web_socket_channel/compare/bf69990..8e95ea7):
  8e95ea7  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/web_socket_channel#376)

webdriver (https://github.com/google/webdriver.dart/compare/f85779e..718e4c3):
  718e4c3  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.7 (google/webdriver.dart#301)

webkit_inspection_protocol (https://github.com/google/webkit_inspection_protocol.dart/compare/5740cc9..32fffa5):
  32fffa5  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart from 1.6.2 to 1.6.5 (google/webkit_inspection_protocol.dart#126)
  73ab344  2024-07-01  dependabot[bot]  Bump actions/checkout from 4.1.4 to 4.1.7 (google/webkit_inspection_protocol.dart#125)

yaml (https://github.com/dart-lang/yaml/compare/4cf24ca..30fd9e0):
  30fd9e0  2024-07-01  dependabot[bot]  Bump dart-lang/setup-dart in the github-actions group (dart-lang/yaml#168)

yaml_edit (https://github.com/dart-lang/yaml_edit/compare/ad3292c..57a28da):
  57a28da  2024-07-01  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/yaml_edit#92)

Change-Id: Ifaff2db977be0b38b631e8a177bbff47c3d24c12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/373880
Reviewed-by: Kevin Moore <kevmoo@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants