Skip to content

Frame for merging scoped lookups #2661

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

Merged
merged 3 commits into from
May 27, 2021

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented May 27, 2021

Making the maximum use of the analyzer's code for looking up symbols will require using the analyzer's Scope and lookup code. Since we don't preserve the AST, construct them ourselves on demand and prioritize the analyzer's lookup, when available, with dartdoc's customizations isolated and added on.

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label May 27, 2021
@jcollins-g jcollins-g requested a review from srawlins May 27, 2021 20:11
recurseChildrenAndFilter(
referenceLookup, referenceChildren[referenceLookup.lookup], filter);

/// Assuming we found a [result], recurse through children, skipping over
Copy link
Member

Choose a reason for hiding this comment

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

This comment seems to presuppose some other action. Would it make more sense for this to be private, and refer to [lookupViaScope]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since implementations from other files will probably need to call it (since any of this working will depend on at least Library overriding lookupViaScope to deal with PrefixElements), making it private will not work. Made that more explicit in the comment.

bool Function(CommentReferable) filter) {
assert(result != null);
if (referenceLookup.remaining.isNotEmpty) {
result = result?.referenceBy(referenceLookup.remaining,
Copy link
Member

Choose a reason for hiding this comment

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

The ?. is not necessary with the above assert, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, fixed

@coveralls
Copy link

coveralls commented May 27, 2021

Coverage Status

Coverage decreased (-0.07%) to 58.316% when pulling 6be1ea0 on jcollins-g:lookup-scoped-frame into 8ec6844 on dart-lang:master.

@jcollins-g jcollins-g merged commit 0922660 into dart-lang:master May 27, 2021
@jcollins-g jcollins-g deleted the lookup-scoped-frame branch May 27, 2021 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants