Skip to content

Import prefix support for dartdoc #1875

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 11 commits into from
Dec 20, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
145 changes: 88 additions & 57 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ final RegExp trailingIgnoreStuff = new RegExp(r'(<.*>|\(.*\))$');
final RegExp leadingIgnoreStuff =
new RegExp(r'^(const|final|var)[\s]+', multiLine: true);

// This is explicitly intended as a reference to a constructor.
final RegExp isConstructor = new RegExp(r'^new[\s]+', multiLine: true);
// If found, this may be intended as a reference to a constructor.
final RegExp isConstructor = new RegExp(r'(^new[\s]+|\(\)$)', multiLine: true);

// This is probably not really intended as a doc reference, so don't try or
// warn about them.
Expand Down Expand Up @@ -299,12 +299,45 @@ class _MarkdownCommentReference {
assert(element != null);
assert(element.packageGraph.allLibrariesAdded);

codeRefChomped = codeRef.replaceFirst(isConstructor, '');
codeRefChomped = codeRef.replaceAll(isConstructor, '');
library =
element is ModelElement ? (element as ModelElement).library : null;
packageGraph = library.packageGraph;
}

String __impliedDefaultConstructor;
bool __impliedDefaultConstructorIsSet = false;

/// Returns the name of the implied default constructor if there is one, or
/// null if not.
///
/// Default constructors are a special case in dartdoc. If we look up a name
/// within a class of that class itself, the first thing we find is the
/// default constructor. But we determine whether that's what they actually
/// intended (vs. the enclosing class) by context -- whether they seem
/// to be calling it with () or have a 'new' in front of it, or
/// whether the name is repeated.
///
/// Similarly, referencing a class by itself might actually refer to its
/// constructor based on these same heuristics.
///
/// With the name of the implied default constructor, other methods can
/// determine whether or not the constructor and/or class we resolved to
/// is actually matching the user's intent.
String get _impliedDefaultConstructor {
if (!__impliedDefaultConstructorIsSet) {
__impliedDefaultConstructorIsSet = true;
if (codeRef.contains(isConstructor) ||
(codeRefChompedParts.length >= 2 &&
codeRefChompedParts[codeRefChompedParts.length - 1] ==
codeRefChompedParts[codeRefChompedParts.length - 2])) {
// If the last two parts of the code reference are equal, this is probably a default constructor.
__impliedDefaultConstructor = codeRefChompedParts.last;
}
}
return __impliedDefaultConstructor;
}

