Report StaticWarningCode.NEW_WITH_NON_TYPE and CompileTimeErrorCode.CONST_WITH_NON_TYPE

Report StaticWarningCode.NEW_WITH_NON_TYPE and CompileTimeErrorCode.CONST_WITH_NON_TYPE

R=brianwilkerson@google.com
BUG=

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

[Brian Wilkerson, 2013-05-10 14:11:16]

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

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/TypeResolverVisitor.java:421:
if (!(element instanceof ClassElement) &&
isTypeNameInInstanceCreationExpression(node)) {
This condition can be true if the element is null, but in that case I think the
better error is "undefined class". Perhaps reverse the order of the tests?

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/TypeResolverVisitor.java:692:
* @param typeName the type name to analyzer
nit: missing comment

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/CompileTimeErrorCodeTest.java
(right):

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/CompileTimeErrorCodeTest.java:722:
assertErrors(CompileTimeErrorCode.AMBIGUOUS_IMPORT,
StaticWarningCode.NEW_WITH_NON_TYPE);
I don't like the fact that we're getting two errors reported for the same
problem. Is there a reasonable way to remove the second error?

[scheglov, 2013-05-10 15:55:03]

PTAL

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

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/TypeResolverVisitor.java:421:
if (!(element instanceof ClassElement) &&
isTypeNameInInstanceCreationExpression(node)) {
On 2013/05/10 14:11:16, Brian Wilkerson wrote:
> This condition can be true if the element is null, but in that case I think
the
> better error is "undefined class". Perhaps reverse the order of the tests?

Done.

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/TypeResolverVisitor.java:692:
* @param typeName the type name to analyzer
On 2013/05/10 14:11:16, Brian Wilkerson wrote:
> nit: missing comment

Done.

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
File
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/CompileTimeErrorCodeTest.java
(right):

https://codereview.chromium.org/14855015/diff/1/editor/tools/plugins/com.goog...
editor/tools/plugins/com.google.dart.engine_test/src/com/google/dart/engine/resolver/CompileTimeErrorCodeTest.java:722:
assertErrors(CompileTimeErrorCode.AMBIGUOUS_IMPORT,
StaticWarningCode.NEW_WITH_NON_TYPE);
On 2013/05/10 14:11:16, Brian Wilkerson wrote:
> I don't like the fact that we're getting two errors reported for the same
> problem. Is there a reasonable way to remove the second error?

Done.

[Brian Wilkerson, 2013-05-10 16:13:29]

LGTM

https://codereview.chromium.org/14855015/diff/6001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/TypeResolverVisitor.java
(right):

https://codereview.chromium.org/14855015/diff/6001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/resolver/TypeResolverVisitor.java:420:
// check element
I think this will work, but it seems (without trying to re-write it) like it
would have been easier to leave the "element == null" case first and put the new
code in an else block.

[scheglov, 2013-05-10 18:00:54]

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