Skip to content

Commit 796c5a7

Browse files
JohnLuck77copybara-github
authored andcommitted
Update AstAnalyzer to account for BigInt
Adding necessary test coverage as well PiperOrigin-RevId: 325345721
1 parent d2b66b8 commit 796c5a7

File tree

3 files changed

+35
-4
lines changed

3 files changed

+35
-4
lines changed

src/com/google/javascript/jscomp/AstAnalyzer.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ public class AstAnalyzer {
6060
// can also be called as constructors but lack side-effects.
6161
// TODO(johnlenz): consider adding an extern annotation for this.
6262
private static final ImmutableSet<String> BUILTIN_FUNCTIONS_WITHOUT_SIDEEFFECTS =
63-
ImmutableSet.of("Object", "Array", "String", "Number", "Boolean", "RegExp", "Error");
63+
ImmutableSet.of(
64+
"Object", "Array", "String", "Number", "BigInt", "Boolean", "RegExp", "Error");
6465
private static final ImmutableSet<String> OBJECT_METHODS_WITHOUT_SIDEEFFECTS =
6566
ImmutableSet.of("toString", "valueOf");
6667
private static final ImmutableSet<String> REGEXP_METHODS = ImmutableSet.of("test", "exec");
@@ -357,6 +358,7 @@ && checkForStateChangeHelper(member.getFirstChild(), checkForNewObjects)) {
357358
// Any context that supports DEFAULT_VALUE is already an assignment. The possiblity of a
358359
// default doesn't itself create a side-effect. Therefore, we prefer to defer the decision.
359360
case NUMBER:
361+
case BIGINT:
360362
case OR:
361363
case COALESCE:
362364
case THIS:

test/com/google/javascript/jscomp/AstAnalyzerTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static com.google.javascript.rhino.Token.ASSIGN;
2626
import static com.google.javascript.rhino.Token.ASSIGN_ADD;
2727
import static com.google.javascript.rhino.Token.AWAIT;
28+
import static com.google.javascript.rhino.Token.BIGINT;
2829
import static com.google.javascript.rhino.Token.BITOR;
2930
import static com.google.javascript.rhino.Token.CALL;
3031
import static com.google.javascript.rhino.Token.CLASS;
@@ -257,6 +258,7 @@ public static Iterable<AnalysisCase> cases() {
257258
kase().js("/abc/gi").token(REGEXP).expect(true),
258259
kase().js("'a'").token(STRING).expect(false),
259260
kase().js("0").token(NUMBER).expect(false),
261+
kase().js("1n").token(BIGINT).expect(false),
260262
kase().js("a + c").token(ADD).expect(false),
261263
kase().js("'c' + a[0]").token(ADD).expect(false),
262264
kase().js("a[0][1]").token(GETELEM).expect(false),
@@ -788,6 +790,35 @@ public void testCallSideEffects() {
788790
}
789791
}
790792

793+
@RunWith(Parameterized.class)
794+
public static class BuiltInFunctionWithoutSideEffects {
795+
796+
@Parameter(0)
797+
public String functionName;
798+
799+
@Parameters(name = "#{index} {0}")
800+
public static final ImmutableList<Object[]> cases() {
801+
return ImmutableList.copyOf(
802+
new Object[][] {
803+
{"Object"},
804+
{"Array"},
805+
{"String"},
806+
{"Number"},
807+
{"BigInt"},
808+
{"Boolean"},
809+
{"RegExp"},
810+
{"Error"}
811+
});
812+
}
813+
814+
@Test
815+
public void test() {
816+
ParseHelper parseHelper = new ParseHelper();
817+
Node func = parseHelper.parseFirst(CALL, SimpleFormat.format("%s(1);", functionName));
818+
assertThat(parseHelper.getAstAnalyzer().functionCallHasSideEffects(func)).isFalse();
819+
}
820+
}
821+
791822
@RunWith(JUnit4.class)
792823
public static class ConstructorCallHasSideEffects {
793824
@Test

test/com/google/javascript/jscomp/integration/AdvancedOptimizationsIntegrationTest.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,7 @@ public void testBigIntStringConcatenation() {
224224
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
225225
options.setLanguage(LanguageMode.ECMASCRIPT_NEXT);
226226
externs = ImmutableList.of(new TestExternsBuilder().addBigInt().buildExternsFile("externs.js"));
227-
// TODO(b/140132715): This test should expect "1nasdf", must update PeepholeFoldConstants
228-
// for this to work as intended
229-
test(options, "1n + 'asdf'", "1n + 'asdf'");
227+
test(options, "1n + 'asdf'", "'1nasdf'");
230228
}
231229

232230
@Test

0 commit comments

Comments
 (0)