Fix for namespace="<window>" names that clash with variable names (#9867

)

For example:

  @JsMethod(namespace = "<window>")
  static native boolean isNaN();

  private void test() {
    boolean isNaN = isNaN();
  }

will incorrectly infer that the method isNaN and the variable isNaN
refer to the same program element and use the name assigned to the
variable for both.

Fixes #9573
Co-authored-by: Roberto Lublinerman <[email protected]>

dev/core/src/com/google/gwt/dev/javac/JsniReferenceResolver.java

@@ -258,8 +258,7 @@ public void endVisit(JsNameRef x, JsContext ctx) {
         // method description including actual signature) so that dispatch everywhere is consistent
         // with the one resolved here.
         ident = jsniRef.getResolvedReference();
-        JsNameRef newRef = new JsNameRef(x.getSourceInfo(), ident);
-        newRef.setQualifier(x.getQualifier());
+        JsNameRef newRef = new JsNameRef(x.getSourceInfo(), ident, x.getQualifier());
         ctx.replaceMe(newRef);
       }
       if (binding != null) {

dev/core/src/com/google/gwt/dev/jjs/impl/DefaultJsInteropExportsGenerator.java

@@ -79,8 +79,8 @@ public void exportMember(JMember x, JsExpression bridgeMethodOrAlias) {
     ensureProvideNamespace(x, null);
 
     // _.memberName = RHS
-    JsNameRef lhs = new JsNameRef(x.getSourceInfo(), x.getJsName());
-    lhs.setQualifier(globalTemp.makeRef(x.getSourceInfo()));
+    JsNameRef lhs = new JsNameRef(x.getSourceInfo(), x.getJsName(),
+            globalTemp.makeRef(x.getSourceInfo()));
     exportStmts.add(createAssignment(lhs, bridgeMethodOrAlias).makeStmt());
   }
 

dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java

@@ -2351,8 +2351,7 @@ private void generatePrototypeDefinitionAlias(JMethod method, JsName alias) {
     }
 
     private JsExprStmt outputDisplayName(JsNameRef function, JMethod method) {
-      JsNameRef displayName = new JsNameRef(function.getSourceInfo(), "displayName");
-      displayName.setQualifier(function);
+      JsNameRef displayName = new JsNameRef(function.getSourceInfo(), "displayName", function);
       String displayStringName = getDisplayName(method);
       JsStringLiteral displayMethodName =
           new JsStringLiteral(function.getSourceInfo(), displayStringName);
@@ -3133,7 +3132,19 @@ private JsName getIndexedFieldJsName(String indexedName) {
     return names.get(program.getIndexedField(indexedName));
   }
 
-  private static JsNameRef createGlobalQualifier(String qualifier, SourceInfo sourceInfo) {
-     return JsUtils.createQualifiedNameRef(JsInteropUtil.normalizeQualifier(qualifier), sourceInfo);
+  private JsNameRef createGlobalQualifier(String qualifier, SourceInfo sourceInfo) {
+    List<String> parts =
+        Lists.newArrayList(JsInteropUtil.normalizeQualifier(qualifier).split("\\."));
+    // Make the top level name unobfuscatable in the top scope, and remove from parts.
+    String topScopeName = parts.remove(0);
+    JsNameRef ref =
+        jsProgram.getScope().declareUnobfuscatableName(topScopeName).makeRef(sourceInfo);
+
+    for (String part : parts) {
+      // The rest can be left as a name ref to be resolved by JsSymbolResolver later.
+      ref = new JsNameRef(sourceInfo, part, ref);
+    }
+
+    return ref;
   }
 }

dev/core/src/com/google/gwt/dev/js/CoverageInstrumentor.java

@@ -127,8 +127,7 @@ private CoverageInstrumentor(JsProgram jsProgram, Multimap<String, Integer> inst
   }
 
   private void addBeforeUnloadListener(SourceInfo info) {
-    JsNameRef onbeforeunload = new JsNameRef(info, "onbeforeunload");
-    onbeforeunload.setQualifier(new JsNameRef(info, "window"));
+    JsNameRef onbeforeunload = new JsNameRef(info, "onbeforeunload", new JsNameRef(info, "window"));
     JsNameRef handler = onBeforeUnloadFnName.makeRef(info);
     JsBinaryOperation assignment = new JsBinaryOperation(info, JsBinaryOperator.ASG,
         onbeforeunload, handler);

dev/core/src/com/google/gwt/dev/js/JsUtils.java

@@ -158,9 +158,7 @@ public static JsExpression createQualifiedNameRef(
       SourceInfo info, JsExpression base, String... names) {
     JsExpression result = base;
     for (String name : names) {
-      JsNameRef nameRef = new JsNameRef(info, name);
-      nameRef.setQualifier(result);
-      result = nameRef;
+      result = new JsNameRef(info, name, result);
     }
     return result;
   }
@@ -284,8 +282,7 @@ private static JsExpression prepareArgumentsForApply(SourceInfo sourceInfo,
     }
 
     JsArrayLiteral argumentsArray = new JsArrayLiteral(sourceInfo, nonVarargsArguments);
-    JsNameRef argumentsConcat = new JsNameRef(sourceInfo,"concat");
-    argumentsConcat.setQualifier(argumentsArray);
+    JsNameRef argumentsConcat = new JsNameRef(sourceInfo,"concat", argumentsArray);
     return new JsInvocation(sourceInfo, argumentsConcat, varargsArgument);
   }
 

dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java

@@ -30,6 +30,11 @@ public JsNameRef(SourceInfo sourceInfo, JsName name) {
     this.name = name;
   }
 
+  public JsNameRef(SourceInfo sourceInfo, String ident, JsExpression qualifier) {
+    this(sourceInfo, ident);
+    setQualifier(qualifier);
+  }
+
   public JsNameRef(SourceInfo sourceInfo, String ident) {
     super(sourceInfo);
     this.ident = StringInterner.get().intern(ident);

user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java

@@ -43,7 +43,7 @@
 import jsinterop.annotations.JsType;
 
 /**
- * Tests Miscelaneous fixes.
+ * Tests Miscellaneous fixes.
  */
 public class CompilerMiscRegressionTest extends GWTTestCase {
 
@@ -67,11 +67,11 @@ native double minusAndDecrement(double val) /*-{
 
   /**
    * The array {@code map.get("one")[0]} gets normalized (by {@link ImplementCastsAndTypeChecks}) to
-   * {@code Cast.dynamicCast(map.get("one"), ...)[0]}. The expression resulting from dynamiCast
+   * {@code Cast.dynamicCast(map.get("one"), ...)[0]}. The expression resulting from dynamicCast
    * would have type Object and that would not be a valid type for an array access operation.
    */
   public void testOverridingReturnType() {
-    Map<String, String[]> map = new HashMap();
+    Map<String, String[]> map = new HashMap<>();
     map.put("one", new String[10]);
 
     map.get("one")[0] = "one";
@@ -429,11 +429,11 @@ private static native Object newArrayThroughCtorReference() /*-{
     return ctor();
   }-*/;
 
-  private static native boolean isNan(double number) /*-{
+  private static native boolean jsniIsNan(double number) /*-{
     return @Global::isNan(D)(number);
   }-*/;
 
-  private static native double getNan() /*-{
+  private static native double jsniGetNan() /*-{
     return @Global::Nan;
   }-*/;
 
@@ -453,8 +453,17 @@ public void testNativeConstructorJSNI() {
     assertTrue(nativeArray instanceof NativeArray);
     assertEquals(2, arrayLength(nativeArray));
     assertTrue(newArrayThroughCtorReference() instanceof NativeArray);
-    assertTrue(Double.isNaN(getNan()));
-    assertTrue(isNan(Double.NaN));
+    assertTrue(Double.isNaN(jsniGetNan()));
+    assertTrue(jsniIsNan(Double.NaN));
+  }
+
+  @JsMethod(namespace = "<window>", name = "isNaN")
+  public static native boolean isNaN(double number);
+
+  // Regression tests for issue #9573
+  public void testTopLevelNameClash() {
+    boolean isNaN = isNaN(Global.Nan);
+    assertTrue(isNaN);
   }
 
   @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Object")
@@ -466,7 +475,7 @@ final NativeObjectWithOverlay getThis() {
     }
   }
 
-  public void testOveralyDispatchOnNull() {
+  public void testOverlayDispatchOnNull() {
     // Define a variable where the compiler can not statically determine that it is actually null.
     NativeObjectWithOverlay objectWithOverlay =
         Math.random() > 1000 ? new NativeObjectWithOverlay() : null;