Permalink
Browse files

frog: improve errors with named param (bug 507)

BUG= 507
TEST=

Review URL: http://codereview.chromium.org//8878001

git-svn-id: http://dart.googlecode.com/svn/branches/bleeding_edge/dart@2212 260f80e4-7a28-3924-810f-c04153c831b5
  • Loading branch information...
1 parent 5737fd1 commit 7c88d774b4316a547b6fccf9db52a5357cbf852f sigmund@google.com committed Dec 8, 2011
Showing with 80 additions and 57 deletions.
  1. +32 −32 frog/member.dart
  2. +25 −25 frog/minfrog
  3. +1 −0 tests/language/language.status
  4. +22 −0 tests/language/src/NamedParameters10NegativeTest.dart
View
@@ -838,17 +838,16 @@ class MethodMember extends Member {
}
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);
}
@@ -897,9 +896,8 @@ class MethodMember extends Member {
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) {
@@ -909,10 +907,10 @@ class MethodMember extends Member {
}
}
+ 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) {
@@ -923,39 +921,41 @@ class MethodMember extends Member {
}
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);
}
}
+ Arguments.removeTrailingNulls(argsCode);
+ }
- 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');
- }
+ 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);
}
-
- Arguments.removeTrailingNulls(argsCode);
+ world.internalError('wrong named arguments calling $name', node.span);
}
var argsString = Strings.join(argsCode, ', ');
View
@@ -5357,14 +5357,14 @@ MethodMember.prototype.needsArgumentConversion = function(args) {
return false;
}
MethodMember._argCountMsg = function(actual, expected, atLeast) {
- return 'wrong number of arguments, expected ' + ('' + (atLeast ? "at least " : "") + expected + ' but found ' + actual);
+ return 'wrong number of positional arguments, expected ' + ('' + (atLeast ? "at least " : "") + expected + ' but found ' + actual);
}
-MethodMember.prototype._argError = function(context, node, target, args, msg) {
+MethodMember.prototype._argError = function(context, node, target, args, msg, span) {
if (this.isStatic || this.get$isConstructor()) {
- $globals.world.error(msg, node.span);
+ $globals.world.error(msg, span);
}
else {
- $globals.world.warning(msg, node.span);
+ $globals.world.warning(msg, span);
}
return target.invokeNoSuchMethod(context, this.name, node, args);
}
@@ -5398,7 +5398,7 @@ MethodMember.prototype.invoke = function(context, node, target, args, isDynamic)
var arg = args.values.$index(i);
if (i >= this.parameters.length) {
var msg = MethodMember._argCountMsg(args.get$length(), this.parameters.length, false);
- return this._argError(context, node, target, args, msg);
+ return this._argError(context, node, target, args, msg, args.nodes.$index(i).get$span());
}
arg = arg.convertTo$4(context, this.parameters.$index(i).get$type(), node, isDynamic);
if (this.isConst && arg.get$isConst()) {
@@ -5408,9 +5408,9 @@ MethodMember.prototype.invoke = function(context, node, target, args, isDynamic)
argsCode.add$1(arg.get$code());
}
}
+ var namedArgsUsed = 0;
if (bareCount < this.parameters.length) {
this.genParameterValues();
- var namedArgsUsed = 0;
for (var i = bareCount;
i < this.parameters.length; i++) {
var arg = args.getValue(this.parameters.$index(i).get$name());
@@ -5423,32 +5423,32 @@ MethodMember.prototype.invoke = function(context, node, target, args, isDynamic)
}
if (arg == null || !this.parameters.$index(i).get$isOptional()) {
var msg = MethodMember._argCountMsg(Math.min(i, args.get$length()), i + 1, true);
- return this._argError(context, node, target, args, msg);
+ return this._argError(context, node, target, args, msg, args.nodes.$index(i).get$span());
}
else {
argsCode.add$1(this.isConst && arg.get$isConst() ? arg.get$canonicalCode() : arg.get$code());
}
}
- if (namedArgsUsed < args.get$nameCount()) {
- var seen = new HashSetImplementation();
- for (var i = bareCount;
- i < args.get$length(); i++) {
- var name = args.getName(i);
- if (seen.contains$1(name)) {
- return this._argError(context, node, target, args, ('duplicate argument "' + name + '"'));
- }
- seen.add$1(name);
- var p = this.indexOfParameter(name);
- if (p < 0) {
- return this._argError(context, node, target, args, ('method does not have optional parameter "' + name + '"'));
- }
- else if (p < bareCount) {
- return this._argError(context, node, target, args, ('argument "' + name + '" passed as positional and named'));
- }
+ Arguments.removeTrailingNulls(argsCode);
+ }
+ if (namedArgsUsed < args.get$nameCount()) {
+ var seen = new HashSetImplementation();
+ for (var i = bareCount;
+ i < args.get$length(); i++) {
+ var name = args.getName(i);
+ if (seen.contains$1(name)) {
+ return this._argError(context, node, target, args, ('duplicate argument "' + name + '"'), args.nodes.$index(i).get$span());
+ }
+ seen.add$1(name);
+ var p = this.indexOfParameter(name);
+ if (p < 0) {
+ return this._argError(context, node, target, args, ('method does not have optional parameter "' + name + '"'), args.nodes.$index(i).get$span());
+ }
+ else if (p < bareCount) {
+ return this._argError(context, node, target, args, ('argument "' + name + '" passed as positional and named'), args.nodes.$index(p).get$span());
}
- $globals.world.internalError(('wrong named arguments calling ' + this.name), node.span);
}
- Arguments.removeTrailingNulls(argsCode);
+ $globals.world.internalError(('wrong named arguments calling ' + this.name), node.span);
}
var argsString = Strings.join(argsCode, ', ');
if (this.get$isConstructor()) {
@@ -125,6 +125,7 @@ MapLiteral3Test: Fail # Issue 221
MapLiteral4Test: Fail # Issue 221
ManyOverriddenNoSuchMethodTest: Fail # Bug 4202974.
MethodOverrideTest: Crash # Bug 5516001.
+NamedParameters10NegativeTest: Fail # Implementation in progress.
NamedParameters2NegativeTest: Fail # Implementation in progress.
NamedParameters3NegativeTest: Fail # Implementation in progress.
NamedParameters4NegativeTest: Fail # Implementation in progress.
@@ -0,0 +1,22 @@
+// 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.
+// Dart test program for testing named parameters.
+
+// This test is very similar to NamedParameters3NegativeTest, but exersizes a
+// different corner case in the frog compiler. frog wasn't detecting unused
+// named arguments when no other arguments were expected. So, this test
+// purposely passes the exact number of positional parameters
+
+int test(int a) {
+ return a;
+}
+
+main() {
+ try {
+ test(10, x:99); // 1 positional arg, as expected. Param x does not exist.
+ } catch (var e) {
+ // This is a negative test that should not compile.
+ // If it runs due to a bug, catch and ignore exceptions.
+ }
+}

0 comments on commit 7c88d77

Please sign in to comment.