Emit compile-time error on duplicate parameter names in typedefs.

sdk/lib/_internal/compiler/implementation/resolution/members.dart

 // Copyright (c) 2012, 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.
 
 part of resolution;
 
 abstract class TreeElements {
   Element get currentElement;
   Set<Node> get superUses;
 
@@ skipping 1705 matching lines @@
     return mapping[node] = element;
   }
 
   DartType useType(TypeAnnotation annotation, DartType type) {
     if (type != null) {
       mapping.setType(annotation, type);
       useElement(annotation, type.element);
     }
     return type;
   }
+
+  Element defineElement(Node node, Element element,
+                        {bool doAddToScope: true}) {
+    compiler.ensure(element != null);
+    mapping[node] = element;
+    if (doAddToScope) {
+      Element existing = scope.add(element);
+      if (existing != element) {
+        reportDuplicateDefinition(node, element, existing);
+      }
+    }
+    return element;
+  }
+
+  void reportDuplicateDefinition(name, Spannable definition,

[ngeoffray, 2013/09/16 16:12:49]

Could you add (/* SourceString|Node */ name, ...) ?

+                                 Spannable existing) {
+    compiler.reportError(
+        definition,
+        MessageKind.DUPLICATE_DEFINITION, {'name': name});
+    compiler.reportMessage(
+        compiler.spanFromSpannable(existing),
+        MessageKind.EXISTING_DEFINITION.error({'name': name}),
+        Diagnostic.INFO);
+  }
 }
 
 /**
  * Core implementation of resolution.
  *
  * Do not subclass or instantiate this class outside this library
  * except for testing.
  */
 class ResolverVisitor extends MappingVisitor<Element> {
   /**
@@ skipping 172 matching lines @@
     DartType type = resolveTypeAnnotation(node);
     if (type != null) {
       if (inCheckContext) {
         compiler.enqueuer.resolution.registerIsCheck(type, mapping);
       }
       return type.element;
     }
     return null;
   }
 
-  Element defineElement(Node node, Element element,
-                        {bool doAddToScope: true}) {
-    compiler.ensure(element != null);
-    mapping[node] = element;
-    if (doAddToScope) {
-      Element existing = scope.add(element);
-      if (existing != element) {
-        compiler.reportError(
-            node, MessageKind.DUPLICATE_DEFINITION, {'name': node});
-        compiler.reportMessage(
-            compiler.spanFromSpannable(existing),
-            MessageKind.EXISTING_DEFINITION.error({'name': node}),
-            Diagnostic.INFO);
-      }
-    }
-    return element;
-  }
-
   bool isNamedConstructor(Send node) => node.receiver != null;
 
   Selector getRedirectingThisOrSuperConstructorSelector(Send node) {
     if (isNamedConstructor(node)) {
       SourceString constructorName = node.selector.asIdentifier().source;
       return new Selector.callConstructor(
           constructorName,
           enclosingElement.getLibrary());
     } else {
       return new Selector.callDefaultConstructor(
@@ skipping 356 matching lines @@
 
   void resolveArguments(NodeList list) {
     if (list == null) return;
     Map<SourceString, Node> seenNamedArguments = new Map<SourceString, Node>();
     for (Link<Node> link = list.nodes; !link.isEmpty; link = link.tail) {
       Expression argument = link.head;
       visit(argument);
       NamedArgument namedArgument = argument.asNamedArgument();
       if (namedArgument != null) {
         SourceString source = namedArgument.name.source;
         if (seenNamedArguments.containsKey(source)) {

[ngeoffray, 2013/09/16 16:12:49]

Do you still need this check? It seems that the defineElement should do it for
you.

-          compiler.reportError(
+          reportDuplicateDefinition(
+              source,
               argument,
-              MessageKind.DUPLICATE_DEFINITION,
-              {'name': source});
-          compiler.reportMessage(
-              compiler.spanFromSpannable(seenNamedArguments[source]),
-              MessageKind.EXISTING_DEFINITION.error({'name': source}),
-              Diagnostic.INFO);
+              seenNamedArguments[source]);
         } else {
           seenNamedArguments[source] = namedArgument;
         }
       } else if (!seenNamedArguments.isEmpty) {
         error(argument, MessageKind.INVALID_ARGUMENT_AFTER_NAMED);
       }
     }
   }
 
   visitSend(Send node) {
@@ skipping 995 matching lines @@
   TypedefResolverVisitor(Compiler compiler,
                          TypedefElement typedefElement,
                          TreeElementMapping mapping)
       : super(compiler, typedefElement, mapping);
 
   visitTypedef(Typedef node) {
     TypedefType type = element.computeType(compiler);
     scope = new TypeDeclarationScope(scope, element);
     resolveTypeVariableBounds(node.typeParameters);
 
-    element.functionSignature = SignatureResolver.analyze(
+    FunctionSignature signature = SignatureResolver.analyze(

[ngeoffray, 2013/09/16 16:25:27]

I would have thought that SignatureResolver would detect the duplicates.

         compiler, node.formals, node.returnType, element,
         defaultValuesAllowed: false);
 
-    element.alias = compiler.computeFunctionType(
-        element, element.functionSignature);
+    scope = new MethodScope(scope, element);
+    signature.forEachParameter((Element element) {
+      defineElement(element.parseNode(compiler), element);
+    });
+
+    element.alias = compiler.computeFunctionType(element, signature);
 
     // TODO(johnniwinther): Check for cyclic references in the typedef alias.
   }
 }
 
 /**
  * The implementation of [ResolverTask.resolveClass].
  *
  * This visitor has to be extra careful as it is building the basic
  * element information, and cannot safely look at other elements as
@@ skipping 871 matching lines @@
     return e;
   }
 
   /// Assumed to be called by [resolveRedirectingFactory].
   Element visitReturn(Return node) {
     Node expression = node.expression;
     return finishConstructorReference(visit(expression),
                                       expression, expression);
   }
 }