/// Calculate reference to a ModelElement.
///
/// Uses a series of calls to the _find* methods in this class to get one
Expand All @@ -318,8 +351,6 @@ class _MarkdownCommentReference {
for (void Function() findMethod in [
// This might be an operator. Strip the operator prefix and try again.
_findWithoutOperatorPrefix,
// Oh, and someone might have some type parameters or other garbage.
_findWithoutTrailingIgnoreStuff,
// Oh, and someone might have thrown on a 'const' or 'final' in front.
_findWithoutLeadingIgnoreStuff,
// Maybe this ModelElement has parameters, and this is one of them.
Expand All @@ -330,6 +361,8 @@ class _MarkdownCommentReference {
_findTypeParameters,
// This could be local to the class, look there first.
_findWithinTryClasses,
// This could be a reference to a renamed library.
_findReferenceFromPrefixes,
// We now need the ref element cache to keep from repeatedly searching [Package.allModelElements].
// But if not, look for a fully qualified match. (That only makes sense
// if the codeRef might be qualified, and contains periods.)
Expand All @@ -340,8 +373,12 @@ class _MarkdownCommentReference {
_findGlobalWithinRefElementCache,
// This could conceivably be a reference to an enum member. They don't show up in allModelElements.
_findEnumReferences,
// Oh, and someone might have some type parameters or other garbage.
// After finding within classes because sometimes parentheses are used
// to imply constructors.
_findWithoutTrailingIgnoreStuff,
// Use the analyzer to resolve a comment reference.
_findAnalyzerReferences
_findAnalyzerReferences,
]) {
findMethod();
// Remove any "null" objects after each step of trying to add to results.
Expand All @@ -355,8 +392,6 @@ class _MarkdownCommentReference {
// This isn't C++. References to class methods are slightly expensive
// in Dart so don't build that list unless you need to.
for (void Function() reduceMethod in [
// If this name could refer to a class or a constructor, prefer the class.
_reducePreferClass,
// If a result is actually in this library, prefer that.
_reducePreferResultsInSameLibrary,
// If a result is accessible in this library, prefer that.
Expand Down Expand Up @@ -404,30 +439,6 @@ class _MarkdownCommentReference {
List<String> get codeRefChompedParts =>
_codeRefChompedParts ??= codeRefChomped.split('.');

/// Returns true if this is a constructor we should consider due to its
/// name and the code reference, or if this isn't a constructor. False
/// otherwise.
bool _ConsiderIfConstructor(ModelElement modelElement) {
// TODO(jcollins-g): Rewrite this to handle constructors in a less hacky way
if (modelElement is! Constructor) return true;
if (codeRef.contains(isConstructor)) return true;
Constructor aConstructor = modelElement;
if (codeRefParts.length > 1) {
// Pick the last two parts, in case a specific library was part of the
// codeRef.
if (codeRefParts[codeRefParts.length - 1] ==
codeRefParts[codeRefParts.length - 2]) {
// Foobar.Foobar -- assume they really do mean the constructor for this class.
return true;
}
}
if (aConstructor.name != aConstructor.enclosingElement.name) {
// This isn't a default constructor so treat it like any other member.
return true;
}
return false;
}

void _reducePreferAnalyzerResolution() {
Element refElement = _getRefElementFromCommentRefs(commentRefs, codeRef);
if (results.any((me) => me.element == refElement)) {
Expand Down Expand Up @@ -471,12 +482,6 @@ class _MarkdownCommentReference {
}
}

void _reducePreferClass() {
if (results.any((r) => r is Class)) {
results.removeWhere((r) => r is Constructor);
}
}

void _findTypeParameters() {
if (element is TypeParameters) {
results.addAll((element as TypeParameters).typeParameters.where((p) =>
Expand Down Expand Up @@ -540,14 +545,51 @@ class _MarkdownCommentReference {
}
}

/// Transform members of [toConvert] that are classes to their default constructor,
/// if a constructor is implied. If not, do the reverse conversion for default
/// constructors.
ModelElement _convertConstructors(ModelElement toConvert) {
if (_impliedDefaultConstructor != null) {
if (toConvert is Class && toConvert.name == _impliedDefaultConstructor) {
return toConvert.defaultConstructor;
}
return toConvert;
} else {
if (toConvert is Constructor &&
(toConvert.enclosingElement as Class).defaultConstructor ==
toConvert) {
return toConvert.enclosingElement;
}
return toConvert;
}
}

void _findReferenceFromPrefixes() {
if (element is! ModelElement) return;
Map<String, Set<Library>> prefixToLibrary =
(element as ModelElement).definingLibrary.prefixToLibrary;
if (prefixToLibrary.containsKey(codeRefChompedParts.first)) {
if (codeRefChompedParts.length == 1) {
results.addAll(prefixToLibrary[codeRefChompedParts.first]);
} else {
String lookup = codeRefChompedParts.sublist(1).join('.');
prefixToLibrary[codeRefChompedParts.first]?.forEach((l) => l
.modelElementsNameMap[lookup]
?.map(_convertConstructors)
?.forEach((m) => _addCanonicalResult(m, _getPreferredClass(m))));
}
}
}

void _findGlobalWithinRefElementCache() {
if (packageGraph.findRefElementCache.containsKey(codeRefChomped)) {
for (final modelElement
in packageGraph.findRefElementCache[codeRefChomped]) {
if (codeRefChomped == modelElement.fullyQualifiedNameWithoutLibrary ||
(modelElement is Library &&
codeRefChomped == modelElement.fullyQualifiedName)) {
_addCanonicalResult(modelElement, null);
_addCanonicalResult(
_convertConstructors(modelElement), preferredClass);
}
}
}
Expand All @@ -557,8 +599,7 @@ class _MarkdownCommentReference {
// Only look for partially qualified matches if we didn't find a fully qualified one.
if (library.modelElementsNameMap.containsKey(codeRefChomped)) {
for (final modelElement in library.modelElementsNameMap[codeRefChomped]) {
if (!_ConsiderIfConstructor(modelElement)) continue;
_addCanonicalResult(modelElement, preferredClass);
_addCanonicalResult(_convertConstructors(modelElement), preferredClass);
}
}
}
Expand All @@ -571,13 +612,12 @@ class _MarkdownCommentReference {
packageGraph.findRefElementCache.containsKey(codeRefChomped)) {
for (final ModelElement modelElement
in packageGraph.findRefElementCache[codeRefChomped]) {
if (!_ConsiderIfConstructor(modelElement)) continue;
// For fully qualified matches, the original preferredClass passed
// might make no sense. Instead, use the enclosing class from the
// element in [packageGraph.findRefElementCache], because that element's
// enclosing class will be preferred from [codeRefChomped]'s perspective.
_addCanonicalResult(
modelElement,
_convertConstructors(modelElement),
modelElement.enclosingElement is Class
? modelElement.enclosingElement
: null);
Expand Down Expand Up @@ -678,26 +718,17 @@ class _MarkdownCommentReference {
/// Get any possible results for this class in the superChain. Returns
/// true if we found something.
void _getResultsForSuperChainElement(Class c, Class tryClass) {
Iterable<ModelElement> membersToCheck;
membersToCheck = (c.allModelElementsByNamePart[codeRefChomped] ?? [])
.where((m) => _ConsiderIfConstructor(m));
Iterable<ModelElement> membersToCheck =
(c.allModelElementsByNamePart[codeRefChomped] ?? [])
.map(_convertConstructors);
for (final ModelElement modelElement in membersToCheck) {
// [thing], a member of this class
_addCanonicalResult(modelElement, tryClass);
}
membersToCheck = (c.allModelElementsByNamePart[codeRefChompedParts.last] ??
<ModelElement>[])
.where((m) => _ConsiderIfConstructor(m));
if (codeRefChompedParts.first == c.name) {
// [Foo...thing], a member of this class (possibly a parameter).
membersToCheck.forEach((m) => _addCanonicalResult(m, tryClass));
} else if (codeRefChompedParts.length > 1 &&
codeRefChompedParts[codeRefChompedParts.length - 2] == c.name) {
// [....Foo.thing], a member of this class partially specified.
membersToCheck
.whereType<Constructor>()
.forEach((m) => _addCanonicalResult(m, tryClass));
}
.map(_convertConstructors);
membersToCheck.forEach((m) => _addCanonicalResult(m, tryClass));
results.remove(null);
if (results.isNotEmpty) return;
if (c.fullyQualifiedNameWithoutLibrary == codeRefChomped) {
Expand Down
34 changes: 33 additions & 1 deletion lib/src/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,15 @@ class Class extends ModelElement
.toList(growable: false);
}

Constructor _defaultConstructor;
Constructor get defaultConstructor {
if (_defaultConstructor == null) {
_defaultConstructor = constructors
.firstWhere((c) => c.isDefaultConstructor, orElse: () => null);
}
return _defaultConstructor;
}

Iterable<Method> get allInstanceMethods =>
quiverIterables.concat([instanceMethods, inheritedMethods]);

Expand Down Expand Up @@ -1227,7 +1236,10 @@ class Constructor extends ModelElement
}

@override
String get fullyQualifiedName => '${library.name}.$name';
String get fullyQualifiedName {
if (isDefaultConstructor) return super.fullyQualifiedName;
return '${library.name}.$name';
}

@override
String get href {
Expand All @@ -1241,6 +1253,8 @@ class Constructor extends ModelElement
@override
bool get isConst => _constructor.isConst;

bool get isDefaultConstructor => name == enclosingElement.name;

bool get isFactory => _constructor.isFactory;

@override
Expand Down Expand Up @@ -2245,6 +2259,24 @@ class Library extends ModelElement with Categorization, TopLevelContainer {
return _importedExportedLibraries;
}

Map<String, Set<Library>> _prefixToLibrary;

/// Map of import prefixes ('import "foo" as prefix;') to [Library].
Map<String, Set<Library>> get prefixToLibrary {
if (_prefixToLibrary == null) {
_prefixToLibrary = {};
// It is possible to have overlapping prefixes.
for (ImportElement i in (element as LibraryElement).imports) {
if (i.prefix?.name != null) {
_prefixToLibrary.putIfAbsent(i.prefix?.name, () => new Set());
_prefixToLibrary[i.prefix?.name].add(
new ModelElement.from(i.importedLibrary, library, packageGraph));
}
}
}
return _prefixToLibrary;
}

String _dirName;
String get dirName {
if (_dirName == null) {
Expand Down
5 changes: 3 additions & 2 deletions test/dartdoc_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ void main() {
Package p = packageGraph.defaultPackage;
expect(p.name, 'test_package');
expect(p.hasDocumentationFile, isTrue);
expect(packageGraph.defaultPackage.publicLibraries, hasLength(10));
// Total number of public libraries in test_package.
expect(packageGraph.defaultPackage.publicLibraries, hasLength(12));
expect(packageGraph.localPackages.length, equals(1));
});

Expand Down Expand Up @@ -307,7 +308,7 @@ void main() {
PackageGraph p = results.packageGraph;
expect(p.defaultPackage.name, 'test_package');
expect(p.defaultPackage.hasDocumentationFile, isTrue);
expect(p.localPublicLibraries, hasLength(9));
expect(p.localPublicLibraries, hasLength(11));
expect(p.localPublicLibraries.map((lib) => lib.name).contains('fake'),
isFalse);
});
Expand Down
Loading