Cache element docs (and add to completions) [#23694].
This does away with the expensive call to `computeDocumentationComment` in favor of cached comments. Notably this makes adding doc content to code completion proposals performant (and so is done here). It should also make `dartdoc` *much* faster for doc generation since there are no more trips to disk to fetch comments for elements (still needed for source though).
For more on the desire for docs in completions see here: https://github.com/dart-lang/sdk/issues/23694
R=brianwilkerson@google.com, scheglov@google.com
Review URL: https://codereview.chromium.org/1534043002 .
diff --git a/pkg/analysis_server/lib/src/computer/computer_hover.dart b/pkg/analysis_server/lib/src/computer/computer_hover.dart
index 1238489..45467ca 100644
--- a/pkg/analysis_server/lib/src/computer/computer_hover.dart
+++ b/pkg/analysis_server/lib/src/computer/computer_hover.dart
@@ -6,52 +6,10 @@
import 'package:analysis_server/plugin/protocol/protocol.dart'
show HoverInformation;
+import 'package:analysis_server/src/utilities/documentation.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/generated/ast.dart';
-
-/**
- * Converts [str] from a Dart Doc string with slashes and stars to a plain text
- * representation of the comment.
- */
-String _removeDartDocDelimiters(String str) {
- if (str == null) {
- return null;
- }
- // remove /** */
- if (str.startsWith('/**')) {
- str = str.substring(3);
- }
- if (str.endsWith("*/")) {
- str = str.substring(0, str.length - 2);
- }
- str = str.trim();
- // remove leading '* '
- List<String> lines = str.split('\n');
- StringBuffer sb = new StringBuffer();
- bool firstLine = true;
- for (String line in lines) {
- line = line.trim();
- if (line.startsWith("*")) {
- line = line.substring(1);
- if (line.startsWith(" ")) {
- line = line.substring(1);
- }
- } else if (line.startsWith("///")) {
- line = line.substring(3);
- if (line.startsWith(" ")) {
- line = line.substring(1);
- }
- }
- if (!firstLine) {
- sb.write('\n');
- }
- firstLine = false;
- sb.write(line);
- }
- str = sb.toString();
- // done
- return str;
-}
+import 'package:analyzer/src/generated/element.dart';
/**
* A computer for the hover at the specified offset of a Dart [CompilationUnit].
@@ -111,6 +69,7 @@
hover.containingLibraryPath = library.source.fullName;
}
}
+
// documentation
hover.dartdoc = _computeDocumentation(element);
}
@@ -132,8 +91,7 @@
if (element is ParameterElement) {
element = element.enclosingElement;
}
- String dartDoc = element.computeDocumentationComment();
- return _removeDartDocDelimiters(dartDoc);
+ return removeDartDocDelimiters(element.documentationComment);
}
static _safeToString(obj) => obj != null ? obj.toString() : null;
diff --git a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
index 218cc9f..2a537bf 100644
--- a/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
+++ b/pkg/analysis_server/lib/src/services/completion/dart/suggestion_builder.dart
@@ -8,6 +8,7 @@
import 'package:analysis_server/src/protocol_server.dart'
hide Element, ElementKind;
import 'package:analysis_server/src/provisional/completion/dart/completion_dart.dart';
+import 'package:analysis_server/src/utilities/documentation.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/dart/element/visitor.dart';
@@ -44,6 +45,12 @@
0,
isDeprecated,
false);
+
+ // Attach docs.
+ String doc = removeDartDocDelimiters(element.documentationComment);
+ suggestion.docComplete = doc;
+ suggestion.docSummary = getDartDocSummary(doc);
+
suggestion.element = protocol.convertElement(element);
Element enclosingElement = element.enclosingElement;
if (enclosingElement is ClassElement) {
diff --git a/pkg/analysis_server/lib/src/utilities/documentation.dart b/pkg/analysis_server/lib/src/utilities/documentation.dart
new file mode 100644
index 0000000..8e8e696
--- /dev/null
+++ b/pkg/analysis_server/lib/src/utilities/documentation.dart
@@ -0,0 +1,69 @@
+// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+library analysis_server.src.utilities.documentation;
+
+String getDartDocSummary(String str) {
+ if (str == null) {
+ return null;
+ }
+ List<String> lines = str.split('\n');
+ StringBuffer sb = new StringBuffer();
+ bool firstLine = true;
+ for (String line in lines) {
+ if (sb.length != 0 && line.isEmpty) {
+ return sb.toString();
+ }
+ if (!firstLine) {
+ sb.write('\n');
+ }
+ firstLine = false;
+ sb.write(line);
+ }
+ return sb.toString();
+}
+
+/**
+ * Converts [str] from a Dart Doc string with slashes and stars to a plain text
+ * representation of the comment.
+ */
+String removeDartDocDelimiters(String str) {
+ if (str == null) {
+ return null;
+ }
+ // remove /** */
+ if (str.startsWith('/**')) {
+ str = str.substring(3);
+ }
+ if (str.endsWith("*/")) {
+ str = str.substring(0, str.length - 2);
+ }
+ str = str.trim();
+ // remove leading '* ' and '/// '
+ List<String> lines = str.split('\n');
+ StringBuffer sb = new StringBuffer();
+ bool firstLine = true;
+ for (String line in lines) {
+ line = line.trim();
+ if (line.startsWith("*")) {
+ line = line.substring(1);
+ if (line.startsWith(" ")) {
+ line = line.substring(1);
+ }
+ } else if (line.startsWith("///")) {
+ line = line.substring(3);
+ if (line.startsWith(" ")) {
+ line = line.substring(1);
+ }
+ }
+ if (!firstLine) {
+ sb.write('\n');
+ }
+ firstLine = false;
+ sb.write(line);
+ }
+ str = sb.toString();
+ // done
+ return str;
+}
diff --git a/pkg/analysis_server/test/services/completion/dart/imported_reference_contributor_test.dart b/pkg/analysis_server/test/services/completion/dart/imported_reference_contributor_test.dart
index 231ce1d..00d80df 100644
--- a/pkg/analysis_server/test/services/completion/dart/imported_reference_contributor_test.dart
+++ b/pkg/analysis_server/test/services/completion/dart/imported_reference_contributor_test.dart
@@ -45,6 +45,71 @@
assertNotSuggested('two');
}
+ test_doc_class() async {
+ addSource(
+ '/libA.dart',
+ r'''
+library A;
+/// My class.
+/// Short description.
+///
+/// Longer description.
+class A {}
+''');
+ addTestSource('import "/libA.dart"; main() {^}');
+
+ await computeSuggestions();
+
+ CompletionSuggestion suggestion = assertSuggestClass('A');
+ expect(suggestion.docSummary, 'My class.\nShort description.');
+ expect(suggestion.docComplete,
+ 'My class.\nShort description.\n\nLonger description.');
+ }
+
+ test_doc_function() async {
+ resolveSource(
+ '/libA.dart',
+ r'''
+library A;
+/// My function.
+/// Short description.
+///
+/// Longer description.
+int myFunc() {}
+''');
+ addTestSource('import "/libA.dart"; main() {^}');
+
+ await computeSuggestions();
+
+ CompletionSuggestion suggestion = assertSuggestFunction('myFunc', 'int');
+ expect(suggestion.docSummary, 'My function.\nShort description.');
+ expect(suggestion.docComplete,
+ 'My function.\nShort description.\n\nLonger description.');
+ }
+
+ test_doc_function_c_style() async {
+ resolveSource(
+ '/libA.dart',
+ r'''
+library A;
+/**
+ * My function.
+ * Short description.
+ *
+ * Longer description.
+ */
+int myFunc() {}
+''');
+ addTestSource('import "/libA.dart"; main() {^}');
+
+ await computeSuggestions();
+
+ CompletionSuggestion suggestion = assertSuggestFunction('myFunc', 'int');
+ expect(suggestion.docSummary, 'My function.\nShort description.');
+ expect(suggestion.docComplete,
+ 'My function.\nShort description.\n\nLonger description.');
+ }
+
test_enum() async {
addSource('/libA.dart', 'library A; enum E { one, two }');
addTestSource('import "/libA.dart"; main() {^}');
diff --git a/pkg/analyzer/lib/dart/element/element.dart b/pkg/analyzer/lib/dart/element/element.dart
index 7d74842..e0efffc 100644
--- a/pkg/analyzer/lib/dart/element/element.dart
+++ b/pkg/analyzer/lib/dart/element/element.dart
@@ -538,9 +538,9 @@
* Elements with a smaller offset will be sorted to be before elements with a
* larger name offset.
*/
- static final Comparator<Element> SORT_BY_OFFSET = (Element firstElement,
- Element secondElement) =>
- firstElement.nameOffset - secondElement.nameOffset;
+ static final Comparator<Element> SORT_BY_OFFSET =
+ (Element firstElement, Element secondElement) =>
+ firstElement.nameOffset - secondElement.nameOffset;
/**
* Return the analysis context in which this element is defined.
@@ -558,9 +558,19 @@
String get displayName;
/**
+ * Return the content of the documentation comment (including delimiters) for
+ * this element, or `null` if this element does not or cannot have
+ * documentation.
+ */
+ String get documentationComment;
+
+ /**
* Return the source range of the documentation comment for this element,
* or `null` if this element does not or cannot have a documentation.
+ *
+ * Deprecated. Use [documentationComment] instead.
*/
+ @deprecated
SourceRange get docRange;
/**
@@ -677,7 +687,10 @@
*
* Throws [AnalysisException] if the documentation comment could not be
* determined because the analysis could not be performed
+ *
+ * Deprecated. Use [documentationComment] instead.
*/
+ @deprecated
String computeDocumentationComment();
/**
diff --git a/pkg/analyzer/lib/src/context/context.dart b/pkg/analyzer/lib/src/context/context.dart
index 32590d9..c9604ef 100644
--- a/pkg/analyzer/lib/src/context/context.dart
+++ b/pkg/analyzer/lib/src/context/context.dart
@@ -558,22 +558,8 @@
}
@override
- String computeDocumentationComment(Element element) {
- if (element == null) {
- return null;
- }
- Source source = element.source;
- if (source == null) {
- return null;
- }
- SourceRange docRange = element.docRange;
- if (docRange == null) {
- return null;
- }
- String code = getContents(source).data;
- String comment = code.substring(docRange.offset, docRange.end);
- return comment.replaceAll('\r\n', '\n');
- }
+ String computeDocumentationComment(Element element) =>
+ element?.documentationComment;
@override
List<AnalysisError> computeErrors(Source source) {
diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart
index 7d5164e..ff39868 100644
--- a/pkg/analyzer/lib/src/dart/element/element.dart
+++ b/pkg/analyzer/lib/src/dart/element/element.dart
@@ -1673,6 +1673,11 @@
ElementLocation _cachedLocation;
/**
+ * The documentation comment for this element.
+ */
+ String _docComment;
+
+ /**
* The offset to the beginning of the documentation comment,
* or `null` if this element does not have a documentation comment.
*/
@@ -1717,6 +1722,16 @@
}
@override
+ String get documentationComment => _docComment;
+
+ /**
+ * The documentation comment source for this element.
+ */
+ void set documentationComment(String doc) {
+ _docComment = doc?.replaceAll('\r\n', '\n');
+ }
+
+ @override
Element get enclosingElement => _enclosingElement;
/**
@@ -1866,13 +1881,7 @@
}
@override
- String computeDocumentationComment() {
- AnalysisContext context = this.context;
- if (context == null) {
- return null;
- }
- return context.computeDocumentationComment(this);
- }
+ String computeDocumentationComment() => documentationComment;
@override
AstNode computeNode() => getNodeMatching((node) => node is AstNode);
@@ -3236,7 +3245,8 @@
}
@override
- bool operator ==(Object object) => object is LibraryElementImpl &&
+ bool operator ==(Object object) =>
+ object is LibraryElementImpl &&
_definingCompilationUnit == object.definingCompilationUnit;
@override
@@ -3658,6 +3668,9 @@
SourceRange get docRange => null;
@override
+ String get documentationComment => null;
+
+ @override
Element get enclosingElement => null;
@override
@@ -4226,7 +4239,8 @@
}
@override
- bool operator ==(Object object) => super == object &&
+ bool operator ==(Object object) =>
+ super == object &&
isGetter == (object as PropertyAccessorElement).isGetter;
@override
diff --git a/pkg/analyzer/lib/src/dart/element/member.dart b/pkg/analyzer/lib/src/dart/element/member.dart
index ef22fa9..fea415f 100644
--- a/pkg/analyzer/lib/src/dart/element/member.dart
+++ b/pkg/analyzer/lib/src/dart/element/member.dart
@@ -442,6 +442,9 @@
@override
SourceRange get docRange => _baseElement.docRange;
+ @override
+ String get documentationComment => _baseElement.documentationComment;
+
int get id => _baseElement.id;
@override
@@ -487,8 +490,7 @@
CompilationUnit get unit => _baseElement.unit;
@override
- String computeDocumentationComment() =>
- _baseElement.computeDocumentationComment();
+ String computeDocumentationComment() => documentationComment;
@override
AstNode computeNode() => _baseElement.computeNode();
diff --git a/pkg/analyzer/lib/src/generated/element_handle.dart b/pkg/analyzer/lib/src/generated/element_handle.dart
index f35533b..f36d436 100644
--- a/pkg/analyzer/lib/src/generated/element_handle.dart
+++ b/pkg/analyzer/lib/src/generated/element_handle.dart
@@ -339,6 +339,9 @@
SourceRange get docRange => actualElement.docRange;
@override
+ String get documentationComment => actualElement.documentationComment;
+
+ @override
Element get enclosingElement => actualElement.enclosingElement;
@override
@@ -392,8 +395,7 @@
accept(ElementVisitor visitor) => actualElement.accept(visitor);
@override
- String computeDocumentationComment() =>
- actualElement.computeDocumentationComment();
+ String computeDocumentationComment() => documentationComment;
@override
AstNode computeNode() => actualElement.computeNode();
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 4266958..a52736d 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -2696,7 +2696,7 @@
interfaceType.typeArguments = typeArguments;
element.type = interfaceType;
element.typeParameters = typeParameters;
- _setDocRange(element, node);
+ _setDoc(element, node);
element.abstract = node.isAbstract;
element.accessors = holder.accessors;
List<ConstructorElement> constructors = holder.constructors;
@@ -2768,7 +2768,7 @@
SimpleIdentifier constructorName = node.name;
ConstructorElementImpl element =
new ConstructorElementImpl.forNode(constructorName);
- _setDocRange(element, node);
+ _setDoc(element, node);
if (node.externalKeyword != null) {
element.external = true;
}
@@ -2873,7 +2873,7 @@
SimpleIdentifier enumName = node.name;
ClassElementImpl enumElement = new ClassElementImpl.forNode(enumName);
enumElement.enum2 = true;
- _setDocRange(enumElement, node);
+ _setDoc(enumElement, node);
InterfaceTypeImpl enumType = new InterfaceTypeImpl(enumElement);
enumElement.type = enumType;
// The equivalent code for enums in the spec shows a single constructor,
@@ -2946,7 +2946,7 @@
SimpleIdentifier functionName = node.name;
FunctionElementImpl element =
new FunctionElementImpl.forNode(functionName);
- _setDocRange(element, node);
+ _setDoc(element, node);
if (node.externalKeyword != null) {
element.external = true;
}
@@ -2993,7 +2993,7 @@
if (node.isGetter) {
PropertyAccessorElementImpl getter =
new PropertyAccessorElementImpl.forNode(propertyNameNode);
- _setDocRange(getter, node);
+ _setDoc(getter, node);
if (node.externalKeyword != null) {
getter.external = true;
}
@@ -3019,7 +3019,7 @@
} else {
PropertyAccessorElementImpl setter =
new PropertyAccessorElementImpl.forNode(propertyNameNode);
- _setDocRange(setter, node);
+ _setDoc(setter, node);
if (node.externalKeyword != null) {
setter.external = true;
}
@@ -3110,7 +3110,7 @@
List<TypeParameterElement> typeParameters = holder.typeParameters;
FunctionTypeAliasElementImpl element =
new FunctionTypeAliasElementImpl.forNode(aliasName);
- _setDocRange(element, node);
+ _setDoc(element, node);
element.parameters = parameters;
element.typeParameters = typeParameters;
_createTypeParameterTypes(typeParameters);
@@ -3181,7 +3181,7 @@
}
MethodElementImpl element =
new MethodElementImpl(nameOfMethod, methodName.offset);
- _setDocRange(element, node);
+ _setDoc(element, node);
element.abstract = node.isAbstract;
if (node.externalKeyword != null) {
element.external = true;
@@ -3218,7 +3218,7 @@
if (node.isGetter) {
PropertyAccessorElementImpl getter =
new PropertyAccessorElementImpl.forNode(propertyNameNode);
- _setDocRange(getter, node);
+ _setDoc(getter, node);
if (node.externalKeyword != null) {
getter.external = true;
}
@@ -3244,7 +3244,7 @@
} else {
PropertyAccessorElementImpl setter =
new PropertyAccessorElementImpl.forNode(propertyNameNode);
- _setDocRange(setter, node);
+ _setDoc(setter, node);
if (node.externalKeyword != null) {
setter.external = true;
}
@@ -3382,7 +3382,7 @@
}
element = field;
if (node.parent.parent is FieldDeclaration) {
- _setDocRange(element, node.parent.parent);
+ _setDoc(element, node.parent.parent);
}
if ((node.parent as VariableDeclarationList).type == null) {
field.hasImplicitType = true;
@@ -3417,7 +3417,7 @@
}
element = variable;
if (node.parent.parent is TopLevelVariableDeclaration) {
- _setDocRange(element, node.parent.parent);
+ _setDoc(element, node.parent.parent);
}
if ((node.parent as VariableDeclarationList).type == null) {
variable.hasImplicitType = true;
@@ -3551,12 +3551,14 @@
}
/**
- * If the given [node] has a documentation comment, remember its range
- * into the given [element].
+ * If the given [node] has a documentation comment, remember its content
+ * and range into the given [element].
*/
- void _setDocRange(ElementImpl element, AnnotatedNode node) {
+ void _setDoc(ElementImpl element, AnnotatedNode node) {
Comment comment = node.documentationComment;
if (comment != null && comment.isDocumentation) {
+ element.documentationComment =
+ comment.tokens.map((Token t) => t.lexeme).join('\n');
element.setDocRange(comment.offset, comment.length);
}
}
diff --git a/pkg/analyzer/lib/src/task/dart.dart b/pkg/analyzer/lib/src/task/dart.dart
index 1a397af..5a64d4c 100644
--- a/pkg/analyzer/lib/src/task/dart.dart
+++ b/pkg/analyzer/lib/src/task/dart.dart
@@ -920,7 +920,7 @@
importElement.deferred = importDirective.deferredKeyword != null;
importElement.combinators = _buildCombinators(importDirective);
importElement.importedLibrary = importedLibrary;
- _setDocRange(importElement, importDirective);
+ _setDoc(importElement, importDirective);
SimpleIdentifier prefixNode = directive.prefix;
if (prefixNode != null) {
importElement.prefixOffset = prefixNode.offset;
@@ -962,7 +962,7 @@
exportElement.uri = exportDirective.uriContent;
exportElement.combinators = _buildCombinators(exportDirective);
exportElement.exportedLibrary = exportedLibrary;
- _setDocRange(exportElement, exportDirective);
+ _setDoc(exportElement, exportDirective);
directive.element = exportElement;
exports.add(exportElement);
if (exportSourceKindMap[exportedSource] != SourceKind.LIBRARY) {
@@ -1003,12 +1003,14 @@
}
/**
- * If the given [node] has a documentation comment, remember its range
- * into the given [element].
+ * If the given [node] has a documentation comment, remember its content
+ * and range into the given [element].
*/
- void _setDocRange(ElementImpl element, AnnotatedNode node) {
+ void _setDoc(ElementImpl element, AnnotatedNode node) {
Comment comment = node.documentationComment;
if (comment != null && comment.isDocumentation) {
+ element.documentationComment =
+ comment.tokens.map((Token t) => t.lexeme).join('\n');
element.setDocRange(comment.offset, comment.length);
}
}
@@ -1379,7 +1381,7 @@
_patchTopLevelAccessors(libraryElement);
}
if (libraryDirective != null) {
- _setDocRange(libraryElement, libraryDirective);
+ _setDoc(libraryElement, libraryDirective);
}
//
// Record outputs.
@@ -1467,12 +1469,14 @@
}
/**
- * If the given [node] has a documentation comment, remember its range
- * into the given [element].
+ * If the given [node] has a documentation comment, remember its content
+ * and range into the given [element].
*/
- void _setDocRange(ElementImpl element, AnnotatedNode node) {
+ void _setDoc(ElementImpl element, AnnotatedNode node) {
Comment comment = node.documentationComment;
if (comment != null && comment.isDocumentation) {
+ element.documentationComment =
+ comment.tokens.map((Token t) => t.lexeme).join('\n');
element.setDocRange(comment.offset, comment.length);
}
}
