diff --git a/src/com/google/javascript/jscomp/AstAnalyzer.java b/src/com/google/javascript/jscomp/AstAnalyzer.java index f84d8a8d571..3f82717d6a4 100644 --- a/src/com/google/javascript/jscomp/AstAnalyzer.java +++ b/src/com/google/javascript/jscomp/AstAnalyzer.java @@ -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 @@ -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()); diff --git a/test/com/google/javascript/jscomp/AstAnalyzerTest.java b/test/com/google/javascript/jscomp/AstAnalyzerTest.java index 066f97f59c9..368b4657281 100644 --- a/test/com/google/javascript/jscomp/AstAnalyzerTest.java +++ b/test/com/google/javascript/jscomp/AstAnalyzerTest.java @@ -314,6 +314,10 @@ public static ImmutableList 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 diff --git a/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java b/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java index db5fe5af7e3..61577502ffb 100644 --- a/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java +++ b/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java @@ -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 + } }