From 4fea218f7763d5864d592fd40884227d0ecbc01a Mon Sep 17 00:00:00 2001 From: Konstantin Shcheglov Date: Tue, 22 Nov 2016 10:34:26 -0800 Subject: [PATCH] Deoptimize Search - request a new resolved unit for local search. It seems to me that it would be easier to integrate if we keep the same search technique - search for Element references. I think any saving can be only for local elements search. If it turns out to be expensive, we could add priority results caching in the driver. R=brianwilkerson@google.com BUG= Review URL: https://codereview.chromium.org/2521913004 . --- .../lib/src/dart/analysis/search.dart | 63 +++++++++---------- pkg/analyzer/test/src/dart/analysis/base.dart | 40 ++++++++++++ .../test/src/dart/analysis/index_test.dart | 37 ----------- .../test/src/dart/analysis/search_test.dart | 27 +++++--- 4 files changed, 89 insertions(+), 78 deletions(-) diff --git a/pkg/analyzer/lib/src/dart/analysis/search.dart b/pkg/analyzer/lib/src/dart/analysis/search.dart index e28a5b6dd5b..1dfe7b3600b 100644 --- a/pkg/analyzer/lib/src/dart/analysis/search.dart +++ b/pkg/analyzer/lib/src/dart/analysis/search.dart @@ -21,47 +21,46 @@ class Search { Search(this._driver); /** - * Returns references to the element at the given [offset] in the file with - * the given [path]. + * Returns references to the [element]. */ - Future> references(String path, int offset) async { - // Search only in added files. - if (!_driver.addedFiles.contains(path)) { - return const []; - } - - AnalysisResult analysisResult = await _driver.getResult(path); - CompilationUnit unit = analysisResult.unit; - - // Prepare the node. - AstNode node = new NodeLocator(offset).searchWithin(unit); - if (node == null) { - return const []; - } - - // Prepare the element. - Element element = ElementLocator.locate(node); + Future> references(Element element) async { if (element == null) { return const []; } ElementKind kind = element.kind; if (kind == ElementKind.LABEL || kind == ElementKind.LOCAL_VARIABLE) { - Block block = node.getAncestor((n) => n is Block); - return _searchReferences_Local(element, unit.element, block); + return _searchReferences_Local(element, (n) => n is Block); } // TODO(scheglov) support other kinds - return []; + return const []; } Future> _searchReferences_Local( - Element element, - CompilationUnitElement enclosingUnitElement, - AstNode enclosingNode) async { + Element element, bool isRootNode(AstNode n)) async { + String path = element.source.fullName; + + // Prepare the unit. + AnalysisResult analysisResult = await _driver.getResult(path); + CompilationUnit unit = analysisResult.unit; + if (unit == null) { + return const []; + } + + // Prepare the node. + AstNode node = new NodeLocator(element.nameOffset).searchWithin(unit); + if (node == null) { + return const []; + } + + // Prepare the enclosing node. + AstNode enclosingNode = node.getAncestor(isRootNode); + + // Find the matches. _LocalReferencesVisitor visitor = - new _LocalReferencesVisitor(element, enclosingUnitElement); - enclosingNode?.accept(visitor); - return visitor.matches; + new _LocalReferencesVisitor(element, unit.element); + enclosingNode.accept(visitor); + return visitor.results; } } @@ -159,7 +158,7 @@ class _ContainingElementFinder extends GeneralizingElementVisitor { * type parameters, import prefixes. */ class _LocalReferencesVisitor extends RecursiveAstVisitor { - final List matches = []; + final List results = []; final Element element; final CompilationUnitElement enclosingUnitElement; @@ -193,15 +192,15 @@ class _LocalReferencesVisitor extends RecursiveAstVisitor { kind = SearchResultKind.WRITE; } } - _addMatch(node, kind); + _addResult(node, kind); } } - void _addMatch(AstNode node, SearchResultKind kind) { + void _addResult(AstNode node, SearchResultKind kind) { bool isQualified = node.parent is Label; var finder = new _ContainingElementFinder(node.offset); enclosingUnitElement.accept(finder); - matches.add(new SearchResult._(element, finder.containingElement, kind, + results.add(new SearchResult._(element, finder.containingElement, kind, node.offset, node.length, true, isQualified)); } } diff --git a/pkg/analyzer/test/src/dart/analysis/base.dart b/pkg/analyzer/test/src/dart/analysis/base.dart index 796b9b8b424..bb716cf041a 100644 --- a/pkg/analyzer/test/src/dart/analysis/base.dart +++ b/pkg/analyzer/test/src/dart/analysis/base.dart @@ -4,6 +4,8 @@ import 'dart:async'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/visitor.dart'; import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/file_system/memory_file_system.dart'; import 'package:analyzer/source/package_map_resolver.dart'; @@ -17,6 +19,30 @@ import 'package:test/test.dart'; import '../../context/mock_sdk.dart'; +/** + * Finds an [Element] with the given [name]. + */ +Element findChildElement(Element root, String name, [ElementKind kind]) { + Element result = null; + root.accept(new _ElementVisitorFunctionWrapper((Element element) { + if (element.name != name) { + return; + } + if (kind != null && element.kind != kind) { + return; + } + result = element; + })); + return result; +} + +typedef bool Predicate(E argument); + +/** + * A function to be called for every [Element]. + */ +typedef void _ElementVisitorFunction(Element element); + class BaseAnalysisDriverTest { static final MockSdk sdk = new MockSdk(); @@ -108,6 +134,20 @@ class BaseAnalysisDriverTest { String _p(String path) => provider.convertPath(path); } +/** + * Wraps an [_ElementVisitorFunction] into a [GeneralizingElementVisitor]. + */ +class _ElementVisitorFunctionWrapper extends GeneralizingElementVisitor { + final _ElementVisitorFunction function; + + _ElementVisitorFunctionWrapper(this.function); + + visitElement(Element element) { + function(element); + super.visitElement(element); + } +} + class _Monitor { Completer _completer = new Completer(); diff --git a/pkg/analyzer/test/src/dart/analysis/index_test.dart b/pkg/analyzer/test/src/dart/analysis/index_test.dart index 8c544c72d7a..04048636e32 100644 --- a/pkg/analyzer/test/src/dart/analysis/index_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/index_test.dart @@ -7,7 +7,6 @@ import 'dart:convert'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/visitor.dart'; import 'package:analyzer/src/dart/analysis/driver.dart'; import 'package:analyzer/src/dart/analysis/index.dart'; import 'package:analyzer/src/summary/format.dart'; @@ -23,28 +22,6 @@ main() { }); } -/** - * Finds an [Element] with the given [name]. - */ -Element findChildElement(Element root, String name, [ElementKind kind]) { - Element result = null; - root.accept(new _ElementVisitorFunctionWrapper((Element element) { - if (element.name != name) { - return; - } - if (kind != null && element.kind != kind) { - return; - } - result = element; - })); - return result; -} - -/** - * A function to be called for every [Element]. - */ -typedef void _ElementVisitorFunction(Element element); - class ExpectedLocation { final CompilationUnitElement unitElement; final int offset; @@ -1210,20 +1187,6 @@ class _ElementIndexAssert { } } -/** - * Wraps an [_ElementVisitorFunction] into a [GeneralizingElementVisitor]. - */ -class _ElementVisitorFunctionWrapper extends GeneralizingElementVisitor { - final _ElementVisitorFunction function; - - _ElementVisitorFunctionWrapper(this.function); - - visitElement(Element element) { - function(element); - super.visitElement(element); - } -} - class _NameIndexAssert { final IndexTest test; final String name; diff --git a/pkg/analyzer/test/src/dart/analysis/search_test.dart b/pkg/analyzer/test/src/dart/analysis/search_test.dart index 03a2a5316bd..67ade8b5a45 100644 --- a/pkg/analyzer/test/src/dart/analysis/search_test.dart +++ b/pkg/analyzer/test/src/dart/analysis/search_test.dart @@ -4,8 +4,11 @@ import 'dart:async'; +import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/src/dart/analysis/driver.dart'; import 'package:analyzer/src/dart/analysis/search.dart'; +import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; @@ -90,13 +93,13 @@ label: } } '''); - int offset = findOffset('label:'); + Element element = await _findElementAtString('label:'); List main = [testUri, 'main']; var expected = [ _expectId(main, SearchResultKind.REFERENCE, 'label; // 1'), _expectId(main, SearchResultKind.REFERENCE, 'label; // 2') ]; - await _verifyReferences(offset, expected); + await _verifyReferences(element, expected); } test_searchReferences_localVariable() async { @@ -109,7 +112,7 @@ main() { v(); } '''); - int offset = findOffset('v;'); + Element element = await _findElementAtString('v;'); List main = [testUri, 'main']; var expected = [ _expectId(main, SearchResultKind.WRITE, 'v = 1;'), @@ -117,7 +120,7 @@ main() { _expectId(main, SearchResultKind.READ, 'v);'), _expectId(main, SearchResultKind.INVOCATION, 'v();') ]; - await _verifyReferences(offset, expected); + await _verifyReferences(element, expected); } test_searchReferences_localVariable_inForEachLoop() async { @@ -131,7 +134,7 @@ main() { } } '''); - int offset = findOffset('v in []'); + Element element = await _findElementAtString('v in []'); List main = [testUri, 'main']; var expected = [ _expectId(main, SearchResultKind.WRITE, 'v = 1;'), @@ -139,7 +142,7 @@ main() { _expectId(main, SearchResultKind.READ, 'v);'), _expectId(main, SearchResultKind.INVOCATION, 'v();') ]; - await _verifyReferences(offset, expected); + await _verifyReferences(element, expected); } ExpectedResult _expectId( @@ -153,10 +156,16 @@ main() { isResolved: isResolved, isQualified: isQualified); } + Future _findElementAtString(String search) async { + AnalysisResult result = await driver.getResult(testFile); + int offset = findOffset(search); + AstNode node = new NodeLocator(offset).searchWithin(result.unit); + return ElementLocator.locate(node); + } + Future _verifyReferences( - int offset, List expectedMatches) async { - List results = - await driver.search.references(testFile, offset); + Element element, List expectedMatches) async { + List results = await driver.search.references(element); _assertResults(results, expectedMatches); expect(results, hasLength(expectedMatches.length)); }