Skip to content

Commit

Permalink
Fixed problems with hoisting
Browse files Browse the repository at this point in the history
Fix hoisting in functions within blocks. Ensure that function declarations in blocks are hoisted so they can be invoked "before" they are declared. Makes Rhino more compatible with the popular engines.

Co-authored-by: Andrea Bergia <[email protected]>
Co-authored-by: satish.srinivasan <[email protected]>
  • Loading branch information
andreabergia and 0xe authored Feb 4, 2025
1 parent b2d4911 commit 177b047
Show file tree
Hide file tree
Showing 4 changed files with 297 additions and 1 deletion.
18 changes: 17 additions & 1 deletion rhino/src/main/java/org/mozilla/javascript/IRFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,26 @@ private Node transformBlock(AstNode node) {
}
try {
List<Node> kids = new ArrayList<>();
/*
Function declarations inside blocks (FUNCTION_EXPRESSION_STATEMENTS) should be
hoisted at the top of block so that they can be referred by statements above them
*/
List<Node> functions = new ArrayList<>();

for (Node kid : node) {
kids.add(transform((AstNode) kid));
if (kid instanceof FunctionNode
&& ((FunctionNode) kid).getFunctionType()
== FunctionNode.FUNCTION_EXPRESSION_STATEMENT) {
functions.add(transform((AstNode) kid));
} else {
kids.add(transform((AstNode) kid));
}
}
node.removeChildren();

for (Node function : functions) {
node.addChildToBack(function);
}
for (Node kid : kids) {
node.addChildToBack(kid);
}
Expand Down
2 changes: 2 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/ast/AstNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,8 @@ public boolean visit(AstNode node) {
buffer.append(" ").append(((Name) node).getIdentifier());
} else if (tt == Token.STRING) {
buffer.append(" ").append(((StringLiteral) node).getValue(true));
} else if (tt == Token.FUNCTION) {
buffer.append(" functionType=").append(((FunctionNode) node).getFunctionType());
}
buffer.append("\n");
return true; // process kids
Expand Down
237 changes: 237 additions & 0 deletions rhino/src/test/java/org/mozilla/javascript/HoistingTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
package org.mozilla.javascript;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import org.mozilla.javascript.tests.Utils;

public class HoistingTest {
@Test
public void hoistedFunctionCallTryCatchShouldNotThrowReferenceError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; \n"
+ " try {\n"
+ " r = _add(2, 3);\n"
+ " function _add(a, b) { return a + b; }\n"
+ " } catch(err) {\n"
+ " throw err;\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
public void functionCallTryCatchExprStmt() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r = 1; \n"
+ " if (r == 1) {\n"
+ " function _add(a, b) { return a + b; };\n"
+ " return _add(5, 4);\n"
+ " }\n"
+ " return _add(2, 3);\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(9, script);
}

@Test
public void hoistedFunctionShouldShadowFunctionWithTheSameName() {
String script =
""
+ "function g() {\n"
+ " result = '';\n"
+ " result += f();\n"
+ "\n"
+ " function f() {\n"
+ " return 1;\n"
+ " }\n"
+ "\n"
+ " do {\n"
+ " result += f();\n"
+ "\n"
+ " function f() {\n"
+ " return 0;\n"
+ " }\n"
+ " } while (0);\n"
+ "\n"
+ " result += f();\n"
+ " return result;\n"
+ "}\n"
+ "\n"
+ "g()";
Utils.assertWithAllModes_ES6("100", script);
}

@Test
public void hoistedFunctionCallDoWhileShouldNotThrowReferenceError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; \n"
+ " do {\n"
+ " r = _add(2, 3);\n"
+ " function _add(a, b) { return a + b; }\n"
+ " } while(0) {\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
public void hoistedFunctionCallBlockShouldNotThrowReferenceError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; \n"
+ " {\n"
+ " r = _add(2, 3);\n"
+ " function _add(a, b) { return a + b; }\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
public void hoistedFunctionCallForLoopShouldNotThrowReferenceError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; \n"
+ " for (var i = 0; i<1; i++) {\n"
+ " r = _add(2, 3);\n"
+ " function _add(a, b) { return a + b; }\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
public void hoistedFunctionCallIfShouldNotThrowReferenceError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; var i = 0; \n"
+ " if (i<1) {\n"
+ " r = _add(2, 3);\n"
+ " function _add(a, b) { return a + b; }\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
@Ignore("switch-doesnt-open-a-block")
public void hoistedFunctionCallSwitchShouldNotThrowReferenceError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; var i = 0; \n"
+ " switch(i) {\n"
+ " case 0:"
+ " r = _add(2, 3);\n"
+ " default:\n"
+ " function _add(a, b) { return a + b; }\n"
+ " break;\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
public void hoistedFunctionCallShouldNotThrowUndefinedErrorNoNestedScope() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; \n"
+ " r = _add(2, 3);\n"
+ " function _add(a, b) { return a + b; }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertWithAllModes_ES6(5, script);
}

