Task to infer static variables

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

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library analyzer.src.task.strong_mode;
 
 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);
 
 /**
  * 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].
  */
 class InferrenceFinder extends SimpleAstVisitor {
@@ skipping 159 matching lines @@
       DartType type = _getReturnType(overriddenMethods[i]);
       if (returnType == null) {
         returnType = type;
       } else if (returnType != type) {
         return typeProvider.dynamicType;
       }
     }
     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) {
       return typeProvider.dynamicType;
     }
     return returnType;
   }
 
   /**
    * Given a [method], return the type of the parameter in the method that
@@ skipping 56 matching lines @@
             .lookupOverrides(
                 accessorElement.enclosingElement, accessorElement.name + '=');
         PropertyAccessorElement setter = (accessorElement.enclosingElement
             as ClassElement).getSetter(accessorElement.name);
         if (setter != null) {
           overriddenSetters.add(setter);
         }
         if (!_isCompatible(newType, overriddenSetters)) {
           newType = typeProvider.dynamicType;
         }
-        _setReturnType(accessorElement, newType);
+        setReturnType(accessorElement, newType);
         (accessorElement.variable as FieldElementImpl).type = newType;
       }
     }
   }
 
   /**
    * Infer type information for all of the instance members in the given
    * [classElement].
    */
   void _inferClass(ClassElement classElement) {
@@ skipping 60 matching lines @@
       if (newType == null) {
         if (fieldElement.initializer != null &&
             (fieldElement.isFinal || overriddenGetters.isEmpty)) {
           newType = fieldElement.initializer.returnType;
         }
       }
       if (newType == null || newType.isBottom) {
         newType = typeProvider.dynamicType;
       }
       (fieldElement as FieldElementImpl).type = newType;
-      _setReturnType(fieldElement.getter, newType);
-      _setParameterType(fieldElement.setter, newType);
+      setReturnType(fieldElement.getter, newType);
+      setParameterType(fieldElement.setter, newType);
     }
   }
 
   /**
    * If the given [methodElement] represents a non-synthetic instance method
    * for which no return type was provided, infer the return type of the method.
    */
   void _inferMethod(MethodElement methodElement) {
     if (methodElement.isSynthetic || methodElement.isStatic) {
       return;
     }
     List<ExecutableElement> overriddenMethods = null;
     //
     // Infer the return type.
     //
     if (methodElement.hasImplicitReturnType) {
       overriddenMethods = inheritanceManager.lookupOverrides(
           methodElement.enclosingElement, methodElement.name);
       if (overriddenMethods.isEmpty || !_onlyMethods(overriddenMethods)) {
         return;
       }
       MethodElementImpl element = methodElement as MethodElementImpl;
-      _setReturnType(element, _computeReturnType(overriddenMethods));
+      setReturnType(element, _computeReturnType(overriddenMethods));
     }
     //
     // Infer the parameter types.
     //
     List<ParameterElement> parameters = methodElement.parameters;
     var length = parameters.length;
     for (int i = 0; i < length; ++i) {
       ParameterElement parameter = parameters[i];
       if (parameter is ParameterElementImpl && parameter.hasImplicitType) {
         overriddenMethods = overriddenMethods ??
@@ skipping 50 matching lines @@
    * Return `true` if the list of [elements] contains only methods.
    */
   bool _onlyMethods(List<ExecutableElement> elements) {
     for (ExecutableElement element in elements) {
       if (element is! MethodElement) {
         return false;
       }
     }
     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);
-      }
-    }
-  }
 }
 
 /**
  * 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
@@ skipping 23 matching lines @@
         results.add(element);
       }
     }
   }
 }
 
 /**
  * A class of exception that is not used anywhere else.
  */
 class _CycleException implements Exception {}