Skip to content

Conversation

srawlins
Copy link
Member

When we first resolve a library, we have full access to both the LibraryElement and the CompilationUnit (AST nodes) objects. We should use those pronto, rather than painstakingly recalculate them with NodeLocator2.

This is a pretty big ball of wax. But this is one step in untangling it. Almost everything we need to know about source elements in Dartdoc is on the element model, with two exceptions:

  • Element documentationComment fields only contain the documentation comment text; it's a String. To get knowledge about comment references, we have to go back to the syntax tree.
  • In order to extract the literal source code, to write in the "implementation" section of docs, we must use offsets only found in the syntax tree.

To hang onto some of this syntax tree data, without hanging onto any AST nodes, we do some pre-processing into ModelNode objects. This pre-processing is led by Library.fromLibraryResult. It used to use an ElementVisitor named _HashableChildLibraryElementVisitor. This is simpler now, and called _ModelNodeGatherer.


  • 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.

_ModelNodeGatherer(this.resolvedLibrary, this.packageGraph);

@override
void visitCompilationUnit(CompilationUnit node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use visitor, if we just iterate manually?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well said! OK I've dropped the visitor, and made this a method on PackageGraph. Much simpler.

@srawlins srawlins merged commit 50e4b67 into dart-lang:main Oct 19, 2023
@srawlins srawlins deleted the no-node-locator branch October 19, 2023 18:34
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