Skip to content

Commit a5166e4

Browse files
tap-prodblickly
authored andcommitted
Automated g4 rollback of changelist 116101354.
*** Reason for rollback *** 500 or more targets failed to build *** Original change description *** Remove the `arguments` usage check. Users never really liked it, and the primary motivation for creating it is a non-issue now that 6a40212 is submitted. *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116103285
1 parent 5163652 commit a5166e4

File tree

5 files changed

+143
-0
lines changed

5 files changed

+143
-0
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.google.javascript.jscomp.ExtractPrototypeMemberDeclarations.Pattern;
3232
import com.google.javascript.jscomp.NodeTraversal.Callback;
3333
import com.google.javascript.jscomp.PassFactory.HotSwapPassFactory;
34+
import com.google.javascript.jscomp.lint.CheckArguments;
3435
import com.google.javascript.jscomp.lint.CheckDuplicateCase;
3536
import com.google.javascript.jscomp.lint.CheckEmptyStatements;
3637
import com.google.javascript.jscomp.lint.CheckEnums;
@@ -1557,6 +1558,7 @@ protected HotSwapCompilerPass create(AbstractCompiler compiler) {
15571558
@Override
15581559
protected HotSwapCompilerPass create(AbstractCompiler compiler) {
15591560
ImmutableList.Builder<Callback> callbacks = ImmutableList.<Callback>builder()
1561+
.add(new CheckArguments(compiler))
15601562
.add(new CheckEmptyStatements(compiler))
15611563
.add(new CheckEnums(compiler))
15621564
.add(new CheckInterfaces(compiler))

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.common.base.Preconditions;
2121
import com.google.common.collect.ImmutableMap;
2222
import com.google.common.collect.ImmutableSet;
23+
import com.google.javascript.jscomp.lint.CheckArguments;
2324
import com.google.javascript.jscomp.lint.CheckDuplicateCase;
2425
import com.google.javascript.jscomp.lint.CheckEmptyStatements;
2526
import com.google.javascript.jscomp.lint.CheckEnums;
@@ -467,6 +468,7 @@ public DiagnosticGroup forName(String name) {
467468
// provide optional suggestions.
468469
public static final DiagnosticGroup LINT_CHECKS =
469470
DiagnosticGroups.registerGroup("lintChecks", // undocumented
471+
CheckArguments.BAD_ARGUMENTS_USAGE,
470472
CheckEmptyStatements.USELESS_EMPTY_STATEMENT,
471473
CheckEnums.COMPUTED_PROP_NAME_IN_ENUM,
472474
CheckEnums.DUPLICATE_ENUM_VALUE,

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import com.google.common.collect.ImmutableList;
1919
import com.google.javascript.jscomp.NodeTraversal.Callback;
20+
import com.google.javascript.jscomp.lint.CheckArguments;
2021
import com.google.javascript.jscomp.lint.CheckDuplicateCase;
2122
import com.google.javascript.jscomp.lint.CheckEmptyStatements;
2223
import com.google.javascript.jscomp.lint.CheckEnums;
@@ -106,6 +107,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
106107
return new CombinedCompilerPass(
107108
compiler,
108109
ImmutableList.<Callback>of(
110+
new CheckArguments(compiler),
109111
new CheckDuplicateCase(compiler),
110112
new CheckEmptyStatements(compiler),
111113
new CheckEnums(compiler),
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright 2015 The Closure Compiler Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.google.javascript.jscomp.lint;
17+
18+
import com.google.javascript.jscomp.AbstractCompiler;
19+
import com.google.javascript.jscomp.CompilerPass;
20+
import com.google.javascript.jscomp.DiagnosticType;
21+
import com.google.javascript.jscomp.JSError;
22+
import com.google.javascript.jscomp.NodeTraversal;
23+
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
24+
import com.google.javascript.jscomp.Var;
25+
import com.google.javascript.rhino.Node;
26+
27+
28+
/**
29+
* Check for the 'arguments' object being used in ways which are unlikely to be optimized well,
30+
* and which we cannot transpile correctly.
31+
*/
32+
public final class CheckArguments extends AbstractPostOrderCallback implements CompilerPass {
33+
public static final DiagnosticType BAD_ARGUMENTS_USAGE =
34+
DiagnosticType.warning(
35+
"JSC_BAD_ARGUMENTS_USAGE",
36+
"This use of the 'arguments' object is discouraged. See "
37+
+ "https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60");
38+
39+
private final AbstractCompiler compiler;
40+
41+
public CheckArguments(AbstractCompiler compiler) {
42+
this.compiler = compiler;
43+
}
44+
45+
@Override
46+
public void process(Node externs, Node root) {
47+
NodeTraversal.traverseEs6(compiler, root, this);
48+
}
49+
50+
@Override
51+
public void visit(NodeTraversal t, Node n, Node parent) {
52+
if (!n.isName()) {
53+
return;
54+
}
55+
56+
Var var = t.getScope().getVar(n.getString());
57+
if (var != null && var.isArguments()) {
58+
checkArgumentsUsage(n, parent);
59+
}
60+
}
61+
62+
private void checkArgumentsUsage(Node arguments, Node parent) {
63+
if (parent.isSpread()
64+
|| (parent.isGetProp() && parent.matchesQualifiedName("arguments.length"))
65+
|| (parent.isForOf() && arguments == parent.getSecondChild())
66+
|| (parent.isGetElem() && arguments == parent.getFirstChild())) {
67+
// No warning.
68+
return;
69+
}
70+
71+
if (parent.isCall() && arguments == parent.getLastChild()) {
72+
Node callee = parent.getFirstChild();
73+
if (callee.isGetProp() && callee.getLastChild().getString().equals("apply")) {
74+
// Probably a call to Function.apply(), so don't warn.
75+
return;
76+
}
77+
}
78+
79+
report(arguments);
80+
}
81+
82+
private void report(Node arguments) {
83+
compiler.report(JSError.make(arguments, BAD_ARGUMENTS_USAGE));
84+
}
85+
}
86+
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Copyright 2015 The Closure Compiler Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.javascript.jscomp.lint;
18+
19+
import static com.google.javascript.jscomp.lint.CheckArguments.BAD_ARGUMENTS_USAGE;
20+
21+
import com.google.javascript.jscomp.Compiler;
22+
import com.google.javascript.jscomp.CompilerPass;
23+
import com.google.javascript.jscomp.Es6CompilerTestCase;
24+
25+
public final class CheckArgumentsTest extends Es6CompilerTestCase {
26+
@Override
27+
public CompilerPass getProcessor(Compiler compiler) {
28+
return new CheckArguments(compiler);
29+
}
30+
31+
public void testCheckArguments_noWarning() {
32+
testSameEs6("function f() { for (let a of arguments) {} }");
33+
testSameEs6("function f() { return [...arguments]; }");
34+
testSameEs6("function f() { return arguments[1]; }");
35+
testSameEs6("function f() { for (var i=0; i<arguments.length; i++) alert(arguments[i]); }");
36+
}
37+
38+
public void testCheckArguments_exceptionForApply() {
39+
testSameEs6("function f() { g.apply(this, arguments); }");
40+
testSameEs6("function f() { Function.prototype.call.apply(arguments); }");
41+
}
42+
43+
public void testCheckArguments_warning() {
44+
testWarning("function f() { g(arguments); }", BAD_ARGUMENTS_USAGE);
45+
testWarning("function f() { var args = arguments; }", BAD_ARGUMENTS_USAGE);
46+
testWarning("function f() { var args = [0, 1, arguments]; }", BAD_ARGUMENTS_USAGE);
47+
testWarning("function f() { arguments.caller; }", BAD_ARGUMENTS_USAGE);
48+
testWarning("function f() { arguments.callee; }", BAD_ARGUMENTS_USAGE);
49+
testWarning("function f() { for (var i in arguments) {} }", BAD_ARGUMENTS_USAGE);
50+
}
51+
}

0 commit comments

Comments
 (0)