Skip to content
Open
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
17 changes: 17 additions & 0 deletions src/com/google/javascript/jscomp/AstAnalyzer.java
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,14 @@ boolean functionCallHasSideEffects(Node callNode) {
if (assumeKnownBuiltinsArePure && BUILTIN_FUNCTIONS_WITHOUT_SIDEEFFECTS.contains(name)) {
return false;
}
// Array constructor with a single numeric length argument has no side effects
// This handles Array() called without the 'new' keyword
if (assumeKnownBuiltinsArePure && "Array".equals(name)) {
Node arg = callee.getNext();
if (arg != null && arg.isNumber()) {
return false;
}
}
} else if (callee.isGetProp() || callee.isOptChainGetProp()) {
if (callNode.hasOneChild()
&& assumeKnownBuiltinsArePure
Expand Down Expand Up @@ -523,6 +531,15 @@ boolean constructorCallHasSideEffects(Node newNode) {
}

Node nameNode = newNode.getFirstChild();
if (assumeKnownBuiltinsArePure && nameNode.isName() && "Array".equals(nameNode.getString())) {
// Array constructor with a single numeric length argument has no side effects
// Array(length) creates an empty array of the given length
Node arg = nameNode.getNext();
if (arg != null && arg.isNumber()) {
return false;
}
}

return !nameNode.isName()
|| !assumeKnownBuiltinsArePure
|| !CONSTRUCTORS_WITHOUT_SIDE_EFFECTS.contains(nameNode.getString());
Expand Down
4 changes: 4 additions & 0 deletions test/com/google/javascript/jscomp/AstAnalyzerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,10 @@ public static ImmutableList<AnalysisCase> cases() {
kase().js("new Array").token(NEW).expect(true),
kase().js("new Array(4)").token(NEW).expect(true),
kase().js("new Array('a', 'b', 'c')").token(NEW).expect(true),
kase().js("Array(10)").token(CALL).expect(false),
kase().js("Array(1,2,3)").token(CALL).expect(false),
kase().js("Array(10)").token(CALL).assumeBuiltinsPure(false).expect(true),
kase().js("Array(1,2,3)").token(CALL).assumeBuiltinsPure(false).expect(true),
kase().js("new SomeClassINeverHeardOf()").token(NEW).expect(true),

// Getters and setters - object rest and spread
Expand Down
14 changes: 14 additions & 0 deletions test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2285,4 +2285,18 @@ public void testSwitchCase() {
"function f() { switch(x) { default: return; case 1: return 5;bar()}}",
"function f() { switch(x) { default: return; case 1: return 5;}}");
}

@Test
public void testArrayConstructor() {
// Array constructor with length argument only should be eliminated when unused
fold("Array(10)", "");
fold("Array(5)", "");

// Array constructor with elements should also be eliminated
fold("Array(1,2,3)", "");

// Array constructor used in expression with side effects
fold("Array(10).push(5)", "Array(10).push(5)"); // push has side effects
fold("Array(10)[0] = 2", "Array(10)[0] = 2"); // assignment has side effects
}
}
Loading