Fix element discovery without analyzer changes #2050
Conversation
jcollins-g
changed the title
| .whereType<ExtensionElement>()); | ||
|
|
||
| _extensions = extensionElements | ||
| _extensions = _exportedAndLocalElements |
There was a problem hiding this comment.
Here and below, consider the shorter form:
return _extensions ??= ...;There was a problem hiding this comment.
Surrounding code style is along the lines of
if (_foo == null) {
do_things_to_create__foo;
}
return _foo;so I would prefer to keep this for now to be consistent
lib/src/model.dart
Outdated
| // Initialize the list of elements defined in this library and | ||
| // exported via its export directives. | ||
| Set<Element> exportedAndLocalElements = NamespaceBuilder() | ||
| .createExportNamespaceForLibrary(element) |
There was a problem hiding this comment.
Would LibraryElement.exportNamespace work here?
There was a problem hiding this comment.
It seems to work. Given that it is part of the public interface I'll switch to that, but I'm surprised that the two seem to use completely different means to generate the same data. Is there an intended difference between the two?
There was a problem hiding this comment.
Well, given that NamespaceBuilder is not a part of public API, while it is definitely not good that we have two implementations, it is better if external users use the one from the public API :-P
| _package._allLibraries.add(this); | ||
| } | ||
|
|
||
| static Iterable<Element> getDefinedElements( |
There was a problem hiding this comment.
I'm adding extensions to LibraryElement.topLevelElements.
https://dart-review.googlesource.com/c/sdk/+/123551
There was a problem hiding this comment.
great. For some reason we don't seem to use/trust LibraryElement.topLevelElements anywhere, but if it gives us the same thing eventually we should switch this whole discovery system over and improve it. Added TODO
|
FWIW, I'm not against landing this, just pointing out that there might be a slightly better ways to do it, in the future sometimes :-) |
|
Yes, actually there are way better ways to do this @scheglov. Transforming this into a single element construction stream, directly calling the ModelElement.from on everything and dispatching into buckets based on the returned, generated type just off the top of my head. But once I start down the refactoring path forever will it dominate my release schedule.... |
Fixes #2047. (And possibly, other similar bugs due to dartdoc's inconsistency internally in discovery handling).
Per discussion with @scheglov and @bwilkerson, an analyzer extension was determined not to be necessary or even all that useful. Therefore, extract the pieces from https://dart-review.googlesource.com/c/sdk/+/122169 that remained useful and drop them directly into dartdoc.
This is a prelude to other changes to support documenting extension methods as part of classes, cross referencing extensions with what they extend, and resolving comment references inside extension methods.