Type inference behaves slightly differently in JDK 8

[omarkilani]

Not sure if this belongs in closure-library or closure-compiler.

When compiling closure-library with --jscomp_error=checkTypes using JDK 8, the compiler errors on this code:

closure/goog/dom/abstractrange.js:79: ERROR - Property item never defined on range
        } else if (!range.length || range.item(0).document != doc) {
                                    ^    

Modifying the code to:

diff --git a/closure/goog/dom/abstractrange.js b/closure/goog/dom/abstractrange.js
--- a/closure/goog/dom/abstractrange.js
+++ b/closure/goog/dom/abstractrange.js
@@ -76,7 +76,7 @@ goog.dom.AbstractRange.getBrowserSelectionForWindow = function(win) {
           if (range.parentElement().document != doc) {
             return null;
           }
-        } else if (!range.length || range.item(0).document != doc) {
+        } else if (!range.length || /** @type {ControlRange} */ (range).item(0).document != doc) {
           // For ControlRanges, check that the range has items, and that
           // the first item in the range is in the correct document.
           return null;

Solves the problem.

I have no idea why type inference works with JDK < 8 in this case.

[MatrixFrog]

Good catch! I'm not able to repro this, but I suspect it has something to do with the 'alternates' property of UnionType, which is typed as a Collection. Probably we're using a HashSet or something similar, and the iteration order of HashSet changed between Java 7 and Java 8.

[MatrixFrog]

edit: Nevermind, the other test failures I saw were from a different change, not from switching to Java 8.

[omarkilani]

I can repro on both Linux and OS X using:

java version "1.8.0_05"
Java(TM) SE Runtime Environment (build 1.8.0_05-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.5-b02, mixed mode)

This isn't an easy repro -- it only happens if a specific set of closure files are processed by the compiler. A simple test case which requires 'goog.dom.Range' doesn't seem to be enough to trigger it.

The strange thing is that this is the only closure library file that gives an error. I'll continue investigating... :)

[MatrixFrog]

I still haven't been able to repro the error you saw. However, by replacing HashMap in various places with an alternate Map implementation whose iteration order is random, I am able to get some similar errors. Specifically, by replacing the HashMap used in RecordTypeBuilder, I can get the errors below to happen sometimes. The alternates in the UnionTypeBuilders in the JSTypeRegistry.typesIndexedByProperty map will be in a different order based on the iteration order of the map that comes from RecordTypeBuilder. Then, the for loop in JSTypeRegistry#canPropertyBeDefined can have different results depending on that order. Or at least, this is what I think is happening.

Here are the errors:

goog/dom/pattern/tag.js:136: ERROR - Property style never defined on Node
token.style)) {
^

goog/editor/plugins/basictextformatter.js:882: ERROR - Property style never defined on Node
parentTag.style.textIndent = '-10000px';
^

goog/editor/plugins/basictextformatter.js:904: ERROR - Property style never defined on Node
parentTag.style.textIndent = '';
^

goog/tweak/tweakui.js:537: ERROR - Property style never defined on Node
this.showDescription_(entryDiv, !entryDiv.style.display);
^

goog/tweak/tweakui.js:551: ERROR - Property style never defined on Node
entryDiv.style.display = show ? 'block' : '';
^

goog/ui/colormenubuttonrenderer.js:125: ERROR - Property style never defined on Node
caption.firstChild.style.borderBottomColor = hexColor ||
^

goog/ui/editor/toolbarfactory.js:137: ERROR - Property style never defined on Node
content.firstChild.style.height = '1.1em';
^

[omarkilani]

I think you're right. It looks like the iteration order of HashMaps has changed in JDK 8:

http://docs.oracle.com/javase/8/docs/technotes/guides/collections/changes8.html

[MatrixFrog]

I believe 4fe6047 will fix this. Please reopen this if you see any issues on Java 8, going forward.