Report errors for default parameter values, issue 351

Report errors for default parameter values, issue 351
http://code.google.com/p/dart/issues/detail?id=351


R=zundel@google.com
BUG=
TEST=

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

[scheglov, 2011-11-14 21:43:27]

Actually, I'm not satisfied by tests, please don't review until next patch.

[scheglov, 2011-11-15 14:53:16]

OK, patch is ready for review.

[zundel, 2011-11-15 16:16:57]

LGTM other than a few error strings.

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/parser/DartParser.java (right):

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/DartParser.java:988:
reportError(parameter, ParserErrorCode.EACH_PARAMETER_IS_REQUIRED);
I think this message is a bit confusing.  Could we change this message to be
something like, NAMED_PARAMETER_NOT_ALLOWED?

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java (right):

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:13:
DEFAULT_PARAMETER_ABSTRACT("Default values can not be specified in abstract
method"),
DEFAULT_VALUE_NOT_IN_ABSTRACT_METHOD("Default value can ...

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:14:
DEFAULT_PARAMETER_CLOSURE("Default values can not be specified in closure
parameter"),
DEFAULT_VALUE_NOT_IN_CLOSURE("Default value can ...

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:16:
DEFAULT_PARAMETER_TYPEDEF("Default values can not be specified in closure type
definition"),
DEFAULT_VALUE_NOT_IN_TYPEDEF("Default value can ...

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:20:
EACH_PARAMETER_IS_REQUIRED("Each parameter should be required"),
NAMED_PARAMETER_NOT_ALLOWED("Named parameter is not allowed for operator or
setter method"

[scheglov, 2011-11-15 16:33:21]

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java (right):

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:13:
DEFAULT_PARAMETER_ABSTRACT("Default values can not be specified in abstract
method"),
On 2011/11/15 16:16:57, zundel wrote:
> DEFAULT_VALUE_NOT_IN_ABSTRACT_METHOD("Default value can ...

Hm... I don't understand why to use "NOT" here and in other places (except of
operators and setter).
This is error, same as ABSTRACT_MEMBER_IN_INTERFACE, i.e. we   tell that
something is invalid - "default value for parameter in abstract method" is
invalid.

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:20:
EACH_PARAMETER_IS_REQUIRED("Each parameter should be required"),
On 2011/11/15 16:16:57, zundel wrote:
> NAMED_PARAMETER_NOT_ALLOWED("Named parameter is not allowed for operator or
> setter method"

Done.

[zundel, 2011-11-15 16:36:46]

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java (right):

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:13:
DEFAULT_PARAMETER_ABSTRACT("Default values can not be specified in abstract
method"),
On 2011/11/15 16:33:21, scheglov wrote:
> On 2011/11/15 16:16:57, zundel wrote:
> > DEFAULT_VALUE_NOT_IN_ABSTRACT_METHOD("Default value can ...
> 
> Hm... I don't understand why to use "NOT" here and in other places (except of
> operators and setter).
> This is error, same as ABSTRACT_MEMBER_IN_INTERFACE, i.e. we   tell that
> something is invalid - "default value for parameter in abstract method" is
> invalid.

The convention in this file (with some exceptions) is that the enumeration name
matches at least the beginning default english translation of the message.  

To follow that convention to the letter, we should name it:
DEFAULT_VALUE_CAN_NOT_BE_SPECIFIED_IN_ABSTRACT

That seems a little long to me, so I shortened it.

[scheglov, 2011-11-15 16:45:58]

OK, I've renamed constants to use long version.

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java (right):

http://codereview.chromium.org/8564027/diff/4001/compiler/java/com/google/dar...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:13:
DEFAULT_PARAMETER_ABSTRACT("Default values can not be specified in abstract
method"),
On 2011/11/15 16:36:46, zundel wrote:
> On 2011/11/15 16:33:21, scheglov wrote:
> > On 2011/11/15 16:16:57, zundel wrote:
> > > DEFAULT_VALUE_NOT_IN_ABSTRACT_METHOD("Default value can ...
> > 
> > Hm... I don't understand why to use "NOT" here and in other places (except
of
> > operators and setter).
> > This is error, same as ABSTRACT_MEMBER_IN_INTERFACE, i.e. we   tell that
> > something is invalid - "default value for parameter in abstract method" is
> > invalid.
> 
> The convention in this file (with some exceptions) is that the enumeration
name
> matches at least the beginning default english translation of the message.  
> 
> To follow that convention to the letter, we should name it:
> DEFAULT_VALUE_CAN_NOT_BE_SPECIFIED_IN_ABSTRACT
> 
> That seems a little long to me, so I shortened it.

Done.