Produce error when duplicate field initializers are found

lib/compiler/implementation/resolver.dart

Index: lib/compiler/implementation/resolver.dart
diff --git a/lib/compiler/implementation/resolver.dart b/lib/compiler/implementation/resolver.dart
index aff2ad7514ac706518559708544f85e8f670c237..ce58e6a6476182d7efe33df38ac6ec72da121975 100644
--- a/lib/compiler/implementation/resolver.dart
+++ b/lib/compiler/implementation/resolver.dart
@@ -429,6 +429,14 @@ class InitializerResolver {
     return node.receiver.asIdentifier().isThis();
   }
 
+  void checkForDuplicateInitializers(SourceString name, Node init) {

[ahe, 2012/08/16 11:56:43]

I hoped you would do that :-)

+    if (initialized.containsKey(name)) {
+      error(init, MessageKind.DUPLICATE_INITIALIZER, [name]);
+      warning(initialized[name], MessageKind.ALREADY_INITIALIZED, [name]);
+    }
+    initialized[name] = init;
+  }
+
   void resolveFieldInitializer(FunctionElement constructor, SendSet init) {
     // init is of the form [this.]field = value.
     final Node selector = init.selector;
@@ -449,12 +457,7 @@ class InitializerResolver {
       error(init, MessageKind.INVALID_RECEIVER_IN_INITIALIZER);
     }
     visitor.useElement(init, target);
-    // Check for duplicate initializers.
-    if (initialized.containsKey(name)) {
-      error(init, MessageKind.DUPLICATE_INITIALIZER, [name]);
-      warning(initialized[name], MessageKind.ALREADY_INITIALIZED, [name]);
-    }
-    initialized[name] = init;
+    checkForDuplicateInitializers(name, init);
     // Resolve initializing value.
     visitor.visitInStaticContext(init.arguments.head);
   }
@@ -552,6 +555,19 @@ class InitializerResolver {
    */
   FunctionElement resolveInitializers(FunctionElement constructor,
                                       FunctionExpression functionNode) {
+     // Keep track of all "this.param" parameters specified for constructor so
+     // that we can ensure that fields are initialized only once.
+     FunctionSignature functionParameters =
+         constructor.computeSignature(visitor.compiler);
+     if (functionParameters != null) {

[ahe, 2012/08/16 11:56:43]

Please use !== here.

[ahe, 2012/08/16 11:56:43]

Why do you have to test for null?

[aam-me, 2012/08/16 12:28:58]

"Defensive" programming, I guess :-)
But I see that computeSignature is used without such checks in other places, so
no good reason to have it here neither.

[ahe, 2012/08/16 18:23:27]

Interesting. I think of defensive programming as writing code that I know will
fail early if I made a mistake. So I try to avoid testing for null when I'm
fairly confident that a null is an error.

On a related note: long term, I think we would like that methods that detect an
error when trying to produce a type, element, etc., return an erroneous type,
element, etc.

+       functionParameters.forEachParameter((Element element) {
+         if (element.kind == ElementKind.FIELD_PARAMETER) {

[ahe, 2012/08/16 11:56:43]

Please use === here.

+           checkForDuplicateInitializers(element.name,
+                                         element.parseNode(visitor.compiler));
+         }
+       });
+     }
+
     if (functionNode.initializers === null) {
       initializers = const EmptyLink<Node>();
     } else {