From 64ae44ee012ba008a9932b1de45474a2ba0ded97 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Wed, 16 Nov 2022 20:53:23 +0100 Subject: [PATCH] JS: store global object in a variable to avoid name clashes between generated declarations (in minified mode) and global declarations --- .../backend/javascript/JavaScriptTarget.java | 4 +- .../javascript/rendering/AstWriter.java | 52 +++-- .../rendering/DefaultGlobalNameWriter.java | 35 +++ .../javascript/rendering/RuntimeRenderer.java | 2 +- .../javascript/rendering/AstWriterTest.java | 206 +++++++++++------- .../org/teavm/jso/impl/JSBodyAstEmitter.java | 13 +- .../org/teavm/jso/impl/JSClassProcessor.java | 14 +- .../teavm/incremental/IncrementalTest.java | 1 + 8 files changed, 213 insertions(+), 114 deletions(-) create mode 100644 core/src/main/java/org/teavm/backend/javascript/rendering/DefaultGlobalNameWriter.java diff --git a/core/src/main/java/org/teavm/backend/javascript/JavaScriptTarget.java b/core/src/main/java/org/teavm/backend/javascript/JavaScriptTarget.java index e57d0de6b..47e6e16e4 100644 --- a/core/src/main/java/org/teavm/backend/javascript/JavaScriptTarget.java +++ b/core/src/main/java/org/teavm/backend/javascript/JavaScriptTarget.java @@ -464,11 +464,11 @@ public class JavaScriptTarget implements TeaVMTarget, TeaVMJavaScriptHost { for (String key : controller.getEntryPoints().keySet()) { writer.append("var ").append(key).append(";").softNewLine(); } - writer.append("(function()").ws().append("{").newLine(); + writer.append("(function($rt_globals)").ws().append("{").newLine(); } private void printWrapperEnd(SourceWriter writer) throws IOException { - writer.append("})();").newLine(); + writer.append("})(this);").newLine(); } private void printStats(Renderer renderer, int totalSize) { diff --git a/core/src/main/java/org/teavm/backend/javascript/rendering/AstWriter.java b/core/src/main/java/org/teavm/backend/javascript/rendering/AstWriter.java index 0cb41f7d8..dca9f05f3 100644 --- a/core/src/main/java/org/teavm/backend/javascript/rendering/AstWriter.java +++ b/core/src/main/java/org/teavm/backend/javascript/rendering/AstWriter.java @@ -16,12 +16,12 @@ package org.teavm.backend.javascript.rendering; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import org.mozilla.javascript.Node; import org.mozilla.javascript.ScriptRuntime; import org.mozilla.javascript.Token; @@ -92,13 +92,16 @@ public class AstWriter { public static final int PRECEDENCE_COMMA = 18; private SourceWriter writer; private Map nameMap = new HashMap<>(); + private boolean rootScope = true; private Set aliases = new HashSet<>(); + private Function globalNameWriter; - public AstWriter(SourceWriter writer) { + public AstWriter(SourceWriter writer, Function globalNameWriter) { this.writer = writer; + this.globalNameWriter = globalNameWriter; } - private void declareName(String name) { + public void declareName(String name) { if (nameMap.containsKey(name)) { return; } @@ -124,14 +127,25 @@ public class AstWriter { } public void hoist(AstNode node) { + declareName("arguments"); node.visit(n -> { if (n instanceof Scope) { - Scope scope = (Scope) n; + var scope = (Scope) n; if (scope.getSymbolTable() != null) { - for (String name : scope.getSymbolTable().keySet()) { + for (var name : scope.getSymbolTable().keySet()) { declareName(name); } } + } else if (n instanceof CatchClause) { + var clause = (CatchClause) n; + var name = clause.getVarName().getIdentifier(); + declareName(name); + } else if (n instanceof VariableInitializer) { + var initializer = (VariableInitializer) n; + if (initializer.getTarget() instanceof Name) { + var id = ((Name) initializer.getTarget()).getIdentifier(); + declareName(id); + } } return true; }); @@ -488,10 +502,10 @@ public class AstWriter { private void print(PropertyGet node) throws IOException { print(node.getLeft(), PRECEDENCE_MEMBER); writer.append('.'); - Map oldNameMap = nameMap; - nameMap = Collections.emptyMap(); + var oldRootScope = rootScope; + rootScope = false; print(node.getRight()); - nameMap = oldNameMap; + rootScope = oldRootScope; } private void print(FunctionCall node, int precedence) throws IOException { @@ -627,11 +641,19 @@ public class AstWriter { } private void print(Name node, int precedence) throws IOException { - NameEmitter alias = nameMap.get(node.getIdentifier()); - if (alias == null) { - alias = prec -> writer.append(node.getIdentifier()); + if (rootScope) { + var alias = nameMap.get(node.getIdentifier()); + if (alias == null) { + if (globalNameWriter != null) { + alias = globalNameWriter.apply(node.getIdentifier()); + } else { + alias = prec -> writer.append(node.getIdentifier()); + } + } + alias.emit(precedence); + } else { + writer.append(node.getIdentifier()); } - alias.emit(precedence); } private void print(RegExpLiteral node) throws IOException { @@ -662,10 +684,10 @@ public class AstWriter { } else if (node.isSetterMethod()) { writer.append("set "); } - Map oldNameMap = nameMap; - nameMap = Collections.emptyMap(); + var oldRootScope = rootScope; + rootScope = false; print(node.getLeft()); - nameMap = oldNameMap; + rootScope = oldRootScope; if (!node.isMethod()) { writer.ws().append(':').ws(); } diff --git a/core/src/main/java/org/teavm/backend/javascript/rendering/DefaultGlobalNameWriter.java b/core/src/main/java/org/teavm/backend/javascript/rendering/DefaultGlobalNameWriter.java new file mode 100644 index 000000000..50af72307 --- /dev/null +++ b/core/src/main/java/org/teavm/backend/javascript/rendering/DefaultGlobalNameWriter.java @@ -0,0 +1,35 @@ +/* + * Copyright 2022 Alexey Andreev. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.teavm.backend.javascript.rendering; + +import java.util.function.Function; +import org.teavm.backend.javascript.codegen.SourceWriter; + +public class DefaultGlobalNameWriter implements Function { + private SourceWriter writer; + + public DefaultGlobalNameWriter(SourceWriter writer) { + this.writer = writer; + } + + @Override + public NameEmitter apply(String s) { + if (s.startsWith("$rt_") || s.startsWith("Long_") || s.equals("Long")) { + return prec -> writer.append(s); + } + return prec -> writer.append("$rt_globals").append('.').append(s); + } +} diff --git a/core/src/main/java/org/teavm/backend/javascript/rendering/RuntimeRenderer.java b/core/src/main/java/org/teavm/backend/javascript/rendering/RuntimeRenderer.java index 1692275d1..a56be6e2e 100644 --- a/core/src/main/java/org/teavm/backend/javascript/rendering/RuntimeRenderer.java +++ b/core/src/main/java/org/teavm/backend/javascript/rendering/RuntimeRenderer.java @@ -87,7 +87,7 @@ public class RuntimeRenderer { AstRoot ast = parseRuntime(name); ast.visit(new StringConstantElimination()); new RuntimeAstTransformer(writer.getNaming()).accept(ast); - AstWriter astWriter = new AstWriter(writer); + var astWriter = new AstWriter(writer, new DefaultGlobalNameWriter(writer)); astWriter.hoist(ast); astWriter.print(ast); } diff --git a/core/src/test/java/org/teavm/backend/javascript/rendering/AstWriterTest.java b/core/src/test/java/org/teavm/backend/javascript/rendering/AstWriterTest.java index 78b397a08..579305192 100644 --- a/core/src/test/java/org/teavm/backend/javascript/rendering/AstWriterTest.java +++ b/core/src/test/java/org/teavm/backend/javascript/rendering/AstWriterTest.java @@ -15,14 +15,12 @@ */ package org.teavm.backend.javascript.rendering; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertEquals; import java.io.IOException; import java.io.StringReader; import org.junit.Test; import org.mozilla.javascript.CompilerEnvirons; import org.mozilla.javascript.Context; -import org.mozilla.javascript.ast.AstRoot; import org.teavm.backend.javascript.codegen.SourceWriter; import org.teavm.backend.javascript.codegen.SourceWriterBuilder; @@ -30,227 +28,269 @@ public class AstWriterTest { private StringBuilder sb = new StringBuilder(); private SourceWriter sourceWriter; private AstWriter writer; + private AstWriter writerWithGlobals; public AstWriterTest() { - SourceWriterBuilder builder = new SourceWriterBuilder(null); + var builder = new SourceWriterBuilder(null); builder.setMinified(true); sourceWriter = builder.build(sb); - writer = new AstWriter(sourceWriter); + writer = new AstWriter(sourceWriter, null); + writerWithGlobals = new AstWriter(sourceWriter, name -> prec -> sourceWriter.append("globals.").append(name)); } @Test public void writesReturn() throws IOException { - assertThat(transform("return x;"), is("return x;")); + assertEquals("return x;", transform("return x;")); } @Test public void writesEmptyReturn() throws IOException { - assertThat(transform("return;"), is("return;")); + assertEquals("return;", transform("return;")); } @Test public void writesThrow() throws IOException { - assertThat(transform("throw x;"), is("throw x;")); + assertEquals("throw x;", transform("throw x;")); } @Test public void writesBreak() throws IOException { - assertThat(transform("a: while (true) { break a; }"), is("a:while(true){break a;}")); + assertEquals("a:while(true){break a;}", transform("a: while (true) { break a; }")); } @Test public void writesEmptyBreak() throws IOException { - assertThat(transform("while(true) { break; }"), is("while(true){break;}")); + assertEquals("while(true){break;}", transform("while(true) { break; }")); } @Test public void writesContinue() throws IOException { - assertThat(transform("a: while (true) { continue a; }"), is("a:while(true){continue a;}")); + assertEquals("a:while(true){continue a;}", transform("a: while (true) { continue a; }")); } @Test public void writesEmptyContinue() throws IOException { - assertThat(transform("while(true) { continue; }"), is("while(true){continue;}")); + assertEquals("while(true){continue;}", transform("while(true) { continue; }")); } @Test public void writesBlock() throws IOException { - assertThat(transform("{ foo(); bar(); }"), is("{foo();bar();}")); + assertEquals("{foo();bar();}", transform("{ foo(); bar(); }")); } @Test public void writesTryCatch() throws IOException { - assertThat(transform("try { foo(); } catch (e) { alert(e); }"), is("try {foo();}catch(e){alert(e);}")); - assertThat(transform("try { foo(); } finally { close(); }"), is("try {foo();}finally {close();}")); + assertEquals("try {foo();}catch(e){alert(e);}", transform("try { foo(); } catch (e) { alert(e); }")); + assertEquals("try {foo();}finally {close();}", transform("try { foo(); } finally { close(); }")); } @Test public void writesFor() throws IOException { - assertThat(transform("for (var i = 0; i < array.length; ++i,++j) foo(array[i]);"), - is("for(var i=0;i b ? 1 : 0;"), is("return ab?1:0;")); - assertThat(transform("return a < b ? -1 : (a > b ? 1 : 0);"), is("return ab?1:0;")); - assertThat(transform("return (a < b ? x == y : x != y) ? 1 : 0;"), is("return (a y ? x : y) : z"), is("return ay?x:y):z;")); + assertEquals("return cond?1:0;", transform("return cond ? 1 : 0;")); + assertEquals("return ab?1:0;", transform("return a < b ? -1 : a > b ? 1 : 0;")); + assertEquals("return ab?1:0;", transform("return a < b ? -1 : (a > b ? 1 : 0);")); + assertEquals("return (ay?x:y):z;", transform("return a < b ? (x > y ? x : y) : z")); } @Test public void writesRegExp() throws IOException { - assertThat(transform("return /[a-z]+/.match(text);"), is("return /[a-z]+/.match(text);")); - assertThat(transform("return /[a-z]+/ig.match(text);"), is("return /[a-z]+/ig.match(text);")); + assertEquals("return /[a-z]+/.match(text);", transform("return /[a-z]+/.match(text);")); + assertEquals("return /[a-z]+/ig.match(text);", transform("return /[a-z]+/ig.match(text);")); } @Test public void writesArrayLiteral() throws IOException { - assertThat(transform("return [];"), is("return [];")); - assertThat(transform("return [a, b + c];"), is("return [a,b+c];")); + assertEquals("return [];", transform("return [];")); + assertEquals("return [a,b+c];", transform("return [a, b + c];")); } @Test public void writesObjectLiteral() throws IOException { - assertThat(transform("return {};"), is("return {};")); - assertThat(transform("return { foo : bar };"), is("return {foo:bar};")); - assertThat(transform("return { foo : bar };"), is("return {foo:bar};")); - assertThat(transform("return { _foo : bar, get foo() { return this._foo; } };"), - is("return {_foo:bar,get foo(){return this._foo;}};")); + assertEquals("return {};", transform("return {};")); + assertEquals("return {foo:bar};", transform("return { foo : bar };")); + assertEquals("return {foo:bar};", transform("return { foo : bar };")); + assertEquals( + "return {_foo:bar,get foo(){return this._foo;}};", + transform("return { _foo : bar, get foo() { return this._foo; } };") + ); } @Test public void writesFunction() throws IOException { - assertThat(transform("return function f(x, y) { return x + y; };"), - is("return function f(x,y){return x+y;};")); + assertEquals( + "return function f(x,y){return x+y;};", + transform("return function f(x, y) { return x + y; };") + ); } @Test public void writesUnary() throws IOException { - assertThat(transform("return -a;"), is("return -a;")); - assertThat(transform("return -(a + b);"), is("return -(a+b);")); - assertThat(transform("return -a + b;"), is("return -a+b;")); - assertThat(transform("return (-a) + b;"), is("return -a+b;")); - assertThat(transform("return (-f)(x);"), is("return ( -f)(x);")); - assertThat(transform("return typeof a;"), is("return typeof a;")); + assertEquals("return -a;", transform("return -a;")); + assertEquals("return -(a+b);", transform("return -(a + b);")); + assertEquals("return -a+b;", transform("return -a + b;")); + assertEquals("return -a+b;", transform("return (-a) + b;")); + assertEquals("return ( -f)(x);", transform("return (-f)(x);")); + assertEquals("return typeof a;", transform("return typeof a;")); } @Test public void writesPostfix() throws IOException { - assertThat(transform("return a++;"), is("return a++;")); + assertEquals("return a++;", transform("return a++;")); } @Test public void respectsPrecedence() throws IOException { - assertThat(transform("return a + b + c;"), is("return a+b+c;")); - assertThat(transform("return (a + b) + c;"), is("return a+b+c;")); - assertThat(transform("return a + (b + c);"), is("return a+b+c;")); - assertThat(transform("return a - b + c;"), is("return a -b+c;")); - assertThat(transform("return (a - b) + c;"), is("return a -b+c;")); - assertThat(transform("return a - (b + c);"), is("return a -(b+c);")); + assertEquals("return a+b+c;", transform("return a + b + c;")); + assertEquals("return a+b+c;", transform("return (a + b) + c;")); + assertEquals("return a+b+c;", transform("return a + (b + c);")); + assertEquals("return a -b+c;", transform("return a - b + c;")); + assertEquals("return a -b+c;", transform("return (a - b) + c;")); + assertEquals("return a -(b+c);", transform("return a - (b + c);")); } @Test public void writesDelete() throws IOException { - assertThat(transform("delete a.b;"), is("delete a.b;")); + assertEquals("delete a.b;", transform("delete a.b;")); + } + + @Test + public void writesGlobalRef() throws IOException { + assertEquals( + "function(x){return x+globals.y;}", + transformGlobals("function(x) { return x + y; }") + ); + assertEquals( + "try {globals.foo();}catch(x){globals.foo(x);}", + transformGlobals("try { foo(); } catch (x) { foo(x); }") + ); + assertEquals( + "for(var i=0;i<10;++i){globals.foo(i,globals.j);}", + transformGlobals("for (var i = 0; i < 10; ++i) { foo(i, j); }") + ); + assertEquals( + "function(){var x=0;globals.foo(x,globals.y);}", + transformGlobals("function() { var x = 0; foo(x, y); }") + ); } private String transform(String text) throws IOException { + return transform(text, writer); + } + + private String transformGlobals(String text) throws IOException { + return transform(text, writerWithGlobals); + } + + private String transform(String text, AstWriter writer) throws IOException { sb.setLength(0); - CompilerEnvirons env = new CompilerEnvirons(); + var env = new CompilerEnvirons(); env.setRecoverFromErrors(true); env.setLanguageVersion(Context.VERSION_1_8); - JSParser factory = new JSParser(env); + var factory = new JSParser(env); factory.enterFunction(); - AstRoot rootNode = factory.parse(new StringReader(text), null, 0); + var rootNode = factory.parse(new StringReader(text), null, 0); factory.exitFunction(); writer.hoist(rootNode); writer.print(rootNode); diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSBodyAstEmitter.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSBodyAstEmitter.java index b75c176ad..022b6473c 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSBodyAstEmitter.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSBodyAstEmitter.java @@ -21,6 +21,7 @@ import org.mozilla.javascript.ast.AstNode; import org.mozilla.javascript.ast.Block; import org.teavm.backend.javascript.codegen.SourceWriter; import org.teavm.backend.javascript.rendering.AstWriter; +import org.teavm.backend.javascript.rendering.DefaultGlobalNameWriter; import org.teavm.backend.javascript.rendering.Precedence; import org.teavm.backend.javascript.spi.GeneratorContext; import org.teavm.backend.javascript.spi.InjectorContext; @@ -29,17 +30,19 @@ import org.teavm.model.MethodReference; class JSBodyAstEmitter implements JSBodyEmitter { private boolean isStatic; private AstNode ast; + private AstNode rootAst; private String[] parameterNames; - JSBodyAstEmitter(boolean isStatic, AstNode ast, String[] parameterNames) { + JSBodyAstEmitter(boolean isStatic, AstNode ast, AstNode rootAst, String[] parameterNames) { this.isStatic = isStatic; this.ast = ast; + this.rootAst = rootAst; this.parameterNames = parameterNames; } @Override public void emit(InjectorContext context) throws IOException { - AstWriter astWriter = new AstWriter(context.getWriter()); + var astWriter = new AstWriter(context.getWriter(), new DefaultGlobalNameWriter(context.getWriter())); int paramIndex = 0; if (!isStatic) { int index = paramIndex++; @@ -51,7 +54,7 @@ class JSBodyAstEmitter implements JSBodyEmitter { astWriter.declareNameEmitter(parameterNames[i], prec -> context.writeExpr(context.getArgument(index), convert(prec))); } - astWriter.hoist(ast); + astWriter.hoist(rootAst); astWriter.print(ast, convert(context.getPrecedence())); } @@ -139,7 +142,7 @@ class JSBodyAstEmitter implements JSBodyEmitter { @Override public void emit(GeneratorContext context, SourceWriter writer, MethodReference methodRef) throws IOException { - AstWriter astWriter = new AstWriter(writer); + var astWriter = new AstWriter(writer, new DefaultGlobalNameWriter(writer)); int paramIndex = 1; if (!isStatic) { int index = paramIndex++; @@ -149,7 +152,7 @@ class JSBodyAstEmitter implements JSBodyEmitter { int index = paramIndex++; astWriter.declareNameEmitter(parameterNames[i], prec -> writer.append(context.getParameterName(index))); } - astWriter.hoist(ast); + astWriter.hoist(rootAst); if (ast instanceof Block) { for (Node child = ast.getFirstChild(); child != null; child = child.getNext()) { astWriter.print((AstNode) child); diff --git a/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java b/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java index ff6b25bd9..b52ed6e4d 100644 --- a/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java +++ b/jso/impl/src/main/java/org/teavm/jso/impl/JSClassProcessor.java @@ -26,7 +26,6 @@ import java.util.Objects; import java.util.Set; import org.mozilla.javascript.CompilerEnvirons; import org.mozilla.javascript.Context; -import org.mozilla.javascript.ast.AstNode; import org.mozilla.javascript.ast.AstRoot; import org.mozilla.javascript.ast.FunctionNode; import org.teavm.backend.javascript.rendering.JSParser; @@ -556,27 +555,26 @@ class JSClassProcessor { .toArray(String[]::new) : new String[0]; // Parse JS script - TeaVMErrorReporter errorReporter = new TeaVMErrorReporter(diagnostics, - new CallLocation(methodToProcess.getReference())); - CompilerEnvirons env = new CompilerEnvirons(); + var errorReporter = new TeaVMErrorReporter(diagnostics, new CallLocation(methodToProcess.getReference())); + var env = new CompilerEnvirons(); env.setRecoverFromErrors(true); env.setLanguageVersion(Context.VERSION_1_8); env.setIdeMode(true); - JSParser parser = new JSParser(env, errorReporter); + var parser = new JSParser(env, errorReporter); AstRoot rootNode; try { rootNode = (AstRoot) parser.parseAsObject(new StringReader("function(){" + script + "}"), null, 0); } catch (IOException e) { throw new RuntimeException("IO Error occurred", e); } - AstNode body = ((FunctionNode) rootNode.getFirstChild()).getBody(); + var body = ((FunctionNode) rootNode.getFirstChild()).getBody(); repository.methodMap.put(methodToProcess.getReference(), proxyMethod); if (errorReporter.hasErrors()) { repository.emitters.put(proxyMethod, new JSBodyBloatedEmitter(isStatic, proxyMethod, script, parameterNames)); } else { - AstNode expr = JSBodyInlineUtil.isSuitableForInlining(methodToProcess.getReference(), + var expr = JSBodyInlineUtil.isSuitableForInlining(methodToProcess.getReference(), parameterNames, body); if (expr != null) { repository.inlineMethods.add(methodToProcess.getReference()); @@ -584,7 +582,7 @@ class JSClassProcessor { expr = body; } javaInvocationProcessor.process(location, expr); - repository.emitters.put(proxyMethod, new JSBodyAstEmitter(isStatic, expr, parameterNames)); + repository.emitters.put(proxyMethod, new JSBodyAstEmitter(isStatic, expr, rootNode, parameterNames)); } } diff --git a/tests/src/test/java/org/teavm/incremental/IncrementalTest.java b/tests/src/test/java/org/teavm/incremental/IncrementalTest.java index b42659fec..e0ca53752 100644 --- a/tests/src/test/java/org/teavm/incremental/IncrementalTest.java +++ b/tests/src/test/java/org/teavm/incremental/IncrementalTest.java @@ -164,6 +164,7 @@ public class IncrementalTest { private String runScript(String script, String fileName) { Scriptable scope = new NativeObject(); scope.setParentScope(rhinoRootScope); + scope.setPrototype(rhinoRootScope); rhinoContext.evaluateString(scope, script, fileName, 1, null); Function main = (Function) scope.get("main", scope); ScriptRuntime.doTopCall(main, rhinoContext, scope, scope,