diff --git a/lib/src/model.dart b/lib/src/model.dart index 0e5cc90bee..8e03022b13 100644 --- a/lib/src/model.dart +++ b/lib/src/model.dart @@ -1929,7 +1929,7 @@ abstract class GetterSetterCombo implements ModelElement { _documentationFrom.addAll(setter.documentationFrom); } if (_documentationFrom.length == 0 || - _documentationFrom.every((e) => e.documentation == '')) + _documentationFrom.every((e) => e.documentationComment == '')) _documentationFrom = computeDocumentationFrom; } return _documentationFrom; @@ -4697,16 +4697,26 @@ class PackageGraph { /// Generate a list of futures for any docs that actually require precaching. Iterable precacheLocalDocs() sync* { + // Prevent reentrancy. + Set precachedElements = new Set(); + + Iterable precacheOneElement(ModelElement m) sync* { + for (ModelElement d in m.documentationFrom.where((d) => d.documentationComment != null)) { + if (needsPrecacheRegExp.hasMatch(d.documentationComment) + && !precachedElements.contains(d)) { + precachedElements.add(d); + yield d._precacheLocalDocs(); + } + } + } + for (ModelElement m in allModelElements) { // Skip if there is a canonicalModelElement somewhere else we can run this // for. Not the same as allCanonicalModelElements since we need to run // for any ModelElement that might not have a canonical ModelElement, // too. if (m.canonicalModelElement != null && !m.isCanonical) continue; - if (m.documentationComment != null && - needsPrecacheRegExp.hasMatch(m.documentationComment)) { - yield m._precacheLocalDocs(); - } + yield* precacheOneElement(m); } } diff --git a/test/model_test.dart b/test/model_test.dart index 898203985c..f35c08a024 100644 --- a/test/model_test.dart +++ b/test/model_test.dart @@ -121,11 +121,14 @@ void main() { group('Tools', () { Class toolUser; Class _NonCanonicalToolUser, CanonicalToolUser, PrivateLibraryToolUser; + Class ImplementingClassForTool, CanonicalPrivateInheritedToolUser; Method invokeTool; Method invokeToolNoInput; Method invokeToolMultipleSections; Method invokeToolNonCanonical, invokeToolNonCanonicalSubclass; Method invokeToolPrivateLibrary, invokeToolPrivateLibraryOriginal; + Method invokeToolParentDoc, invokeToolParentDocOriginal; + final RegExp packageInvocationIndexRegexp = new RegExp(r'PACKAGE_INVOCATION_INDEX: (\d+)'); setUpAll(() { _NonCanonicalToolUser = fakeLibrary.allClasses @@ -134,6 +137,10 @@ void main() { .firstWhere((c) => c.name == 'CanonicalToolUser'); PrivateLibraryToolUser = fakeLibrary.allClasses .firstWhere((c) => c.name == 'PrivateLibraryToolUser'); + ImplementingClassForTool = fakeLibrary.allClasses + .firstWhere((c) => c.name == 'ImplementingClassForTool'); + CanonicalPrivateInheritedToolUser = fakeLibrary.allClasses + .firstWhere((c) => c.name == 'CanonicalPrivateInheritedToolUser'); toolUser = exLibrary.classes.firstWhere((c) => c.name == 'ToolUser'); invokeTool = toolUser.allInstanceMethods.firstWhere((m) => m.name == 'invokeTool'); @@ -151,24 +158,48 @@ void main() { (invokeToolPrivateLibrary.definingEnclosingElement as Class) .allInstanceMethods .firstWhere((m) => m.name == 'invokeToolPrivateLibrary'); + invokeToolParentDoc = CanonicalPrivateInheritedToolUser.allInstanceMethods + .firstWhere((m) => m.name == 'invokeToolParentDoc'); + invokeToolParentDocOriginal = ImplementingClassForTool.allInstanceMethods + .firstWhere((m) => m.name == 'invokeToolParentDoc'); packageGraph.allLocalModelElements.forEach((m) => m.documentation); }); - test('does _not_ invoke a tool multiple times unnecessarily', () { - RegExp packageInvocationIndexRegexp = - new RegExp(r'PACKAGE_INVOCATION_INDEX: (\d+)'); - expect(invokeToolNonCanonical.isCanonical, isFalse); - expect(invokeToolNonCanonicalSubclass.isCanonical, isTrue); - expect( - packageInvocationIndexRegexp - .firstMatch(invokeToolNonCanonical.documentation) - .group(1), - equals(packageInvocationIndexRegexp - .firstMatch(invokeToolNonCanonicalSubclass.documentation) - .group(1))); - expect(invokeToolPrivateLibrary.documentation, isNot(contains('{@tool'))); - expect( - invokeToolPrivateLibraryOriginal.documentation, contains('{@tool')); + test('invokes tool when inherited documentation is the only means for it to be seen', () { + // Verify setup of the test is correct. + expect(invokeToolParentDoc.isCanonical, isTrue); + expect(invokeToolParentDoc.documentationComment, isNull); + // Error message here might look strange due to toString() on Methods, but if this + // fails that means we don't have the correct invokeToolParentDoc instance. + expect(invokeToolParentDoc.documentationFrom, contains(invokeToolParentDocOriginal)); + // Tool should be substituted out here. + expect(invokeToolParentDoc.documentation, isNot(contains('{@tool'))); + }); + + group('does _not_ invoke a tool multiple times unnecessarily', () { + test('non-canonical subclass case', () { + expect(invokeToolNonCanonical.isCanonical, isFalse); + expect(invokeToolNonCanonicalSubclass.isCanonical, isTrue); + expect( + packageInvocationIndexRegexp + .firstMatch(invokeToolNonCanonical.documentation) + .group(1), + equals(packageInvocationIndexRegexp + .firstMatch(invokeToolNonCanonicalSubclass.documentation) + .group(1))); + expect(invokeToolPrivateLibrary.documentation, isNot(contains('{@tool'))); + expect( + invokeToolPrivateLibraryOriginal.documentation, contains('{@tool')); + }); + + test('Documentation borrowed from implementer case', () { + expect(packageInvocationIndexRegexp + .firstMatch(invokeToolParentDoc.documentation) + .group(1), + equals(packageInvocationIndexRegexp + .firstMatch(invokeToolParentDocOriginal.documentation) + .group(1))); + }); }); test('can invoke a tool and pass args and environment', () { diff --git a/testing/test_package/lib/fake.dart b/testing/test_package/lib/fake.dart index 4af1f2ad87..248f7d9895 100644 --- a/testing/test_package/lib/fake.dart +++ b/testing/test_package/lib/fake.dart @@ -1021,3 +1021,20 @@ abstract class _NonCanonicalToolUser { } abstract class CanonicalToolUser extends _NonCanonicalToolUser {} + +abstract class ImplementingClassForTool { + /// Invokes a tool from inherited documentation via `implemented` + /// + /// {@tool drill} + /// This is some drill text right here. + /// {@end-tool} + void invokeToolParentDoc(); +} + +/// The method [invokeToolParentDoc] gets its documentation from an interface class. +abstract class CanonicalPrivateInheritedToolUser implements ImplementingClassForTool { + @override + void invokeToolParentDoc() { + print('hello, tool world'); + } +}