Switch dart2js 'typedef name = mixinApplication' to 'class name = mixinApplication'.

Switch dart2js 'typedef name = mixinApplication' to 'class name = mixinApplication'.

BUG=
R=johnniwinther@google.com

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

[aam-me, 2013-10-10 03:52:39]

Hi,

  saw dartbug.com/13899 and then noticed change in the spec regarding mixin
application definition and decided to give it a go with corresponding change in
dart2js.

Hope this helps!

[kasperl, 2013-10-10 06:47:12]

Adding Johnni as a reviewer.

[Johnni Winther, 2013-10-10 07:29:56]

Nice work. Can you support abstract named mixin applications as well? For
instance:

abstract class C = S with M1, M2;

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/scanner/parser.dart (right):

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:275: 
Extra line.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:466: token =
parseModifiers(token);
Modifiers should preceed the 'class' keyword:

abstract class C = S with M1, M2;

[ahe, 2013-10-10 11:30:14]

I agree with Johnni, very nice but I guess the modifiers issue should be
addressed before we can approve this CL.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3690:
compiler.reportWarning(node.classKeyword,
Add TODO that this should be a compile-time error eventually?

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3691:
MessageKind.DEPRECATED_TYPEDEF_MIXIN_SYNTAX);
Weird indentation.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/warnings.dart (right):

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/warnings.dart:1036: "was deprecated,
replaced with 'class'.",
I think this would be enough: "Warning: 'typedef' not allowed here."

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/warnings.dart:1037: howToFix: "Try
replacing 'typedef' with 'class'.");
Can you add an example as well?

[aam-me, 2013-10-11 03:02:06]

Johnni, Peter, I appreciate your comments very much.

All taken care of I believe.

PTAL.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3690:
compiler.reportWarning(node.classKeyword,
On 2013/10/10 11:30:15, ahe wrote:
> Add TODO that this should be a compile-time error eventually?

Done.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3691:
MessageKind.DEPRECATED_TYPEDEF_MIXIN_SYNTAX);
On 2013/10/10 11:30:15, ahe wrote:
> Weird indentation.

Done.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/scanner/parser.dart (right):

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:275: 
On 2013/10/10 07:29:56, Johnni Winther wrote:
> Extra line.

Done.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/scanner/parser.dart:466: token =
parseModifiers(token);
On 2013/10/10 07:29:56, Johnni Winther wrote:
> Modifiers should preceed the 'class' keyword:
> 
> abstract class C = S with M1, M2;

Done.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/warnings.dart (right):

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/warnings.dart:1036: "was deprecated,
replaced with 'class'.",
On 2013/10/10 11:30:15, ahe wrote:
> I think this would be enough: "Warning: 'typedef' not allowed here."

Done.

https://codereview.chromium.org/26651006/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/warnings.dart:1037: howToFix: "Try
replacing 'typedef' with 'class'.");
On 2013/10/10 11:30:15, ahe wrote:
> Can you add an example as well?

Done.

Very cool message_kind_test.dart test. But I spent more time on getting the
example pass through that test than I expected because  in
compiler\implementation\resolution\members.dart:3693 I did not have
".message()" method invocation on MessageKind.DEPRECATED_TYPEDEF_MIXIN_SYNTAX.
MessageKind.toString() method returns just metadata without howToFix part. That
resulted in missing howToFix part in diagnostic message. Is that as intended? I
see there are a lot of diagnostic in members.dart, that doesn't use message()
method on MessageKind instances neither.

[Johnni Winther, 2013-10-11 08:01:27]

LGTM!

[ahe, 2013-10-11 08:32:26]

Thank you, Alexander!

https://codereview.chromium.org/26651006/diff/10001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/26651006/diff/10001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3693:
MessageKind.DEPRECATED_TYPEDEF_MIXIN_SYNTAX.message());
You should use compiler.reportWarningCode instead, and that is probably why you
had such a hard time getting the test to work.

We haven't yet converted all the calls to reportWarning to reportWarningCode.
Once we do, this becomes much simpler, and we can rename reportWarningCode to
reportWarning. Sorry for the confusion.

[ahe, 2013-10-11 08:34:15]

BTW: I don't mean to imply you can't submit this as is. Johnni has already
approved this CL, and I have a lot of other reviews to get through, so I'm
deferring to his judgement :-)

[aam-me, 2013-10-11 11:21:31]

Thanks for the review!

I went ahead with reportWarning->reportWarningCode change.

PTAL

https://chromiumcodereview.appspot.com/26651006/diff/10001/sdk/lib/_internal/...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://chromiumcodereview.appspot.com/26651006/diff/10001/sdk/lib/_internal/...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:3693:
MessageKind.DEPRECATED_TYPEDEF_MIXIN_SYNTAX.message());
Ah, I understand. Thanks for the explanation. Switched to reportWarningCode.

[Johnni Winther, 2013-10-11 11:23:11]

Still LGTM!

[aam-me, 2013-10-11 11:42:00]

Thank you Johnni!

I will commit later in the US EDT evening when it is easier for me to monitor
the build if that's okay.

[ahe, 2013-10-11 11:42:27]

On 2013/10/11 11:42:00, aam wrote:
> Thank you Johnni!
> 
> I will commit later in the US EDT evening when it is easier for me to monitor
> the build if that's okay.

Sure! :-)

[aam-me, 2013-10-12 00:55:37]

Committed patchset #3 manually as r28561 (presubmit successful).