Implement a task to compute the dependencies between static variables

pkg/analyzer/lib/src/task/strong_mode.dart

Index: pkg/analyzer/lib/src/task/strong_mode.dart
diff --git a/pkg/analyzer/lib/src/task/strong_mode.dart b/pkg/analyzer/lib/src/task/strong_mode.dart
index 823319893f36c3e1831fe4786ed144c8a8f823e0..728a45fe16efc6494dda2ca9cab85cd44c24cb18 100644
--- a/pkg/analyzer/lib/src/task/strong_mode.dart
+++ b/pkg/analyzer/lib/src/task/strong_mode.dart
@@ -8,10 +8,71 @@ import 'dart:collection';
 
 import 'package:analyzer/src/generated/ast.dart';
 import 'package:analyzer/src/generated/element.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:analyzer/src/generated/java_engine.dart';
 import 'package:analyzer/src/generated/resolver.dart';
 import 'package:analyzer/src/generated/utilities_dart.dart';
 
 /**
+ * A function that returns `true` if the given [variable] passes the filter.
+ */
+typedef bool VariableFilter(VariableElement element);
+
+/**
+ * A visitor that will locate the declaration of a specific variable within an
+ * AST structure.
+ */
+class DeclarationFinder extends RecursiveAstVisitor {

[Paul Berry, 2015/08/27 18:56:07]

I'm concerned about the execution speed of this class, since it recursively
visits the entire AST, even parts of it that are distant from the target
element.

Why not use NodeLocator instead?  It is smart about pruning the search to just
the parts of the AST that overlap with the searched-for location, so it should
be much faster.

[Brian Wilkerson, 2015/08/27 20:43:54]

Because I didn't think of it. You're right, it would be better.

+  /**
+   * The element representing the variable whose declaration is to be found.
+   */
+  final VariableElement targetElement;
+
+  /**
+   * The declaration of the variable, or `null` if the AST structure did not
+   * contain a declaration of the variable.
+   */
+  VariableDeclaration declaration = null;
+
+  /**
+   * Initialize a newly created finder to search for the given [targetElement].
+   */
+  DeclarationFinder(this.targetElement);
+
+  @override
+  void visitVariableDeclaration(VariableDeclaration node) {
+    if (node.name.staticElement == targetElement) {
+      declaration = node;
+    }

[Paul Berry, 2015/08/27 18:56:07]

Were you intending to add:

throw new DeclarationFoundException();

here?  Assuming you were, consider changing the code so that the found
declaration is stored inside the DeclarationFoundException itself, rather than
in the visitor class.  That way the data flow is clearer, since the data we are
searching for rides along with the exception class that we are using to unwind
the stack when it is found.

[Brian Wilkerson, 2015/08/27 20:43:54]

moot

+  }
+
+  /**
+   * Search inside the given AST [node] for the declaration of the variable with
+   * the given [element].
+   */
+  static VariableDeclaration find(AstNode node, VariableElement element) {

[Paul Berry, 2015/08/27 18:56:08]

I'm not comfortable with the way this function is handling error conditions. 
There are two sources of error conditions:

1. an unexpected exception occurs while traversing the AST structure.  In this
case I think it would make sense to simply let the exception bubble up the stack
(as it would in any other code that wasn't being tricky with exceptions).  But
instead we turn the exception into a log entry and return null, which makes it
harder to notice, and may cause the caller to misbehave in a way that won't be
easily traceable to the source of the problem.

2. the node is not found.  In this case I think we should throw an exception,
because it is an exceptional circumstance (this should never happen in the
absence of a bug in the task model).  But instead we return null.  I'm less
bothered by this case than the previous one (since if it ever arises, I think it
will be relatively easy to guess based on the null return value that the node
wasn't found), but I think raising an exception would be even better since there
would be no guesswork required.

[Brian Wilkerson, 2015/08/27 20:43:54]

moot

+    DeclarationFinder finder = new DeclarationFinder(element);
+    try {
+      node.accept(finder);
+    } on DeclarationFoundException {
+      // A declaration of the element was found
+    } catch (exception, stackTrace) {
+      AnalysisEngine.instance.logger.logInformation(
+          "Unable to locate declaration of $element",
+          new CaughtException(exception, stackTrace));
+      return null;
+    }
+    return finder.declaration;
+  }
+}
+
+/**
+ * An exception used by a [DeclarationFinder] to cancel visiting after a
+ * declaration has been found.
+ */
+class DeclarationFoundException extends Error {}
+
+/**
  * An object used to find static variables whose types should be inferred and
  * classes whose members should have types inferred. Clients are expected to
  * visit a [CompilationUnit].
@@ -79,7 +140,10 @@ class InferrenceFinder extends SimpleAstVisitor {
   void _addVariables(NodeList<VariableDeclaration> variables) {
     for (VariableDeclaration variable in variables) {
       if (variable.initializer != null) {
-        staticVariables.add(variable.element);
+        VariableElement element = variable.element;
+        if (element.hasImplicitType) {
+          staticVariables.add(element);
+        }
       }
     }
   }
@@ -529,6 +593,43 @@ class InstanceMemberInferrer {
 }
 
 /**
+ * A visitor that will gather all of the variables referenced within a given
+ * AST structure. The collection can be restricted to contain only those
+ * variables that pass a specified filter.
+ */
+class VariableGatherer extends RecursiveAstVisitor {
+  /**
+   * The filter used to limit which variables are gathered, or `null` if no
+   * filtering is to be performed.
+   */
+  final VariableFilter filter;
+
+  /**
+   * The variables that were found.
+   */
+  final Set<VariableElement> results = new HashSet<VariableElement>();
+
+  /**
+   * Initialize a newly created gatherer to gather all of the variables that
+   * pass the given [filter] (or all variables if no filter is provided).
+   */
+  VariableGatherer([this.filter = null]);
+
+  @override
+  void visitSimpleIdentifier(SimpleIdentifier node) {
+    if (!node.inDeclarationContext()) {
+      Element element = node.staticElement;
+      if (element is PropertyAccessorElement && element.isSynthetic) {
+        element = (element as PropertyAccessorElement).variable;
+      }
+      if (element is VariableElement && (filter == null || filter(element))) {
+        results.add(element);
+      }
+    }
+  }
+}
+
+/**
  * A class of exception that is not used anywhere else.
  */
 class _CycleException implements Exception {}