DartC User Warning Framework

New ErrorCode implementations.


R=zundel@google.com
BUG=
TEST=

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

[scheglov, 2011-10-25 19:00:55]

Right now I created ErrorSeverity, SubSystem and moved all existing errors into
appropriate *ErrorCode implementations.

Open question now is how to fire and handle these errors.
I mostly worry about "typeError", because it is now protected by
shouldWarnOnNoSuchType() and explicit event.getErrorCode() checks.

[zundel, 2011-10-25 19:31:02]

Maybe we can just commit this without the change to DartCompilerListener.  This
does most of the dirty work without making any breaking API changes, I think.  
It would be nice to coordinate with the Editor team before making the breaking
change.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java (right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java:11:
ENTRY_POINT_METHOD_CANNOT_HAVE_PARAMETERS("Main entry point method cannot have
parameters"),
are these left behind errors with no references? If so, it is OK to remove them.
 They look like remnants of some old ways the compiler used to work before
public release.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/DartCompilerMainContext.java
(right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/DartCompilerMainContext.java:64: public
void typeError(DartCompilationError event) {
instead of special logic inside of typeError,we'll have one onErrror() message
with a test in it:

if (!shouldWarnOnNoSuchType && event.getSubSytem(STATIC_TYPE)) ...

Then we can change the DartCompilerListener definition as specified in the
design.  If you like, we can land this change first and then change the error
listeners separately.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java (right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:14:
DISALLOWED_ABSTRACT_KEYWORD("SyntaxError: abstract keyword not allowed here"),
the prefix "Syntax error"  is not uniformly applied.  I wonder if we should
remove the explicit prefix in each message and just paste it on in front of
every message in getMessage()?

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java
(right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java:56:
FACTORY_CANNOT_BE_ABSTRACT("SyntaxError: A factory cannot be abstract"),
these are labeled as 'syntax errors' but  this code made it past the syntax
stage. Let's remove the 'syntax error prefix, it (it doesn't have a spece
either, doesn't seem like a proper user message)

[zundel, 2011-10-25 19:56:39]

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/TypeErrorCode.java (right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/TypeErrorCode.java:53:
this(ErrorSeverity.ERROR, message);
Change to WARNING by default.

[scheglov, 2011-10-25 20:24:21]

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java (right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java:11:
ENTRY_POINT_METHOD_CANNOT_HAVE_PARAMETERS("Main entry point method cannot have
parameters"),

> are these left behind errors with no references? If so, it is OK to remove
them.
>  They look like remnants of some old ways the compiler used to work before
> public release.

I've removed all errors without references.
These left in DartCompilerErrorCode are actually referenced by DartCompiler
itself.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/DartCompilerMainContext.java
(right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/DartCompilerMainContext.java:64: public
void typeError(DartCompilationError event) {
On 2011/10/25 19:31:02, zundel wrote:
> instead of special logic inside of typeError,we'll have one onErrror() message
> with a test in it:
> 
> if (!shouldWarnOnNoSuchType && event.getSubSytem(STATIC_TYPE)) ...
> 
> Then we can change the DartCompilerListener definition as specified in the
> design.  If you like, we can land this change first and then change the error
> listeners separately. 

Done.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java (right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/parser/ParserErrorCode.java:14:
DISALLOWED_ABSTRACT_KEYWORD("SyntaxError: abstract keyword not allowed here"),
On 2011/10/25 19:31:02, zundel wrote:
> the prefix "Syntax error"  is not uniformly applied.  I wonder if we should
> remove the explicit prefix in each message and just paste it on in front of
> every message in getMessage()?

Done.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java
(right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java:56:
FACTORY_CANNOT_BE_ABSTRACT("SyntaxError: A factory cannot be abstract"),
On 2011/10/25 19:31:02, zundel wrote:
> these are labeled as 'syntax errors' but  this code made it past the syntax
> stage. Let's remove the 'syntax error prefix, it (it doesn't have a spece
> either, doesn't seem like a proper user message)

I will remove prefix here (+2 below), but in general these errors are strange.
They exist in both Parser and Resolver, without tests in either.

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/TypeErrorCode.java (right):

http://codereview.chromium.org/8395013/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/TypeErrorCode.java:53:
this(ErrorSeverity.ERROR, message);
On 2011/10/25 19:56:40, zundel wrote:
> Change to WARNING by default.

Done.

[zundel, 2011-10-27 16:19:54]

LGTM with one nit

http://codereview.chromium.org/8395013/diff/7001/compiler/java/com/google/dar...
File compiler/java/com/google/dart/runner/DartRunner.java (right):

http://codereview.chromium.org/8395013/diff/7001/compiler/java/com/google/dar...
compiler/java/com/google/dart/runner/DartRunner.java:129:
((DefaultErrorFormatter) formatter).setOutputStream(stderr);
this is pretty ugly, could you make an overload constructor to set the output
stream instead?