Generate dart2js and analyzer files for shared messages.

Generate dart2js and analyzer files for shared messages.

R=brianwilkerson@google.com, sigmund@google.com

Committed: https://github.com/dart-lang/sdk/commit/c849a627449881c043671553a7e05fc39a789580

[floitsch, 2016-01-13 19:28:14]

floitsch@google.com changed reviewers:
+ johnniwinther@google.com

[floitsch, 2016-01-13 19:28:14]

I'm not sure if we should keep the exampleMessage.
I would keep it in until we have at least some real shared error messages. Once
they are in, we can safely remove the example.

[floitsch, 2016-01-14 17:47:34]

floitsch@google.com changed reviewers:
+ brianwilkerson@google.com

[floitsch, 2016-01-14 17:47:34]

Updated the publish-script to generate the analyzer file.

The idea for the analyzer is to the generated errors from within the ErrorCode
classes.
Once all codes of an error-class are in the shared messages, we should consider
generating the full error-code class. This would require adding the severity and
type information to the shared data (which is probably ok).

[Siggi Cherem (dart-lang), 2016-01-14 17:57:40]

sigmund@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2016-01-14 17:57:40]

lgtm for dartjs, some minor suggestions below.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
File pkg/dart_messages/bin/publish.dart (right):

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:18: DON'T EDIT. GENERATED. DON'T EDIT.
I think it's ok to only say this once or twice :)

