Task to infer 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 7b406d85872f98a96e63b8b4fb425d2b52ffb608..3c18f89f9c30dd3007d67b8e08047cb9aae80609 100644
--- a/pkg/analyzer/lib/src/task/strong_mode.dart
+++ b/pkg/analyzer/lib/src/task/strong_mode.dart
@@ -8,12 +8,65 @@ 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';
 
 /**
+ * Set the type of the sole parameter of the given [element] to the given [type].
+ */
+void setParameterType(PropertyAccessorElement element, DartType type) {
+  if (element is PropertyAccessorElementImpl) {

[Paul Berry, 2015/08/29 01:14:55]

I realize this is just code motion, so feel free to defer this to a later CL if
you'd prefer.

Could this function ever get called for a non-PropertyAccessorElementImpl in the
absence of analyzer bugs?  If not, please add an assertion so that checked mode
unit tests will fail if this function ever gets passed an unexpected type.

Similarly for the other "if" tests in this function.

[Brian Wilkerson, 2015/08/31 15:08:52]

Each of the 'if' statements in these methods is there to avoid needing to have
an explicit cast somewhere later. Unfortunately assert statements don't have the
same effect.

[Paul Berry, 2015/08/31 15:50:59]

I understand that.  I am not suggesting replacing the "if" tests with
assertions.  I'm suggesting *adding* assertions so that bugs won't go unnoticed.

For example, you could do:

assert(element is PropertyAccessorImpl);
if (element is PropertyAccessorImpl) {
  ...
}

or:

if (element is PropertyAccessorImpl) {
  ...
} else {
  assert(false);
}

Both of these alternatives would have the advantage of extra unit test strength,
without sacrificing runtime performance or robustness in unchecked code.

[Brian Wilkerson, 2015/08/31 18:35:38]

While I understand the testing value, I really don't like either alternative.
They both clutter the code; the first worse than the second because of the
duplication of the condition. I'll add the second form, but under protest.

+    ParameterElement parameter = _getParameter(element);
+    if (parameter is ParameterElementImpl) {
+      //
+      // Update the type of the parameter.
+      //
+      parameter.type = type;
+      //
+      // Update the type of the setter to reflect the new parameter type.
+      //
+      FunctionType functionType = element.type;
+      if (functionType is FunctionTypeImpl) {
+        element.type =
+            new FunctionTypeImpl(element, functionType.prunedTypedefs);
+      }
+    }
+  }
+}
+
+/**
+ * Set the return type of the given [element] to the given [type].
+ */
+void setReturnType(ExecutableElement element, DartType type) {
+  if (element is ExecutableElementImpl) {

[Paul Berry, 2015/08/29 01:14:55]

Similar concerns in this function.

+    //
+    // Update the return type of the element, which is stored in two places:
+    // directly in the element and indirectly in the type of the element.
+    //
+    element.returnType = type;
+    FunctionType functionType = element.type;
+    if (functionType is FunctionTypeImpl) {
+      element.type = new FunctionTypeImpl(element, functionType.prunedTypedefs);
+    }
+  }
+}
+
+/**
+ * Return the element for the single parameter of the given [setter], or `null`
+ * if the executable element is not a setter or does not have a single
+ * parameter.
+ */
+ParameterElement _getParameter(ExecutableElement setter) {
+  if (setter is PropertyAccessorElement && setter.isSetter) {
+    List<ParameterElement> parameters = setter.parameters;
+    if (parameters.length == 1) {
+      return parameters[0];
+    }
+  }
+  return null;
+}
+
+/**
  * A function that returns `true` if the given [variable] passes the filter.
  */
 typedef bool VariableFilter(VariableElement element);
@@ -193,21 +246,6 @@ class InstanceMemberInferrer {
     return returnType == null ? typeProvider.dynamicType : returnType;
   }
 
-  /**
-   * Return the element for the single parameter of the given [setter], or
-   * `null` if the executable element is not a setter or does not have a single
-   * parameter.
-   */
-  ParameterElement _getParameter(ExecutableElement setter) {
-    if (setter is PropertyAccessorElement && setter.isSetter) {
-      List<ParameterElement> parameters = setter.parameters;
-      if (parameters.length == 1) {
-        return parameters[0];
-      }
-    }
-    return null;
-  }
-
   DartType _getReturnType(ExecutableElement element) {
     DartType returnType = element.returnType;
     if (returnType == null) {
@@ -284,7 +322,7 @@ class InstanceMemberInferrer {
         if (!_isCompatible(newType, overriddenSetters)) {
           newType = typeProvider.dynamicType;
         }
-        _setReturnType(accessorElement, newType);
+        setReturnType(accessorElement, newType);
         (accessorElement.variable as FieldElementImpl).type = newType;
       }
     }
@@ -365,8 +403,8 @@ class InstanceMemberInferrer {
         newType = typeProvider.dynamicType;
       }
       (fieldElement as FieldElementImpl).type = newType;
-      _setReturnType(fieldElement.getter, newType);
-      _setParameterType(fieldElement.setter, newType);
+      setReturnType(fieldElement.getter, newType);
+      setParameterType(fieldElement.setter, newType);
     }
   }
 
@@ -389,7 +427,7 @@ class InstanceMemberInferrer {
         return;
       }
       MethodElementImpl element = methodElement as MethodElementImpl;
-      _setReturnType(element, _computeReturnType(overriddenMethods));
+      setReturnType(element, _computeReturnType(overriddenMethods));
     }
     //
     // Infer the parameter types.
@@ -460,47 +498,6 @@ class InstanceMemberInferrer {
     }
     return true;
   }
-
-  /**
-   * Set the type of the sole parameter of the given [element] to the given [type].
-   */
-  void _setParameterType(PropertyAccessorElement element, DartType type) {
-    if (element is PropertyAccessorElementImpl) {
-      ParameterElement parameter = _getParameter(element);
-      if (parameter is ParameterElementImpl) {
-        //
-        // Update the type of the parameter.
-        //
-        parameter.type = type;
-        //
-        // Update the type of the setter to reflect the new parameter type.
-        //
-        FunctionType functionType = element.type;
-        if (functionType is FunctionTypeImpl) {
-          element.type =
-              new FunctionTypeImpl(element, functionType.prunedTypedefs);
-        }
-      }
-    }
-  }
-
-  /**
-   * Set the return type of the given [element] to the given [type].
-   */
-  void _setReturnType(ExecutableElement element, DartType type) {
-    if (element is ExecutableElementImpl) {
-      //
-      // Update the return type of the element, which is stored in two places:
-      // directly in the element and indirectly in the type of the element.
-      //
-      element.returnType = type;
-      FunctionType functionType = element.type;
-      if (functionType is FunctionTypeImpl) {
-        element.type =
-            new FunctionTypeImpl(element, functionType.prunedTypedefs);
-      }
-    }
-  }
 }
 
 /**