From c7c6b8da86afdf6dd2fa22021438bc60da4777dc Mon Sep 17 00:00:00 2001 From: Roberto Lublinerman Date: Tue, 28 Nov 2017 16:10:07 -0800 Subject: [PATCH 1/4] Fix for namespace="" names that clash with variable names. 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. Bug: #9573 Bug-Link: https://github.com/gwtproject/gwt/issues/9573 Change-Id: I48eecdae468c6159b3f49ccf3fee4f64c1889d88 --- .../gwt/dev/jjs/impl/GenerateJavaScriptAST.java | 16 ++++++++++++++-- .../com/google/gwt/dev/js/ast/JsNameRef.java | 5 +++++ .../jjs/test/CompilerMiscRegressionTest.java | 17 +++++++++++++---- 3 files changed, 32 insertions(+), 6 deletions(-) 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 8a8b69f0766..7dbabd8ab76 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 @@ -3091,7 +3091,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/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 7f9d57e40e3..dc5e5b34d1a 100644 --- a/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java +++ b/user/test/com/google/gwt/dev/jjs/test/CompilerMiscRegressionTest.java @@ -428,11 +428,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; }-*/; @@ -452,7 +452,16 @@ 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); } } From 8b8fd6e96348026b7f523ea69f7fbd34fc7df020 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 15 Nov 2023 19:24:27 -0600 Subject: [PATCH 2/4] Apply new constructor to other locations that assign qualifier --- .../com/google/gwt/dev/javac/JsniReferenceResolver.java | 3 +-- .../gwt/dev/jjs/impl/DefaultJsInteropExportsGenerator.java | 3 +-- .../com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java | 3 +-- .../src/com/google/gwt/dev/js/CoverageInstrumentor.java | 3 +-- dev/core/src/com/google/gwt/dev/js/JsUtils.java | 7 ++----- 5 files changed, 6 insertions(+), 13 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..3ac95d3de23 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,7 @@ 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 74d1f167e88..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); 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); } From 154950068b625163613dd880ab760dfa252e1484 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 15 Nov 2023 19:27:50 -0600 Subject: [PATCH 3/4] Test cleanup --- .../gwt/dev/jjs/test/CompilerMiscRegressionTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 15a32b6ac33..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"; @@ -462,7 +462,7 @@ public void testNativeConstructorJSNI() { // Regression tests for issue #9573 public void testTopLevelNameClash() { - boolean isNaN = isNaN(Global.Nan); + boolean isNaN = isNaN(Global.Nan); assertTrue(isNaN); } @@ -475,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; From 02a43f1717ff50f5c13da2fd9059652464fc44a0 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 5 Dec 2023 09:59:27 -0600 Subject: [PATCH 4/4] fix style --- .../gwt/dev/jjs/impl/DefaultJsInteropExportsGenerator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 3ac95d3de23..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,7 +79,8 @@ public void exportMember(JMember x, JsExpression bridgeMethodOrAlias) { ensureProvideNamespace(x, null); // _.memberName = RHS - JsNameRef lhs = new JsNameRef(x.getSourceInfo(), x.getJsName(), globalTemp.makeRef(x.getSourceInfo())); + JsNameRef lhs = new JsNameRef(x.getSourceInfo(), x.getJsName(), + globalTemp.makeRef(x.getSourceInfo())); exportStmts.add(createAssignment(lhs, bridgeMethodOrAlias).makeStmt()); }