Consider adding here a note about how it is autogenerated, to guide someone who
discovers the generated file but doesn't know that's the next step.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:110: out.write("SharedMessageKind.$name:
const MessageTemplate(");
indent the output here.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:115: out.write(", howToFix:
${escapeString(message.howToFix)}");
split each name argument to a separate line (and indent)

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:126: out.write(escapeString(example));
maybe switch to use escapeString for ''' here and below? I think we can also
indent the contents, since these strings are used for automated testing and not
to present messages to users.

Basically all the suggestions above are about making the generated code more
readable. For example:

Instead of:

const Map<SharedMessageKind, MessageTemplate> TEMPLATES = const
<SharedMessageKind, MessageTemplate>{
SharedMessageKind.exampleMessage: const MessageTemplate(SharedMessageKind.exampl
eMessage, "#use #named #arguments", howToFix: "an explanation on how to fix thin
gs", examples: const ["Some multiline example;\nThat generates the bug.",const {
'fileA.dart':"or a map from file to content.\nagain multiline",'fileB.dart':"wit
h possibly multiple files.\nmuliline too",},]),

We can generate:

const Map<SharedMessageKind, MessageTemplate> TEMPLATES =
    const <SharedMessageKind, MessageTemplate>{
  SharedMessageKind.exampleMessage: const MessageTemplate(
      SharedMessageKind.exampleMessage,
      "#use #named #arguments",
      howToFix: "an explanation on how to fix things",
      examples: const ['''
          Some multiline example;
          That generates the bug.''',
      const {
        'fileA.dart': '''
            or a map from file to content.
            again multiline
            ''',
        'fileB.dart':'''
            with possibly multiple files.
            muliline too
            ''',
       },
  ]),
  ...

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:101: Some multiline example;
consider indenting this if the extra space is not relevant (e.g. if this only
used for testing like we do in dart2js, or if we can format the output anyways
afterwards).

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
File pkg/dart_messages/lib/shared_messages.json (right):

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.json:1: {"exampleMessage":{"id":"use an Id
generated by bin/message_id.dart","template":"#use #named
#arguments","howToFix":"an explanation on how to fix things"}}
Up to yoU:
 - consider moving this under a generated folder?
 - consider adding an AUTOGENERATED comment?

I know JSON doesn't let you put comments that easily. One option would be to add
a bogus entry. For example:
[" /** AUTOGENERATED --- this file was generated by `bin/publish.dart` **/ ",
{"exampleMessage":{....}})

Or to add a line with an arbitraty format, but skip it before parsing in your
tools.

[floitsch, 2016-01-14 18:16:16]

On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> lgtm for dartjs, some minor suggestions below.
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> File pkg/dart_messages/bin/publish.dart (right):
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> pkg/dart_messages/bin/publish.dart:18: DON'T EDIT. GENERATED. DON'T EDIT.
> I think it's ok to only say this once or twice :)
> 
> Consider adding here a note about how it is autogenerated, to guide someone
who
> discovers the generated file but doesn't know that's the next step.
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> pkg/dart_messages/bin/publish.dart:110: out.write("SharedMessageKind.$name:
> const MessageTemplate(");
> indent the output here.
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> pkg/dart_messages/bin/publish.dart:115: out.write(", howToFix:
> ${escapeString(message.howToFix)}");
> split each name argument to a separate line (and indent)
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> pkg/dart_messages/bin/publish.dart:126: out.write(escapeString(example));
> maybe switch to use escapeString for ''' here and below? I think we can also
> indent the contents, since these strings are used for automated testing and
not
> to present messages to users.
> 
> Basically all the suggestions above are about making the generated code more
> readable. For example:
> 
> Instead of:
> 
> const Map<SharedMessageKind, MessageTemplate> TEMPLATES = const
> <SharedMessageKind, MessageTemplate>{
> SharedMessageKind.exampleMessage: const
MessageTemplate(SharedMessageKind.exampl
> eMessage, "#use #named #arguments", howToFix: "an explanation on how to fix
thin
> gs", examples: const ["Some multiline example;\nThat generates the bug.",const
{
> 'fileA.dart':"or a map from file to content.\nagain
multiline",'fileB.dart':"wit
> h possibly multiple files.\nmuliline too",},]),
> 
> We can generate:
> 
> const Map<SharedMessageKind, MessageTemplate> TEMPLATES =
>     const <SharedMessageKind, MessageTemplate>{
>   SharedMessageKind.exampleMessage: const MessageTemplate(
>       SharedMessageKind.exampleMessage,
>       "#use #named #arguments",
>       howToFix: "an explanation on how to fix things",
>       examples: const ['''
>           Some multiline example;
>           That generates the bug.''',
>       const {
>         'fileA.dart': '''
>             or a map from file to content.
>             again multiline
>             ''',
>         'fileB.dart':'''
>             with possibly multiple files.
>             muliline too
>             ''',
>        },
>   ]),
>   ...
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> File pkg/dart_messages/lib/shared_messages.dart (right):
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> pkg/dart_messages/lib/shared_messages.dart:101: Some multiline example;
> consider indenting this if the extra space is not relevant (e.g. if this only
> used for testing like we do in dart2js, or if we can format the output anyways
> afterwards).
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> File pkg/dart_messages/lib/shared_messages.json (right):
> 
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> pkg/dart_messages/lib/shared_messages.json:1: {"exampleMessage":{"id":"use an
Id
> generated by bin/message_id.dart","template":"#use #named
> #arguments","howToFix":"an explanation on how to fix things"}}
> Up to yoU:
>  - consider moving this under a generated folder?
>  - consider adding an AUTOGENERATED comment?
> 
> I know JSON doesn't let you put comments that easily. One option would be to
add
> a bogus entry. For example:
> [" /** AUTOGENERATED --- this file was generated by `bin/publish.dart` **/ ",
> {"exampleMessage":{....}})
> 
> Or to add a line with an arbitraty format, but skip it before parsing in your
> tools.

I would prefer if the output didn't look clean. In a big file (and especially
with the IDE) the "GENERATED" warning isn't visible, and then it's way too easy
to modify the generated file. (Happened far too often to me :)

[Siggi Cherem (dart-lang), 2016-01-14 18:38:05]

On 2016/01/14 18:16:16, floitsch wrote:
> On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> > lgtm for dartjs, some minor suggestions below.
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> > File pkg/dart_messages/bin/publish.dart (right):
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> > pkg/dart_messages/bin/publish.dart:18: DON'T EDIT. GENERATED. DON'T EDIT.
> > I think it's ok to only say this once or twice :)
> > 
> > Consider adding here a note about how it is autogenerated, to guide someone
> who
> > discovers the generated file but doesn't know that's the next step.
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> > pkg/dart_messages/bin/publish.dart:110: out.write("SharedMessageKind.$name:
> > const MessageTemplate(");
> > indent the output here.
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> > pkg/dart_messages/bin/publish.dart:115: out.write(", howToFix:
> > ${escapeString(message.howToFix)}");
> > split each name argument to a separate line (and indent)
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
> > pkg/dart_messages/bin/publish.dart:126: out.write(escapeString(example));
> > maybe switch to use escapeString for ''' here and below? I think we can also
> > indent the contents, since these strings are used for automated testing and
> not
> > to present messages to users.
> > 
> > Basically all the suggestions above are about making the generated code more
> > readable. For example:
> > 
> > Instead of:
> > 
> > const Map<SharedMessageKind, MessageTemplate> TEMPLATES = const
> > <SharedMessageKind, MessageTemplate>{
> > SharedMessageKind.exampleMessage: const
> MessageTemplate(SharedMessageKind.exampl
> > eMessage, "#use #named #arguments", howToFix: "an explanation on how to fix
> thin
> > gs", examples: const ["Some multiline example;\nThat generates the
bug.",const
> {
> > 'fileA.dart':"or a map from file to content.\nagain
> multiline",'fileB.dart':"wit
> > h possibly multiple files.\nmuliline too",},]),
> > 
> > We can generate:
> > 
> > const Map<SharedMessageKind, MessageTemplate> TEMPLATES =
> >     const <SharedMessageKind, MessageTemplate>{
> >   SharedMessageKind.exampleMessage: const MessageTemplate(
> >       SharedMessageKind.exampleMessage,
> >       "#use #named #arguments",
> >       howToFix: "an explanation on how to fix things",
> >       examples: const ['''
> >           Some multiline example;
> >           That generates the bug.''',
> >       const {
> >         'fileA.dart': '''
> >             or a map from file to content.
> >             again multiline
> >             ''',
> >         'fileB.dart':'''
> >             with possibly multiple files.
> >             muliline too
> >             ''',
> >        },
> >   ]),
> >   ...
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> > File pkg/dart_messages/lib/shared_messages.dart (right):
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> > pkg/dart_messages/lib/shared_messages.dart:101: Some multiline example;
> > consider indenting this if the extra space is not relevant (e.g. if this
only
> > used for testing like we do in dart2js, or if we can format the output
anyways
> > afterwards).
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> > File pkg/dart_messages/lib/shared_messages.json (right):
> > 
> >
>
https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
> > pkg/dart_messages/lib/shared_messages.json:1: {"exampleMessage":{"id":"use
an
> Id
> > generated by bin/message_id.dart","template":"#use #named
> > #arguments","howToFix":"an explanation on how to fix things"}}
> > Up to yoU:
> >  - consider moving this under a generated folder?
> >  - consider adding an AUTOGENERATED comment?
> > 
> > I know JSON doesn't let you put comments that easily. One option would be to
> add
> > a bogus entry. For example:
> > [" /** AUTOGENERATED --- this file was generated by `bin/publish.dart` **/
",
> > {"exampleMessage":{....}})
> > 
> > Or to add a line with an arbitraty format, but skip it before parsing in
your
> > tools.
> 
> I would prefer if the output didn't look clean. In a big file (and especially
> with the IDE) the "GENERATED" warning isn't visible, and then it's way too
easy
> to modify the generated file. (Happened far too often to me :)

You could also put the GENERATED reminder in front of every message...
or as a margin at the end of every line ;-P

const Map<SharedMessageKind, MessageTemplate> TEMPLATES = // GENERATED FILE
    const <SharedMessageKind, MessageTemplate>{           // DO NOT EDIT
  SharedMessageKind.exampleMessage: const MessageTemplate(//
      SharedMessageKind.exampleMessage,                   // GENERATED FILE
      "#use #named #arguments",                           // DO NOT EDIT
      howToFix: "an explanation on how to fix things",    //
      examples: const ['''                                // GENERATED FILE
          Some multiline example;                         // DO NOT EDIT
          That generates the bug.''',                     //
      const {                                             // GENERATED FILE
        'fileA.dart': '''                                 // DO NOT EDIT
            or a map from file to content.                // ...
            again multiline
            ''',
        'fileB.dart':'''
            with possibly multiple files.
            muliline too
            ''',
     },
  ]),

[floitsch, 2016-01-15 17:20:39]

Went with warning messages after every message.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
File pkg/dart_messages/bin/publish.dart (right):

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:18: DON'T EDIT. GENERATED. DON'T EDIT.
On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> I think it's ok to only say this once or twice :)
> 
> Consider adding here a note about how it is autogenerated, to guide someone
who
> discovers the generated file but doesn't know that's the next step.

Done.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:110: out.write("SharedMessageKind.$name:
const MessageTemplate(");
On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> indent the output here.

Done.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:115: out.write(", howToFix:
${escapeString(message.howToFix)}");
On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> split each name argument to a separate line (and indent)

Done.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/bin/p...
pkg/dart_messages/bin/publish.dart:126: out.write(escapeString(example));
On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> maybe switch to use escapeString for ''' here and below? I think we can also
> indent the contents, since these strings are used for automated testing and
not
> to present messages to users.
> 
> Basically all the suggestions above are about making the generated code more
> readable. For example:
> 
> Instead of:
> 
> const Map<SharedMessageKind, MessageTemplate> TEMPLATES = const
> <SharedMessageKind, MessageTemplate>{
> SharedMessageKind.exampleMessage: const
MessageTemplate(SharedMessageKind.exampl
> eMessage, "#use #named #arguments", howToFix: "an explanation on how to fix
thin
> gs", examples: const ["Some multiline example;\nThat generates the bug.",const
{
> 'fileA.dart':"or a map from file to content.\nagain
multiline",'fileB.dart':"wit
> h possibly multiple files.\nmuliline too",},]),
> 
> We can generate:
> 
> const Map<SharedMessageKind, MessageTemplate> TEMPLATES =
>     const <SharedMessageKind, MessageTemplate>{
>   SharedMessageKind.exampleMessage: const MessageTemplate(
>       SharedMessageKind.exampleMessage,
>       "#use #named #arguments",
>       howToFix: "an explanation on how to fix things",
>       examples: const ['''
>           Some multiline example;
>           That generates the bug.''',
>       const {
>         'fileA.dart': '''
>             or a map from file to content.
>             again multiline
>             ''',
>         'fileB.dart':'''
>             with possibly multiple files.
>             muliline too
>             ''',
>        },
>   ]),
>   ...

Done.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:101: Some multiline example;
On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> consider indenting this if the extra space is not relevant (e.g. if this only
> used for testing like we do in dart2js, or if we can format the output anyways
> afterwards).

Done.

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
File pkg/dart_messages/lib/shared_messages.json (right):

https://codereview.chromium.org/1582903003/diff/60001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.json:1: {"exampleMessage":{"id":"use an Id
generated by bin/message_id.dart","template":"#use #named
#arguments","howToFix":"an explanation on how to fix things"}}
On 2016/01/14 17:57:40, Siggi Cherem (dart-lang) wrote:
> Up to yoU:
>  - consider moving this under a generated folder?
>  - consider adding an AUTOGENERATED comment?
> 
> I know JSON doesn't let you put comments that easily. One option would be to
add
> a bogus entry. For example:
> [" /** AUTOGENERATED --- this file was generated by `bin/publish.dart` **/ ",
> {"exampleMessage":{....}})
> 
> Or to add a line with an arbitraty format, but skip it before parsing in your
> tools.

Moved, but didn't add auto-generated notice. It's one long line of JSON, which
is very uninviting to edit.

[Siggi Cherem (dart-lang), 2016-01-15 17:44:58]

lgtm, thanks!

[floitsch, 2016-01-19 12:50:16]

Description was changed from

==========
Generate dart2js files for shared messages.
==========

to

==========
Generate dart2js and analyzer files for shared messages.
==========

[floitsch, 2016-01-19 12:50:44]

ping Brian.
The CL now also generates the analyzer version.

[Brian Wilkerson, 2016-01-23 18:27:11]

LGTM

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
File pkg/dart_messages/bin/publish.dart (right):

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:19: This file has been generated by
'publish.dart' in the dart_messages package.
This comment should include instructions for how to make changes to the error
messages, including (a) which file to edit and (b) how to run 'publish.dart'.

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:172: ///     const EXAMPLE_MESSAGE = const
FooCategoryErrorCode(
I think this is missing the class name:

const FooCategoryErrorCode EXAMPLE_MESSAGE = ...

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:174: ///         "Don't use #foo with #bar",
And I think messages get converted to

"Don't use {0} with {1}",

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:175: ///         "Just don't do it"); // The
how-to-fix.
"The how-to-fix." --> "Generated. Don't edit." (or just remove it.)

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:186: out.writeln("import '../error.dart';");
We avoid relative paths where possible, so I would prefer

out.writeln("import 'package:analyzer/src/generated/error.dart';");

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:200: if (message.howToFix) {
I think this wants to be "message.howToFix != null", otherwise it will fail in
checked mode.

But I'd simplify the code to:

out.writeln("    ${escapeString(message.analyzerTemplate)});");
if (message.howToFix == null) {
  out.writeln("    null);");
} else {
  out.writeln("    ${escapeString(message.howToFix)});");
}

The explicit `null` argument won't hurt anything, and should hopefully be fairly
rare as we improve our error messages. (In fact, we might want to make it a goal
that we always add fix messages as we port messages over.)

https://chromiumcodereview.appspot.com/1582903003/diff/160001/pkg/dart_messag...
pkg/dart_messages/bin/publish.dart:204: out.writeln(");  // Generated. Don't
edit.");
Having this on every constant seems like overkill. There's already a message at
the beginning of the file telling us not to edit it.

[floitsch, 2016-01-25 12:08:42]

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
File pkg/dart_messages/bin/publish.dart (right):

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:19: This file has been generated by
'publish.dart' in the dart_messages package.
On 2016/01/23 18:27:10, Brian Wilkerson wrote:
> This comment should include instructions for how to make changes to the error
> messages, including (a) which file to edit and (b) how to run 'publish.dart'.

Done.

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:172: ///     const EXAMPLE_MESSAGE = const
FooCategoryErrorCode(
On 2016/01/23 18:27:11, Brian Wilkerson wrote:
> I think this is missing the class name:
> 
> const FooCategoryErrorCode EXAMPLE_MESSAGE = ...

Done.

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:174: ///         "Don't use #foo with #bar",
On 2016/01/23 18:27:10, Brian Wilkerson wrote:
> And I think messages get converted to
> 
> "Don't use {0} with {1}",

Done.

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:175: ///         "Just don't do it"); // The
how-to-fix.
On 2016/01/23 18:27:10, Brian Wilkerson wrote:
> "The how-to-fix." --> "Generated. Don't edit." (or just remove it.)

Done.

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:186: out.writeln("import '../error.dart';");
On 2016/01/23 18:27:10, Brian Wilkerson wrote:
> We avoid relative paths where possible, so I would prefer
> 
> out.writeln("import 'package:analyzer/src/generated/error.dart';");

I do the exact opposite: avoid non-relative paths as much as possible...

done.

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:200: if (message.howToFix) {
On 2016/01/23 18:27:11, Brian Wilkerson wrote:
> I think this wants to be "message.howToFix != null", otherwise it will fail in
> checked mode.
> 
> But I'd simplify the code to:
> 
> out.writeln("    ${escapeString(message.analyzerTemplate)});");
> if (message.howToFix == null) {
>   out.writeln("    null);");
> } else {
>   out.writeln("    ${escapeString(message.howToFix)});");
> }
> 
> The explicit `null` argument won't hurt anything, and should hopefully be
fairly
> rare as we improve our error messages. (In fact, we might want to make it a
goal
> that we always add fix messages as we port messages over.)

Done.

https://codereview.chromium.org/1582903003/diff/160001/pkg/dart_messages/bin/...
pkg/dart_messages/bin/publish.dart:204: out.writeln(");  // Generated. Don't
edit.");
On 2016/01/23 18:27:10, Brian Wilkerson wrote:
> Having this on every constant seems like overkill. There's already a message
at
> the beginning of the file telling us not to edit it.

Up to you, but since they don't hurt I think they are worth it.
Basically, the way this usually happens: someone wants to add a new error
message, and finds the declaration of a similar message. Then, she copy/pastes
that error message next to the old one...

I'm leaving it for now, but can remove it in a follow-up CL, if you aren't
convinced.

[floitsch, 2016-01-25 12:08:44]

Description was changed from

==========
Generate dart2js and analyzer files for shared messages.
==========

to

==========
Generate dart2js and analyzer files for shared messages.

R=brianwilkerson@google.com, sigmund@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c849a627449881c043671553a7e05fc39a789580
==========

[floitsch, 2016-01-25 12:08:46]

Committed patchset #10 (id:180001) manually as
c849a627449881c043671553a7e05fc39a789580 (presubmit successful).