Issue 27542. Guard against CompilationUnitElement is null in SearchMa…

…tch.element getter. [email protected] BUG= #27542 Review URL: https://codereview.chromium.org/2409403002 .
dart-lang · Oct 11, 2016 · 56bde6d · 56bde6d
1 parent 9b2c5ce
commit 56bde6d
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 45 deletions.
diff --git a/pkg/analysis_server/lib/src/search/element_references.dart b/pkg/analysis_server/lib/src/search/element_references.dart
@@ -6,7 +6,6 @@ library search.element_references;
 
 import 'dart:async';
 
-import 'package:analysis_server/src/collections.dart';
 import 'package:analysis_server/src/protocol_server.dart'
     show SearchResult, newSearchResult_fromMatch;
 import 'package:analysis_server/src/services/search/hierarchy.dart';
@@ -25,40 +24,44 @@ class ElementReferencesComputer {
   /**
    * Computes [SearchResult]s for [element] references.
    */
-  Future<List<SearchResult>> compute(Element element, bool withPotential) {
-    var futureGroup = new _ConcatFutureGroup<SearchResult>();
-    // find element references
-    futureGroup.add(_findElementsReferences(element));
-    // add potential references
+  Future<List<SearchResult>> compute(
+      Element element, bool withPotential) async {
+    List<SearchResult> results = <SearchResult>[];
+
+    // Add element references.
+    results.addAll(await _findElementsReferences(element));
+
+    // Add potential references.
     if (withPotential && _isMemberElement(element)) {
       String name = element.displayName;
-      var matchesFuture = searchEngine.searchMemberReferences(name);
-      var resultsFuture = matchesFuture.then((List<SearchMatch> matches) {
-        return matches.where((match) => !match.isResolved).map(toResult);
-      });
-      futureGroup.add(resultsFuture);
+      List<SearchMatch> matches =
+          await searchEngine.searchMemberReferences(name);
+      matches = SearchMatch.withNotNullElement(matches);
+      results.addAll(matches.where((match) => !match.isResolved).map(toResult));
     }
-    // merge results
-    return futureGroup.future;
+
+    return results;
   }
 
   /**
    * Returns a [Future] completing with a [List] of references to [element] or
    * to the corresponding hierarchy [Element]s.
    */
   Future<List<SearchResult>> _findElementsReferences(Element element) async {
+    List<SearchResult> allResults = <SearchResult>[];
     Iterable<Element> refElements = await _getRefElements(element);
-    var futureGroup = new _ConcatFutureGroup<SearchResult>();
     for (Element refElement in refElements) {
       // add declaration
       if (_isDeclarationInteresting(refElement)) {
         SearchResult searchResult = _newDeclarationResult(refElement);
-        futureGroup.add(searchResult);
+        allResults.add(searchResult);
       }
       // do search
-      futureGroup.add(_findSingleElementReferences(refElement));
+      List<SearchResult> elementResults =
+          await _findSingleElementReferences(refElement);
+      allResults.addAll(elementResults);
     }
-    return futureGroup.future;
+    return allResults;
   }
 
   /**
@@ -67,6 +70,7 @@ class ElementReferencesComputer {
   Future<List<SearchResult>> _findSingleElementReferences(
       Element element) async {
     List<SearchMatch> matches = await searchEngine.searchReferences(element);
+    matches = SearchMatch.withNotNullElement(matches);
     return matches.map(toResult).toList();
   }
 
@@ -129,26 +133,3 @@ class ElementReferencesComputer {
     return element.enclosingElement is ClassElement;
   }
 }
-
-/**
- * A collection of [Future]s that concats [List] results of added [Future]s into
- * a single [List].
- */
-class _ConcatFutureGroup<E> {
-  final List<Future<List<E>>> _futures = <Future<List<E>>>[];
-
-  Future<List<E>> get future {
-    return Future.wait(_futures).then(concatToList);
-  }
-
-  /**
-   * Adds a [Future] or an [E] value to results.
-   */
-  void add(value) {
-    if (value is Future) {
-      _futures.add(value as Future<List<E>>);
-    } else {
-      _futures.add(new Future.value(<E>[value as E]));
-    }
-  }
-}
diff --git a/pkg/analysis_server/lib/src/search/search_domain.dart b/pkg/analysis_server/lib/src/search/search_domain.dart
@@ -98,6 +98,7 @@ class SearchDomainHandler implements protocol.RequestHandler {
     // search
     List<SearchMatch> matches =
         await searchEngine.searchMemberDeclarations(params.name);
+    matches = SearchMatch.withNotNullElement(matches);
     _sendSearchNotification(searchId, true, matches.map(toResult));
   }
 
@@ -112,6 +113,7 @@ class SearchDomainHandler implements protocol.RequestHandler {
     // search
     List<SearchMatch> matches =
         await searchEngine.searchMemberReferences(params.name);
+    matches = SearchMatch.withNotNullElement(matches);
     _sendSearchNotification(searchId, true, matches.map(toResult));
   }
 
@@ -135,6 +137,7 @@ class SearchDomainHandler implements protocol.RequestHandler {
     // search
     List<SearchMatch> matches =
         await searchEngine.searchTopLevelDeclarations(params.pattern);
+    matches = SearchMatch.withNotNullElement(matches);
     _sendSearchNotification(searchId, true, matches.map(toResult));
   }
 

diff --git a/pkg/analysis_server/lib/src/services/search/search_engine.dart b/pkg/analysis_server/lib/src/services/search/search_engine.dart
@@ -155,16 +155,20 @@ class SearchMatch {
       this.sourceRange, this.isResolved, this.isQualified);
 
   /**
-   * Return the [Element] containing the match.
+   * Return the [Element] containing the match. Can return `null` if the unit
+   * does not exist, or its element was invalidated, or the element cannot be
+   * found, etc.
    */
   Element get element {
     if (_element == null) {
       CompilationUnitElement unitElement =
           context.getCompilationUnitElement(unitSource, librarySource);
-      _ContainingElementFinder finder =
-          new _ContainingElementFinder(sourceRange.offset);
-      unitElement.accept(finder);
-      _element = finder.containingElement;
+      if (unitElement != null) {
+        _ContainingElementFinder finder =
+            new _ContainingElementFinder(sourceRange.offset);
+        unitElement.accept(finder);
+        _element = finder.containingElement;
+      }
     }
     return _element;
   }
@@ -237,6 +241,16 @@ class SearchMatch {
     buffer.write(")");
     return buffer.toString();
   }
+
+  /**
+   * Return elements of [matches] which has not-null elements.
+   *
+   * When [SearchMatch.element] is not `null` we cache its value, so it cannot
+   * become `null` later.
+   */
+  static List<SearchMatch> withNotNullElement(List<SearchMatch> matches) {
+    return matches.where((match) => match.element != null).toList();
+  }
 }
 
 /**

diff --git a/pkg/analysis_server/test/services/search/search_engine_test.dart b/pkg/analysis_server/test/services/search/search_engine_test.dart
@@ -617,6 +617,23 @@ main(A<int> a) {
     await _verifyReferences(method, expected);
   }
 
+  test_searchReferences_null_noUnitElement() async {
+    _indexTestUnit('''
+class A {
+  m() {}
+}
+main(A a) {
+  a.m();
+}
+''');
+    MethodElement method = findElement('m');
+    List<SearchMatch> matches = await searchEngine.searchReferences(method);
+    expect(matches, hasLength(1));
+    // Set the source contents, so the element is invalidated.
+    context.setContents(testSource, '');
+    expect(matches.single.element, isNull);
+  }
+
   test_searchReferences_ParameterElement_ofConstructor() async {
     _indexTestUnit('''
 class C {