Resynthesize field formal parameters from Kernel.

pkg/analyzer/lib/src/dart/element/element.dart

 // Copyright (c) 2014, 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.dart.element.element;
 
 import 'dart:collection';
 import 'dart:math' show min;
 
 import 'package:analyzer/dart/ast/ast.dart';
@@ skipping 1891 matching lines @@
  * class.
  */
 class ConstFieldElementImpl extends FieldElementImpl with ConstVariableElement {
   /**
    * Initialize a newly created synthetic field element to have the given
    * [name] and [offset].
    */
   ConstFieldElementImpl(String name, int offset) : super(name, offset);
 
   /**
+   * Initialize using the given kernel.
+   */
+  ConstFieldElementImpl.forKernel(
+      ElementImpl enclosingElement, kernel.Field kernel)
+      : super.forKernel(enclosingElement, kernel);
+
+  /**
    * Initialize a newly created field element to have the given [name].
    */
   ConstFieldElementImpl.forNode(Identifier name) : super.forNode(name);
 
   /**
    * Initialize using the given serialized information.
    */
   ConstFieldElementImpl.forSerialized(
       UnlinkedVariable unlinkedVariable, ElementImpl enclosingElement)
       : super.forSerialized(unlinkedVariable, enclosingElement);
-
-  /**
-   * Initialize using the given kernel.
-   */
-  ConstFieldElementImpl.forKernel(
-      ElementImpl enclosingElement, kernel.Field kernel)
-      : super.forKernel(enclosingElement, kernel);
 }
 
 /**
  * A field element representing an enum constant.
  */
 class ConstFieldElementImpl_EnumValue extends ConstFieldElementImpl_ofEnum {
   final UnlinkedEnumValue _unlinkedEnumValue;
   final int _index;
 
   ConstFieldElementImpl_EnumValue(
@@ skipping 605 matching lines @@
 class DefaultFieldFormalParameterElementImpl
     extends FieldFormalParameterElementImpl with ConstVariableElement {
   /**
    * Initialize a newly created parameter element to have the given [name] and
    * [nameOffset].
    */
   DefaultFieldFormalParameterElementImpl(String name, int nameOffset)
       : super(name, nameOffset);
 
   /**
+   * Initialize using the given kernel.
+   */
+  DefaultFieldFormalParameterElementImpl.forKernel(ElementImpl enclosingElement,
+      kernel.VariableDeclaration kernel, ParameterKind parameterKind)
+      : super.forKernel(enclosingElement, kernel, parameterKind);
+
+  /**
    * Initialize a newly created parameter element to have the given [name].
    */
   DefaultFieldFormalParameterElementImpl.forNode(Identifier name)
       : super.forNode(name);
 
   /**
    * Initialize using the given serialized information.
    */
   DefaultFieldFormalParameterElementImpl.forSerialized(
       UnlinkedParam unlinkedParam, ElementImpl enclosingElement)
       : super.forSerialized(unlinkedParam, enclosingElement);
 }
 
 /**
  * A [ParameterElement] for parameters that have an initializer.
  */
 class DefaultParameterElementImpl extends ParameterElementImpl
     with ConstVariableElement {
   /**
    * Initialize a newly created parameter element to have the given [name] and
    * [nameOffset].
    */
   DefaultParameterElementImpl(String name, int nameOffset)
       : super(name, nameOffset);
 
   /**
+   * Initialize using the given kernel.
+   */
+  DefaultParameterElementImpl.forKernel(ElementImpl enclosingElement,
+      kernel.VariableDeclaration kernel, ParameterKind parameterKind)
+      : super.forKernel(enclosingElement, kernel, parameterKind);
+
+  /**
    * Initialize a newly created parameter element to have the given [name].
    */
   DefaultParameterElementImpl.forNode(Identifier name) : super.forNode(name);
 
   /**
    * Initialize using the given serialized information.
    */
   DefaultParameterElementImpl.forSerialized(
       UnlinkedParam unlinkedParam, ElementImpl enclosingElement)
       : super.forSerialized(unlinkedParam, enclosingElement);
@@ skipping 1844 matching lines @@
   FieldElement _field;
 
   /**
    * Initialize a newly created parameter element to have the given [name] and
    * [nameOffset].
    */
   FieldFormalParameterElementImpl(String name, int nameOffset)
       : super(name, nameOffset);
 
   /**
+   * Initialize using the given kernel.
+   */
+  FieldFormalParameterElementImpl.forKernel(ElementImpl enclosingElement,
+      kernel.VariableDeclaration kernel, ParameterKind parameterKind)
+      : super.forKernel(enclosingElement, kernel, parameterKind);
+
+  /**
    * Initialize a newly created parameter element to have the given [name].
    */
   FieldFormalParameterElementImpl.forNode(Identifier name)
       : super.forNode(name);
 
   /**
    * Initialize using the given serialized information.
    */
   FieldFormalParameterElementImpl.forSerialized(
       UnlinkedParam unlinkedParam, ElementImpl enclosingElement)
@@ skipping 3476 matching lines @@
     }
   }
 
   /**
    * Create and return [ParameterElement]s for the given [function].
    */
   static List<ParameterElement> forKernelFunction(
       ElementImpl enclosingElement, kernel.FunctionNode function) {
     if (function.positionalParameters.isNotEmpty ||
         function.namedParameters.isNotEmpty) {
+      // Prepare all field formal parameters if a constructor.
+      var fieldFormalParameters = <kernel.VariableDeclaration>[];
+      {
+        var procedure = function.parent;
+        if (procedure is kernel.Constructor) {
+          for (kernel.Initializer initializer in procedure.initializers) {
+            if (initializer is kernel.FieldInitializer) {
+              kernel.Expression value = initializer.value;
+              if (value is kernel.VariableGet) {
+                fieldFormalParameters.add(value.variable);

[Paul Berry, 2017/07/19 22:11:59]

It looks like what you're doing is: if you see `C(x) : this.x = x;` you assume
the user actually said `C(this.x);` and it got desugared to `C(x) : this.x = x`,
so you create a FieldFormalParameterElementImpl in the element model.

But what if the user really did say `C(x) : this.x = x` in the first place? 
Then the element model will be incorrect.

Another problem: for a non-const constructor I don't think we'll be storing the
constructor initializer expressions in the outline.  Which means that for
non-const constructors this approach will eventually stop working, once we start
using kernel summaries that have method bodies and constructor initializer
expressions stripped out.

In the end it shouldn't really matter, since the two ways of writing the
constructor are equivalent from an API perspective.  Probably most clients won't
care about the difference, and the few clients that do really ought to be using
the AST to examine the constructor rather than the element model.

But the people writing client code probably don't know that--they'll just do
whatever is easiest and appears to work.  Which means that if we try to guess
whether a parameter is a field formal parameter, and we don't always do it
reliably, people are likely to write bad client code without realizing it, since
their simple test cases may pass anyhow.  That seems like a dangerous situation.

I think that when resynthesizing from kernel, we should always resynthesize the
element model as though the user's code was `C(x) : this.x = x`.  If this causes
clients to break, at least it will break them consistently so they'll probably
notice it in testing, rather than having bizarre bugs in the field that are hard
to track down.  And if they complain to us that we've taken away functionality
they need, we can always explain why we had to remove it, and tell them that for
info like this they need to look at the constructor's AST.

+              }
+            }
+          }
+        }
+      }
+
       var parameters = <ParameterElement>[];
+
+      // Add positional required and optional parameters.
       for (int i = 0; i < function.positionalParameters.length; i++) {
-        parameters.add(new ParameterElementImpl.forKernel(
-            enclosingElement,
-            function.positionalParameters[i],
-            i < function.requiredParameterCount
-                ? ParameterKind.REQUIRED
-                : ParameterKind.POSITIONAL));
+        kernel.VariableDeclaration parameter = function.positionalParameters[i];
+        bool isFieldFormal = fieldFormalParameters.contains(parameter);
+        if (i < function.requiredParameterCount) {
+          if (isFieldFormal) {
+            parameters.add(new FieldFormalParameterElementImpl.forKernel(
+                enclosingElement, parameter, ParameterKind.REQUIRED));
+          } else {
+            parameters.add(new ParameterElementImpl.forKernel(
+                enclosingElement, parameter, ParameterKind.REQUIRED));
+          }
+        } else {
+          if (isFieldFormal) {
+            parameters.add(new DefaultFieldFormalParameterElementImpl.forKernel(
+                enclosingElement, parameter, ParameterKind.POSITIONAL));
+          } else {
+            parameters.add(new DefaultParameterElementImpl.forKernel(
+                enclosingElement, parameter, ParameterKind.POSITIONAL));
+          }
+        }
       }
+
+      // Add named parameters.
       for (var k in function.namedParameters) {
-        parameters.add(new ParameterElementImpl.forKernel(
-            enclosingElement, k, ParameterKind.NAMED));
+        bool isFieldFormal = fieldFormalParameters.contains(k);
+        if (isFieldFormal) {
+          parameters.add(new DefaultFieldFormalParameterElementImpl.forKernel(
+              enclosingElement, k, ParameterKind.NAMED));
+        } else {
+          parameters.add(new DefaultParameterElementImpl.forKernel(
+              enclosingElement, k, ParameterKind.NAMED));
+        }
       }
+
       return parameters;
     } else {
       return const <ParameterElement>[];
     }
   }
 
   /**
    * Create and return [ParameterElement]s for the given [unlinkedParameters].
    */
   static List<ParameterElement> resynthesizeList(
@@ skipping 1283 matching lines @@
 
   @override
   DartObject computeConstantValue() => null;
 
   @override
   void visitChildren(ElementVisitor visitor) {
     super.visitChildren(visitor);
     _initializer?.accept(visitor);
   }
 }