map fasta error codes to analyzer

map fasta error codes to analyzer

Rather than have multple fine grained error codes for each type
of modifier in each situation, this CL adds a new analyzer
ParserErrorCode.EXTRANEOUS_MODIFIER parallel to the fast parser
error code and updates the AstBuilder to generate that error
rather than ParserErrorCode.ABSTRACT_CLASS_MEMBER.

My goal is to review each analyzer error code similar to
ParserErrorCode.ABSTRACT_CLASS_MEMBER and map each to the same
ParserErrorCode.EXTRANEOUS_MODIFIER error code.

R=paulberry@google.com

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

[danrubel, 2017-08-20 16:57:55]

danrubel@google.com changed reviewers:
+ paulberry@google.com

[Paul Berry, 2017-08-21 05:05:47]

lgtm

Code lgtm, and in general I am in favor of this approach.

However I have a concern specifically about the case of extraneous "abstract"
modifiers.  When we talk about a method in an abstract class that lacks a
definition, we tend to call it an "abstract method", and as a result even
seasoned Dart programmers sometimes make the error of putting the "abstract"
keyword on a method instead of (or in addition to) putting it on the class.  (I
do it myself occasionally).  Since it's such an easy error to make, it seems
like it might be nice to keep a special error code for it, so that we can give
the user a nice informative error message.

I agree, however, that in general we don't need a special error code for every
possible situation where a modifier appears where it is not expected.

[ahe, 2017-08-22 13:03:48]

On 2017/08/21 05:05:47, Paul Berry wrote:
> lgtm
> 
> Code lgtm, and in general I am in favor of this approach.
> 
> However I have a concern specifically about the case of extraneous "abstract"
> modifiers.  When we talk about a method in an abstract class that lacks a
> definition, we tend to call it an "abstract method", and as a result even
> seasoned Dart programmers sometimes make the error of putting the "abstract"
> keyword on a method instead of (or in addition to) putting it on the class. 
(I
> do it myself occasionally).  Since it's such an easy error to make, it seems
> like it might be nice to keep a special error code for it, so that we can give
> the user a nice informative error message.
> 
> I agree, however, that in general we don't need a special error code for every
> possible situation where a modifier appears where it is not expected.

In that case, perhaps the parser needs to change so it reports a different error
code on abstract?

[danrubel, 2017-08-25 13:00:23]

Description was changed from

==========
map fasta error codes to analyzer

Rather than have multple fine grained error codes for each type
of modifier in each situation, this CL adds a new analyzer
ParserErrorCode.EXTRANEOUS_MODIFIER parallel to the fast parser
error code and updates the AstBuilder to generate that error
rather than ParserErrorCode.ABSTRACT_CLASS_MEMBER.

My goal is to review each analyzer error code similar to
ParserErrorCode.ABSTRACT_CLASS_MEMBER and map each to the same
ParserErrorCode.EXTRANEOUS_MODIFIER error code.
==========

to

==========
map fasta error codes to analyzer

Rather than have multple fine grained error codes for each type
of modifier in each situation, this CL adds a new analyzer
ParserErrorCode.EXTRANEOUS_MODIFIER parallel to the fast parser
error code and updates the AstBuilder to generate that error
rather than ParserErrorCode.ABSTRACT_CLASS_MEMBER.

My goal is to review each analyzer error code similar to
ParserErrorCode.ABSTRACT_CLASS_MEMBER and map each to the same
ParserErrorCode.EXTRANEOUS_MODIFIER error code.

R=paulberry@google.com

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

[danrubel, 2017-08-25 13:00:26]

Committed patchset #4 (id:60001) manually as
8786656783866fbfb2bcb12490cd036bfe0c8ca4 (presubmit successful).

[danrubel, 2017-09-07 19:56:16]

On 2017/08/21 05:05:47, Paul Berry wrote:
> lgtm
> 
> Code lgtm, and in general I am in favor of this approach.
> 
> However I have a concern specifically about the case of extraneous "abstract"
> modifiers.  When we talk about a method in an abstract class that lacks a
> definition, we tend to call it an "abstract method", and as a result even
> seasoned Dart programmers sometimes make the error of putting the "abstract"
> keyword on a method instead of (or in addition to) putting it on the class. 
(I
> do it myself occasionally).  Since it's such an easy error to make, it seems
> like it might be nice to keep a special error code for it, so that we can give
> the user a nice informative error message.
> 
> I agree, however, that in general we don't need a special error code for every
> possible situation where a modifier appears where it is not expected.

Fixed in https://dart-review.googlesource.com/c/sdk/+/4143