Report CompileTimeErrorCode.INVALID_CONST_FACTORY

Report CompileTimeErrorCode.INVALID_CONST_FACTORY

R=brianwilkerson@google.com
BUG=

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

[Brian Wilkerson, 2013-05-14 21:48:10]

LGTM

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java
(right):

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java:472:
* factoryConstructorSignature: factory identifier (`.' identifier)?
formalParameterList ;
nit: wrong quote character

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java
(right):

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java:1760:
* This verifies that the passed constructor declaration is not both 'const' and
'factory'.
"." --> " unless it's a redirecting factory constructor."

[jwren, 2013-05-14 21:52:05]

On 2013/05/14 21:36:36, scheglov wrote:

Everything in the CTEC enum set can be described in the spec with something like
"It is a compile-time error if...".  INVALID_CONST_FACTORY is not a compile time
error, it is a parser error.  The error code and logic to generate the error
should be moved to Parser/ ParserErrorCode, and the test(s) should go into
SimpleParserTest and/or ComplexParserTest.

[Brian Wilkerson, 2013-05-14 22:03:44]

I missed the fact that the error was being added, not just given a message. I
did wonder that we were not already checking in the parser (we do a lot of other
similar checks there), but then I realized that the parser checks modifiers long
before we know whether or not we have a redirecting factory constructor.

All that to say that it isn't 100% clear where this check should go. We should
at least add a TODO to come back and re-examine the issue.

[scheglov, 2013-05-15 00:39:28]

PTAL

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java
(right):

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/CompileTimeErrorCode.java:472:
* factoryConstructorSignature: factory identifier (`.' identifier)?
formalParameterList ;
On 2013/05/14 21:48:10, Brian Wilkerson wrote:
> nit: wrong quote character

Done.

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java
(right):

https://codereview.chromium.org/14654013/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/verifier/ErrorVerifier.java:1760:
* This verifies that the passed constructor declaration is not both 'const' and
'factory'.
On 2013/05/14 21:48:10, Brian Wilkerson wrote:
> "." --> " unless it's a redirecting factory constructor."

Done.

[Brian Wilkerson, 2013-05-15 04:16:22]

LGTM

[scheglov, 2013-05-15 16:24:06]

Committed patchset #2 manually as r22746 (presubmit successful).