frog: improve errors with named param (bug 507)

frog/member.dart

 // Copyright (c) 2011, 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.
 
 /** A formal parameter to a [Method]. */
 class Parameter {
   FormalNode definition;
   Member method;
 
   String name;
@@ skipping 820 matching lines @@
         if (arg != null && arg.needsConversion(parameters[i].type)) {
           return true;
         }
       }
     }
 
     return false;
   }
 
   static String _argCountMsg(int actual, int expected, [bool atLeast=false]) {
-    // TODO(jimhug): better messages with default named args.
-    return 'wrong number of arguments, expected ' +
+    return 'wrong number of positional arguments, expected ' +
         '${atLeast ? "at least " : ""}$expected but found $actual';
   }
 
   Value _argError(MethodGenerator context, Node node, Value target,
-      Arguments args, String msg) {
+      Arguments args, String msg, SourceSpan span) {
     if (isStatic || isConstructor) {
-      world.error(msg, node.span);
+      world.error(msg, span);
     } else {
-      world.warning(msg, node.span);
+      world.warning(msg, span);
     }
     return target.invokeNoSuchMethod(context, name, node, args);
   }
 
   genParameterValues() {
     // Pure lazy?
     for (var p in parameters) p.genValue(this, generator);
   }
 
   /**
@@ skipping 28 matching lines @@
 
     var argsCode = [];
     if (!target.isType && (isConstructor || target.isSuper)) {
       argsCode.add('this');
     }
 
     int bareCount = args.bareCount;
     for (int i = 0; i < bareCount; i++) {
       var arg = args.values[i];
       if (i >= parameters.length) {
-        // TODO(jimhug): better error location
         var msg = _argCountMsg(args.length, parameters.length);
-        return _argError(context, node, target, args, msg);
+        return _argError(context, node, target, args, msg, args.nodes[i].span);
       }
       arg = arg.convertTo(context, parameters[i].type, node, isDynamic);
       if (isConst && arg.isConst) {
         argsCode.add(arg.canonicalCode);
       } else {
         argsCode.add(arg.code);
       }
     }
 
+    int namedArgsUsed = 0;
     if (bareCount < parameters.length) {
       genParameterValues();
 
-      int namedArgsUsed = 0;
       for (int i = bareCount; i < parameters.length; i++) {
         var arg = args.getValue(parameters[i].name);
         if (arg == null) {
           arg = parameters[i].value;
         } else {
           arg = arg.convertTo(context, parameters[i].type, node, isDynamic);
           namedArgsUsed++;
         }
 
         if (arg == null || !parameters[i].isOptional) {
-          // TODO(jimhug): better error location
           var msg = _argCountMsg(Math.min(i, args.length), i + 1, atLeast:true);
-          return _argError(context, node, target, args, msg);
+          return _argError(context, node, target, args, msg,
+              args.nodes[i].span);
         } else {
           argsCode.add(isConst && arg.isConst
               ? arg.canonicalCode : arg.code);
         }
       }
-
-      if (namedArgsUsed < args.nameCount) {
-        // TODO(jmesserly): better error location
-        // Find the unused argument name
-        var seen = new Set<String>();
-        for (int i = bareCount; i < args.length; i++) {
-          var name = args.getName(i);
-          if (seen.contains(name)) {
-            return _argError(context, node, target, args,
-                'duplicate argument "$name"');
-          }
-          seen.add(name);
-          int p = indexOfParameter(name);
-          if (p < 0) {
-            return _argError(context, node, target, args,
-                'method does not have optional parameter "$name"');
-          } else if (p < bareCount) {
-            return _argError(context, node, target, args,
-                'argument "$name" passed as positional and named');
-          }
-        }
-        world.internalError('wrong named arguments calling $name', node.span);
-      }
-
       Arguments.removeTrailingNulls(argsCode);
     }
 
+    if (namedArgsUsed < args.nameCount) {
+      // Find the unused argument name
+      var seen = new Set<String>();
+      for (int i = bareCount; i < args.length; i++) {
+        var name = args.getName(i);
+        if (seen.contains(name)) {
+          return _argError(context, node, target, args,
+              'duplicate argument "$name"', args.nodes[i].span);
+        }
+        seen.add(name);
+        int p = indexOfParameter(name);
+        if (p < 0) {
+          return _argError(context, node, target, args,
+              'method does not have optional parameter "$name"',
+              args.nodes[i].span);
+        } else if (p < bareCount) {
+          return _argError(context, node, target, args,
+              'argument "$name" passed as positional and named',
+              // Given that the named was mentioned explicitly, highlight the
+              // positional location instead:
+              args.nodes[p].span);
+        }
+      }
+      world.internalError('wrong named arguments calling $name', node.span);
+    }
+
     var argsString = Strings.join(argsCode, ', ');
 
     if (isConstructor) {
       return _invokeConstructor(context, node, target, args, argsString);
     }
 
     if (target.isSuper) {
       return new Value(inferredResult,
           '${declaringType.jsname}.prototype.$jsname.call($argsString)',
           node.span);
@@ skipping 776 matching lines @@
   }
 
   void forEach(void f(Member member)) {
     factories.forEach((_, Map constructors) {
       constructors.forEach((_, Member member) {
         f(member);
       });
     });
   }
 }