Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}
3 changes: 1 addition & 2 deletions dev/core/src/com/google/gwt/dev/js/CoverageInstrumentor.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 2 additions & 5 deletions dev/core/src/com/google/gwt/dev/js/JsUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}

Expand Down
5 changes: 5 additions & 0 deletions dev/core/src/com/google/gwt/dev/js/ast/JsNameRef.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
import jsinterop.annotations.JsType;

/**
* Tests Miscelaneous fixes.
* Tests Miscellaneous fixes.
*/
public class CompilerMiscRegressionTest extends GWTTestCase {

Expand All @@ -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";
Expand Down Expand Up @@ -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;
}-*/;

Expand All @@ -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")
Expand All @@ -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;
Expand Down
Loading