Fixed tests so they do indeed confirm that values on optional named parameters are not allowed. Add…

Fixed tests so they do indeed confirm that values on optional named parameters are not allowed. Added check to the dart2js parser, so that setting values on optional named parameters is flagged as error. Tests pass.

BUG=dart:351
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=11573

[ahe, 2012-08-21 21:20:39]

Hi Alexander,

I hope you're still interested in this. I suggest that you split this CL in two:
one for the language tests, and one for the compiler changes.

Regarding the compiler changes, we try to accept as much as possible in the
parser and detect errors later (during resolution). This is so that we can
create a semantic model of illegal programs which is generally useful if you try
to build tools on top of the compiler's infrastructure.

For inspiration on how to implement these check in the resolver, take a look at
this change: https://code.google.com/p/dart/source/detail?r=10956, in particular
the changes to dart/lib/compiler/implementation/resolver.dart.

Cheers,
Peter

https://chromiumcodereview.appspot.com/10800027/diff/1/tests/language/named_p...
File tests/language/named_parameters6_negative_test.dart (right):

https://chromiumcodereview.appspot.com/10800027/diff/1/tests/language/named_p...
tests/language/named_parameters6_negative_test.dart:12: Callback cb1 = null;
I think the only way to force dart2js to compile Callback is to write something
like this:

Expect.isTrue(null is Callback);

[aam-me, 2012-08-23 01:40:01]

Thanks, Peter.
I removed compiler changes - will create separate CL for that.
So this CL is just for tests changes as you suggested.
I aggregated five negative tests into one multi-test.

> tests/language/named_parameters6_negative_test.dart:12: Callback cb1 = null;
> I think the only way to force dart2js to compile Callback is to write
something
> like this:
> Expect.isTrue(null is Callback);

  If I do this I get "Internal error: visitIs for typedefs not implemented". So
I left "Callback cb = null;" since it seems to work. Let me know if you have
further thoughts on this.

[ahe, 2012-08-23 11:39:25]

LGTM

https://chromiumcodereview.appspot.com/10800027/diff/4001/tests/language/lang...
File tests/language/language_dart2js.status (right):

https://chromiumcodereview.appspot.com/10800027/diff/4001/tests/language/lang...
tests/language/language_dart2js.status:100: named_parameters_aggregated_test/01:
Fail # not handling def-vals for optional params
Please avoid abbreviations. It actually took me a few seconds to figure out what
you meant :-)

Also, comment should be a proper sentence (start with an uppercase letter and
end with a period or such).

https://chromiumcodereview.appspot.com/10800027/diff/4001/tests/language/name...
File tests/language/named_parameters_aggregated_test.dart (right):

https://chromiumcodereview.appspot.com/10800027/diff/4001/tests/language/name...
tests/language/named_parameters_aggregated_test.dart:48: Callback cb1 = null;
I discussed this with Kasper, and we don't want to guarantee resolution of
Callback though this mechanism.

Kasper had a better alternative:

class TypeTester<T> {}

...
new TypeTester<Callback>();

If dart2js cannot handle this, just mark it as failing in
language_dart2js.status.

https://chromiumcodereview.appspot.com/10800027/diff/4001/tests/language/name...
tests/language/named_parameters_aggregated_test.dart:57:
NamedParametersAggregatedTests.testMain();
This is a left-over from when we didn't have top-level declarations. Now that we
do, I would inline testMain in main.

[aam-me, 2012-08-23 22:01:10]

Thank you, Peter. All taken care of.

On 2012/08/23 11:39:25, ahe wrote:
>
https://chromiumcodereview.appspot.com/10800027/diff/4001/tests/language/lang...
> tests/language/language_dart2js.status:100:
named_parameters_aggregated_test/01:
> Fail # not handling def-vals for optional params
> Please avoid abbreviations. It actually took me a few seconds to figure out
what
> you meant :-)

I tried to squeeze description in 80 columns. Hopefully getting over 80 columns
is not a big deal for this file. Just let me know.

[ahe, 2012-08-24 03:54:41]

Still LGTM!

Lines longer than 80 columns are allowed in status files.

[aam-me, 2012-08-26 03:48:52]

Peter, please, review. 
I replace failed deleted test with new one in language.status for dartc build to
succeed.

[aam-me, 2012-08-27 03:16:53]

Peter, I have moved diagnostic for default values for optional named parameters
to resolver: https://chromiumcodereview.appspot.com/10874078. Please, take a
look when you have a chance.

[ahe, 2012-08-29 05:46:16]

LGTM!