@Test
public void hoistedFunctionExpressionCallShouldThrowUndefinedError() {
String script =
""
+ "var result = test();\n"
+ "function test() {\n"
+ " var r; \n"
+ " try {\n"
+ " r = _add(2, 3);\n"
+ " var _add = function(a, b) { return a + b; }\n"
+ " } catch(err) {\n"
+ " throw err;\n"
+ " }\n"
+ " return r;\n"
+ "}\n"
+ "result";
Utils.assertJavaScriptException_ES6(
"TypeError: _add is not a function, it is undefined. (test#8)", script);
}

@Test
public void hoistedReferenceShouldNotThrowReferenceError() {
Utils.runWithAllModes(
(cx) -> {
int languageVersion = cx.getLanguageVersion();
try {
cx.setLanguageVersion(Context.VERSION_ES6);
Scriptable scope = cx.initStandardObjects();
String script =
""
+ "var arr_obj = [];\n"
+ "test();\n"
+ "function test() {\n"
+ "try {\n"
+ " arr_obj.push({test: \"a\", key: 6});\n"
+ " arr_obj.push({test: \"b\", key: 3});\n"
+ " arr_obj.push({test: \"c\", key: 5});\n"
+ " arr_obj.sort(_sortByKey);\n"
+ " function _sortByKey(a, b) {\n"
+ " return a.key - b.key;\n"
+ " }\n"
+ "} catch(err) {\n"
+ " throw err;\n"
+ "}\n"
+ "}\n"
+ "\n"
+ "arr_obj";
Object result = cx.evaluateString(scope, script, "hoistedRef", 1, null);
Assert.assertTrue(result instanceof NativeArray);
Assert.assertEquals(3, ((NativeArray) result).size());
} catch (Exception e) {
e.printStackTrace();
Assert.fail();
} finally {
cx.setLanguageVersion(languageVersion);
}
return null;
});
}
}
41 changes: 41 additions & 0 deletions testutils/src/main/java/org/mozilla/javascript/tests/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.mozilla.javascript.ContextFactory;
import org.mozilla.javascript.EcmaError;
import org.mozilla.javascript.EvaluatorException;
import org.mozilla.javascript.JavaScriptException;
import org.mozilla.javascript.Scriptable;
import org.mozilla.javascript.ScriptableObject;
import org.mozilla.javascript.TopLevel;
Expand Down Expand Up @@ -289,6 +290,46 @@ public static void assertEcmaError_1_8(final String expectedMessage, final Strin
assertException(Context.VERSION_1_8, EcmaError.class, expectedMessage, script);
}

/**
* Execute the provided script and assert an {@link org.mozilla.javascript.JavaScriptException}.
* The error message of the {@link org.mozilla.javascript.JavaScriptException} has to start with
* the provided expectedMessage.
*
* @param expectedMessage the expected result
* @param script the javascript script to execute
*/
public static void assertJavaScriptException(
final String expectedMessage, final String script) {
assertException(-1, JavaScriptException.class, expectedMessage, script);
}

/**
* Execute the provided script and assert an {@link JavaScriptException}. The error message of
* the {@link JavaScriptException} has to start with the provided expectedMessage. Before the
* execution the language version is set to {@link Context#VERSION_1_8}.
*
* @param expectedMessage the expected result
* @param script the javascript script to execute
*/
public static void assertJavaScriptException_1_8(
final String expectedMessage, final String script) {
assertException(Context.VERSION_1_8, JavaScriptException.class, expectedMessage, script);
}

/**
* Execute the provided script and assert an {@link org.mozilla.javascript.JavaScriptException}.
* The error message of the {@link org.mozilla.javascript.JavaScriptException} has to start with
* the provided expectedMessage. Before the execution the language version is set to {@link
* Context#VERSION_1_8}.
*
* @param expectedMessage the expected result
* @param script the javascript script to execute
*/
public static void assertJavaScriptException_ES6(
final String expectedMessage, final String script) {
assertException(Context.VERSION_ES6, JavaScriptException.class, expectedMessage, script);
}

/**
* Execute the provided script and assert an {@link EcmaError}. The error message of the {@link
* EcmaError} has to start with the provided expectedMessage. Before the execution the language
Expand Down

0 comments on commit 177b047

Please sign in to comment.