diff --git a/pkg/analysis_server/tool/code_completion/completion_metrics.dart b/pkg/analysis_server/tool/code_completion/completion_metrics.dart index 46638b12ea9..49d514d371c 100644 --- a/pkg/analysis_server/tool/code_completion/completion_metrics.dart +++ b/pkg/analysis_server/tool/code_completion/completion_metrics.dart @@ -58,6 +58,32 @@ import 'output_utilities.dart'; import 'relevance_table_generator.dart'; import 'visitors.dart'; +/// Completion metrics are computed by taking a single package and iterating +/// over the compilation units in the package. For each unit we visit the AST +/// structure to find all of the places where completion suggestions should be +/// offered (essentially, everywhere that there's a keyword or identifier). At +/// each location we compute the completion suggestions using the same code path +/// used by the analysis server. We then compare the suggestions against the +/// token that was actually at that location in the file. +/// +/// This approach has several drawbacks: +/// +/// - The AST is always complete and correct, and that's rarely the case for +/// real completion requests. Usually the tree is incomplete and often has a +/// completely different structure because of the way recovery works. We +/// currently have no way of measuring completions under realistic conditions. +/// +/// - We can't measure completions for several keywords because the presence of +/// the keyword in the AST causes it to not be suggested. +/// +/// - The time it takes to compute the suggestions doesn't include the time +/// required to finish analyzing the file if the analysis hasn't been +/// completed before suggestions are requested. While the times are accurate +/// (within the accuracy of the `Stopwatch` class) they are the minimum +/// possible time. This doesn't give us a measure of how completion will +/// perform in production, but does give us an optimistic approximation. +/// +/// The first is arguably the worst of the limitations of our current approach. Future main(List args) async { var parser = createArgParser(); var result = parser.parse(args); @@ -1318,7 +1344,7 @@ class CompletionMetricsComputer { var filePath = result.path; // Use the ExpectedCompletionsVisitor to compute the set of expected // completions for this CompilationUnit. - final visitor = ExpectedCompletionsVisitor(filePath); + final visitor = ExpectedCompletionsVisitor(result); _resolvedUnitResult.unit.accept(visitor); for (var expectedCompletion in visitor.expectedCompletions) { diff --git a/pkg/analysis_server/tool/code_completion/visitors.dart b/pkg/analysis_server/tool/code_completion/visitors.dart index 5a17c5ef157..578375842a9 100644 --- a/pkg/analysis_server/tool/code_completion/visitors.dart +++ b/pkg/analysis_server/tool/code_completion/visitors.dart @@ -5,11 +5,13 @@ import 'package:analysis_server/src/protocol/protocol_internal.dart'; import 'package:analysis_server/src/protocol_server.dart' as protocol; import 'package:analysis_server/src/services/completion/dart/keyword_contributor.dart'; +import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/syntactic_entity.dart'; import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart' as element; +import 'package:analyzer/source/line_info.dart'; class ExpectedCompletion { final String _filePath; @@ -122,11 +124,12 @@ class ExpectedCompletion { } class ExpectedCompletionsVisitor extends RecursiveAstVisitor { - final List expectedCompletions; + /// The result of resolving the file being visited. + final ResolvedUnitResult result; - final String filePath; - - late CompilationUnit _enclosingCompilationUnit; + /// The completions that are expected to be produced in the file being + /// visited. + final List expectedCompletions = []; /// This boolean is set to enable whether or not we should assert that some /// found keyword in Dart syntax should be in the completion set returned from @@ -141,22 +144,22 @@ class ExpectedCompletionsVisitor extends RecursiveAstVisitor { /// comment don't yield an error like Dart syntax mistakes would yield. final bool _doExpectCommentRefs = false; - ExpectedCompletionsVisitor(this.filePath) - : expectedCompletions = []; + ExpectedCompletionsVisitor(this.result); + + /// Return the path of the file that is being visited. + String get filePath => result.path; + + /// Return the line info for the file that is being visited. + LineInfo get lineInfo => result.lineInfo; void safelyRecordEntity(SyntacticEntity? entity, {protocol.CompletionSuggestionKind? kind, protocol.ElementKind? elementKind}) { // Only record if this entity is not null, has a length, etc. - if (entity != null && entity.offset > 0 && entity.length > 0) { - // Compute the line number at this offset - var lineNumber = _enclosingCompilationUnit.lineInfo! - .getLocation(entity.offset) - .lineNumber; - - var columnNumber = _enclosingCompilationUnit.lineInfo! - .getLocation(entity.offset) - .columnNumber; + if (entity != null && entity.offset >= 0 && entity.length > 0) { + var location = lineInfo.getLocation(entity.offset); + var lineNumber = location.lineNumber; + var columnNumber = location.columnNumber; bool isKeyword() => kind == protocol.CompletionSuggestionKind.KEYWORD; @@ -314,12 +317,6 @@ class ExpectedCompletionsVisitor extends RecursiveAstVisitor { super.visitClassTypeAlias(node); } - @override - void visitCompilationUnit(CompilationUnit node) { - _enclosingCompilationUnit = node; - super.visitCompilationUnit(node); - } - @override void visitConfiguration(Configuration node) { safelyRecordKeywordCompletion(node.ifKeyword);