Add additional hints to the analyzer to catch the 'x is double' dart2js bug.

Add additional hints to the analyzer to catch the 'x is double' dart2js bug.
- A preference has been added to the editor to enable/disable the dart2js related hints.

R=brianwilkerson@google.com

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

[Brian Wilkerson, 2013-09-11 21:28:45]

LGTM, although I think it would be good to rename everything with "JS" in the
name to use "js" (in my mind "dart2js" is all one name).

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisOptions.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisOptions.java:22:
* Return {@code true} if analysis is to generate dart2js related hints results.
nit: "hints" --> "hint"

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java:38:
private boolean dart2jsHints = true;
nit: "dart2jsHints" --> "dart2jsHint" (for consistency (or reverse the other
names))

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java:47:
public boolean getDart2JSHint() {
nit: Perhaps "getDart2jsHint" and "setDart2jsHint"?

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java:48:
return dart2jsHints;
Should we 'and' this with 'hint' so that 'hint' becomes a master switch?

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/Dart2JSVerifier.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/Dart2JSVerifier.java:86:
String libraryName = libraryElement != null ? libraryElement.getName() : "";
Note that LibraryElement defines a method named isDartCore().

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/HintGenerator.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/HintGenerator.java:86:
dart2jsVerifier = new Dart2JSVerifier(errorReporter);
Why are deadCodeVerifier and dart2jsVerifier instance fields when we re-create
them each time through the method?

[jwren, 2013-09-11 22:21:07]

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisOptions.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/context/AnalysisOptions.java:22:
* Return {@code true} if analysis is to generate dart2js related hints results.
On 2013/09/11 21:28:46, Brian Wilkerson wrote:
> nit: "hints" --> "hint"

Done.

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java:38:
private boolean dart2jsHints = true;
On 2013/09/11 21:28:46, Brian Wilkerson wrote:
> nit: "dart2jsHints" --> "dart2jsHint" (for consistency (or reverse the other
> names))

Done.

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java:47:
public boolean getDart2JSHint() {
On 2013/09/11 21:28:46, Brian Wilkerson wrote:
> nit: Perhaps "getDart2jsHint" and "setDart2jsHint"?

Done.

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/context/AnalysisOptionsImpl.java:48:
return dart2jsHints;
On 2013/09/11 21:28:46, Brian Wilkerson wrote:
> Should we 'and' this with 'hint' so that 'hint' becomes a master switch?

I don't think we need to.

If the master hint pref is false, then HintGenerator will never be run to query
whether or not the dart2js hints are off.

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/Dart2JSVerifier.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/Dart2JSVerifier.java:86:
String libraryName = libraryElement != null ? libraryElement.getName() : "";
On 2013/09/11 21:28:46, Brian Wilkerson wrote:
> Note that LibraryElement defines a method named isDartCore().

Done.

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/HintGenerator.java
(right):

https://codereview.chromium.org/23496048/diff/4001/editor/tools/plugins/com.g...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/hint/HintGenerator.java:86:
dart2jsVerifier = new Dart2JSVerifier(errorReporter);
On 2013/09/11 21:28:46, Brian Wilkerson wrote:
> Why are deadCodeVerifier and dart2jsVerifier instance fields when we re-create
> them each time through the method?

Done.

[jwren, 2013-09-11 22:26:38]

Committed patchset #6 manually as r27410 (presubmit successful).

[karlklose, 2013-09-12 10:53:56]

DBC

https://codereview.chromium.org/23496048/diff/16001/editor/tools/plugins/com....
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/HintCode.java
(right):

https://codereview.chromium.org/23496048/diff/16001/editor/tools/plugins/com....
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/HintCode.java:52:
IS_INT("When compiled to JS, this test will return true when the left hand side
is a double"),
It '*may* return true' (if the double is of the form 'x.0').

https://codereview.chromium.org/23496048/diff/16001/editor/tools/plugins/com....
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/HintCode.java:61:
* Hint for the {@code x is! int} type checks.
Ditto.

https://codereview.chromium.org/23496048/diff/16001/editor/tools/plugins/com....
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/HintCode.java:63:
IS_NOT_INT("When compiled to JS, this test will return false when the left hand
side is a double"),
Long line? (I am not sure which limit you use)

[jwren, 2013-09-12 20:45:06]

https://codereview.chromium.org/23496048/diff/16001/editor/tools/plugins/com....
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/HintCode.java
(right):

https://codereview.chromium.org/23496048/diff/16001/editor/tools/plugins/com....
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/error/HintCode.java:52:
IS_INT("When compiled to JS, this test will return true when the left hand side
is a double"),
On 2013/09/12 10:53:56, karlklose wrote:
> It '*may* return true' (if the double is of the form 'x.0').

Agreed.  Sending a doc to you and the rest of the dart2js team shortly... we
were trying to keep the message short.  I realize that "will"->"may" would fix
it, but I'm not sure that having ambiguity in the message like that would be a
good thing.