From f9028fe906e62c1519eef3944f7bb3292a2b9cfa Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 5 Dec 2023 11:24:35 -0600 Subject: [PATCH] Fix for namespace="" names that clash with variable names (#9867) For example: @JsMethod(namespace = "") 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 --- .../gwt/dev/javac/JsniReferenceResolver.java | 3 +-- .../DefaultJsInteropExportsGenerator.java | 4 +-- .../dev/jjs/impl/GenerateJavaScriptAST.java | 19 +++++++++++--- .../gwt/dev/js/CoverageInstrumentor.java | 3 +-- .../src/com/google/gwt/dev/js/JsUtils.java | 7 ++---- .../com/google/gwt/dev/js/ast/JsNameRef.java | 5 ++++ .../jjs/test/CompilerMiscRegressionTest.java | 25 +++++++++++++------ 7 files changed, 43 insertions(+), 23 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/javac/JsniReferenceResolver.java b/dev/core/src/com/google/gwt/dev/javac/JsniReferenceResolver.java index ae1b8975434..5b3e0488932 100644 --- a/dev/core/src/com/google/gwt/dev/javac/JsniReferenceResolver.java +++ b/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) { diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DefaultJsInteropExportsGenerator.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DefaultJsInteropExportsGenerator.java index 8cc5661b721..0c649ebe67d 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/DefaultJsInteropExportsGenerator.java +++ b/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()); } diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java index 6647795445e..5df3c5e0d65 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java +++ b/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 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; } } diff --git a/dev/core/src/com/google/gwt/dev/js/CoverageInstrumentor.java b/dev/core/src/com/google/gwt/dev/js/CoverageInstrumentor.java index ca7f0f9d6fb..f5c30eaa11e 100644 --- a/dev/core/src/com/google/gwt/dev/js/CoverageInstrumentor.java +++ b/dev/core/src/com/google/gwt/dev/js/CoverageInstrumentor.java @@ -127,8 +127,7 @@ private CoverageInstrumentor(JsProgram jsProgram, Multimap 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); diff --git a/dev/core/src/com/google/gwt/dev/js/JsUtils.java b/dev/core/src/com/google/gwt/dev/js/JsUtils.java index 6011d45ff5a..4e964d24c56 100644 --- a/dev/core/src/com/google/gwt/dev/js/JsUtils.java +++ b/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); } diff --git a/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java b/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java index 2c1512afa19..0fcee987eba 100644 --- a/dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java +++ b/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); diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java index 2811bd27a24..4ec71bcec9a 100644 --- a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java +++ b/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 map = new HashMap(); + Map 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 = "", 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;