Share the first error messages.

Share the first error messages.

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

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

[floitsch, 2016-02-02 22:09:18]

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

[floitsch, 2016-02-02 22:09:19]

Baby steps...

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/gener...
File pkg/dart_messages/lib/generated/shared_messages.json (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/gener...
pkg/dart_messages/lib/generated/shared_messages.json:1:
{"exampleMessage":{"id":"use an Id generated by
bin/message_id.dart","subId":0,"category":"AnalysisOptionsError","template":"#use
#named
#arguments","templateHoleOrder":["arguments","named","use"],"howToFix":"an
explanation on how to fix things","options":null,"usedBy":[],"examples":["     
Some multiline example;\n      That generates the bug.",{"fileA.dart":"       
or a map from file to content.\n        again multiline","fileB.dart":"       
with possibly multiple files.\n        muliline
too"}]},"CONST_CONSTRUCTOR_HAS_BODY":{"id":"LGJGHW","subId":0,"category":"ParserError","template":"Const
constructor or factory can't have a
body.","templateHoleOrder":null,"howToFix":"Remove the 'const' keyword or the
body.","options":null,"usedBy":["Platform.dart2js"],"examples":["         class
C {\n           const C() {}\n         }\n\n         main() => new C();","      
  class C {\n           const factory C() {}\n         }\n\n         main() =>
new
C();"]},"CONST_CONSTRUCTOR_WITH_BODY":{"id":"LGJGHW","subId":1,"category":"ParserError","template":"Const
constructor or factory can't have a
body.","templateHoleOrder":null,"howToFix":"Remove the 'const' keyword or the
body.","options":null,"usedBy":["Platform.analyzer"],"examples":["         class
C {\n           const C() {}\n         }\n\n         main() => new
C();"]},"CONST_FACTORY":{"id":"LGJGHW","subId":2,"category":"ParserError","template":"Only
redirecting factory constructors can be declared to be
'const'","templateHoleOrder":null,"howToFix":"Remove the 'const' keyword or
replace the body with '=' followed by a valid
target","options":null,"usedBy":["Platform.analyzer"],"examples":["        
class C {\n           const factory C() {}\n         }\n\n         main() => new
C();"]}}
I now include everything in the JSON, to make sure that the test catches more
changes.

[Brian Wilkerson, 2016-02-03 00:27:12]

This might not have been the best example :-). It doesn't do a very good job of
illustrating the value because we aren't actually sharing any messages. (Of
course, it does add a correction message that analyzer didn't have before.)

https://codereview.chromium.org/1660703002/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/generated/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/generated/shared_messages.dart:19: const
ParserErrorCode CONST_FACTORY = const ParserErrorCode(
nit: add a blank line between constants for readability?

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:161: category: Category.parserError,
Missing subId?

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:184: template: "Const constructor or
factory can't have a body.",
The "or factory" is confusing. If the constructor is a factory constructor the
CONST_FACTORY message will be used. Hence this message should not mention
factories.

Which means that we can revert this to the current message. Note that every time
we change a message we risk breaking one of the post-processing tools that are
matching the message. (I wish there weren't any such tools, but until we get
stable id's and convert the tools to use them we're kind of stuck.)

[floitsch, 2016-02-03 17:50:25]

https://codereview.chromium.org/1660703002/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/generated/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/generated/shared_messages.dart:19: const
ParserErrorCode CONST_FACTORY = const ParserErrorCode(
On 2016/02/03 00:27:12, Brian Wilkerson wrote:
> nit: add a blank line between constants for readability?

Done.

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:161: category: Category.parserError,
On 2016/02/03 00:27:12, Brian Wilkerson wrote:
> Missing subId?

Missing subid implies 0, but I added it.

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:184: template: "Const constructor or
factory can't have a body.",
On 2016/02/03 00:27:12, Brian Wilkerson wrote:
> The "or factory" is confusing. If the constructor is a factory constructor the
> CONST_FACTORY message will be used. Hence this message should not mention
> factories.
> 
> Which means that we can revert this to the current message. Note that every
time
> we change a message we risk breaking one of the post-processing tools that are
> matching the message. (I wish there weren't any such tools, but until we get
> stable id's and convert the tools to use them we're kind of stuck.)

My mistake. Removed the factory part.
I don't want to use the original message, since it starts with a lower case
character.
If you feel strongly, I will change, though.

[Brian Wilkerson, 2016-02-03 20:37:22]

LGTM

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:161: category: Category.parserError,
Good to know.

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:184: template: "Const constructor or
factory can't have a body.",
Like I said, every time we change a message we risk breaking someone. But if you
do change it I would expect "Const constructors cannot have a body." (or
"can't", if we're being informal).

[floitsch, 2016-02-03 21:15:39]

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:184: template: "Const constructor or
factory can't have a body.",
On 2016/02/03 20:37:22, Brian Wilkerson wrote:
> Like I said, every time we change a message we risk breaking someone. But if
you
> do change it I would expect "Const constructors cannot have a body." (or
> "can't", if we're being informal).

The latest version is "Const constructor cannot have a body". (Is it, that you
want to make it plural?)

I agree with you that changing the error-messages is dangerous. However, I don't
expect anyone to grep for this particular message. That just should never happen
in code.

[Brian Wilkerson, 2016-02-03 21:30:59]

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
File pkg/dart_messages/lib/shared_messages.dart (right):

https://codereview.chromium.org/1660703002/diff/1/pkg/dart_messages/lib/share...
pkg/dart_messages/lib/shared_messages.dart:184: template: "Const constructor or
factory can't have a body.",
> The latest version is "Const constructor cannot have a body".

Sounds good.

> (Is it, that you want to make it plural?)

Nope.

> I agree with you that changing the error-messages is dangerous. However, I
don't
> expect anyone to grep for this particular message. That just should never
happen
> in code.

True. Probably more of an issue for warnings and hints.

[Johnni Winther, 2016-02-04 08:40:12]

lgtm

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

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:163: template: "Const constructor or
factory cannot have a body.",
`cannot` -> `can't` ? (cf. item 6 in the dartdoc) (I'm in doubt here)

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:164: howToFix: "Remove the 'const'
keyword or the body.",
I think the [howToFix] should be a suggestion and not a command, cf. item 10 in
the dartdoc; e.g. `Try removing the 'const' keyword or the body.` Maybe run this
by Luke.

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:202: "Only redirecting factory
constructors can be declared to be 'const'",
Add period to finish the sentence.

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:204: "followed by a valid target",
Ditto.

[floitsch, 2016-02-04 10:44:41]

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

[floitsch, 2016-02-04 10:44:42]

CCing Luke.
I will commit now, but can do a follow-up change if Luke has comments.

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

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:163: template: "Const constructor or
factory cannot have a body.",
On 2016/02/04 08:40:11, Johnni Winther wrote:
> `cannot` -> `can't` ? (cf. item 6 in the dartdoc) (I'm in doubt here)

Done.

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:164: howToFix: "Remove the 'const'
keyword or the body.",
On 2016/02/04 08:40:11, Johnni Winther wrote:
> I think the [howToFix] should be a suggestion and not a command, cf. item 10
in
> the dartdoc; e.g. `Try removing the 'const' keyword or the body.` Maybe run
this
> by Luke.

Done.

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:202: "Only redirecting factory
constructors can be declared to be 'const'",
On 2016/02/04 08:40:11, Johnni Winther wrote:
> Add period to finish the sentence.

Done.

https://codereview.chromium.org/1660703002/diff/40001/pkg/dart_messages/lib/s...
pkg/dart_messages/lib/shared_messages.dart:204: "followed by a valid target",
On 2016/02/04 08:40:11, Johnni Winther wrote:
> Ditto.

Done.

[floitsch, 2016-02-04 11:52:43]

Description was changed from

==========
Share the first error messages.
==========

to

==========
Share the first error messages.

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

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

[floitsch, 2016-02-04 11:52:45]

Committed patchset #6 (id:90011) manually as
6468ba5022ad5ba385be22badc14351e518f6573 (presubmit successful).