Skip to content

Commit 0b751cd

Browse files
graememorganError Prone Team
authored andcommitted
ExpicitArrayForVarargs: flag unnecessary explicit construction of an array to provide varargs.
This is definitely one of those "hard to be sure overloads won't cause pain" checks. Flume: http://unknown commit PiperOrigin-RevId: 808102926
1 parent 982fe20 commit 0b751cd

File tree

3 files changed

+173
-0
lines changed

3 files changed

+173
-0
lines changed
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright 2025 The Error Prone 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.errorprone.bugpatterns;
18+
19+
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
20+
import static com.google.errorprone.matchers.Description.NO_MATCH;
21+
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
22+
import static com.google.errorprone.util.ASTHelpers.getSymbol;
23+
import static com.google.errorprone.util.ASTHelpers.getType;
24+
25+
import com.google.errorprone.BugPattern;
26+
import com.google.errorprone.VisitorState;
27+
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
28+
import com.google.errorprone.bugpatterns.BugChecker.NewClassTreeMatcher;
29+
import com.google.errorprone.fixes.SuggestedFix;
30+
import com.google.errorprone.fixes.SuggestedFixes;
31+
import com.google.errorprone.matchers.Description;
32+
import com.sun.source.tree.ExpressionTree;
33+
import com.sun.source.tree.MethodInvocationTree;
34+
import com.sun.source.tree.NewArrayTree;
35+
import com.sun.source.tree.NewClassTree;
36+
import com.sun.source.tree.Tree;
37+
import com.sun.tools.javac.code.Symbol.MethodSymbol;
38+
import com.sun.tools.javac.code.Type.ArrayType;
39+
import java.util.List;
40+
41+
/** A {@link BugChecker}; see the summary. */
42+
@BugPattern(summary = "Avoid explicit array creation for varargs", severity = WARNING)
43+
public final class ExplicitArrayForVarargs extends BugChecker
44+
implements MethodInvocationTreeMatcher, NewClassTreeMatcher {
45+
@Override
46+
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
47+
return handle(tree, getSymbol(tree), tree.getArguments(), state);
48+
}
49+
50+
@Override
51+
public Description matchNewClass(NewClassTree tree, VisitorState state) {
52+
return handle(tree, getSymbol(tree), tree.getArguments(), state);
53+
}
54+
55+
private Description handle(
56+
Tree tree, MethodSymbol symbol, List<? extends ExpressionTree> args, VisitorState state) {
57+
if (!symbol.isVarArgs()) {
58+
return NO_MATCH;
59+
}
60+
if (args.isEmpty()) {
61+
return NO_MATCH;
62+
}
63+
// The last argument isn't substituting for varargs if it isn't in place of the varargs
64+
// parameter.
65+
if (args.size() != symbol.getParameters().size()) {
66+
return NO_MATCH;
67+
}
68+
if (!(args.get(args.size() - 1) instanceof NewArrayTree newArrayTree)) {
69+
return NO_MATCH;
70+
}
71+
// Bail out if we're constructing a multidimensional array.
72+
if (((ArrayType) getType(newArrayTree)).getComponentType() instanceof ArrayType) {
73+
return NO_MATCH;
74+
}
75+
var initializers = newArrayTree.getInitializers();
76+
var fix =
77+
initializers.isEmpty()
78+
? SuggestedFixes.removeElement(newArrayTree, args, state)
79+
: SuggestedFix.builder()
80+
.replace(getStartPosition(newArrayTree), getStartPosition(initializers.get(0)), "")
81+
.replace(
82+
state.getEndPosition(
83+
newArrayTree.getInitializers().get(initializers.size() - 1)),
84+
state.getEndPosition(newArrayTree),
85+
"")
86+
.build();
87+
return describeMatch(tree, fix);
88+
}
89+
}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@
143143
import com.google.errorprone.bugpatterns.ErroneousThreadPoolConstructorChecker;
144144
import com.google.errorprone.bugpatterns.ExpectedExceptionChecker;
145145
import com.google.errorprone.bugpatterns.ExpensiveLenientFormatString;
146+
import com.google.errorprone.bugpatterns.ExplicitArrayForVarargs;
146147
import com.google.errorprone.bugpatterns.ExtendingJUnitAssert;
147148
import com.google.errorprone.bugpatterns.ExtendsAutoValue;
148149
import com.google.errorprone.bugpatterns.FallThrough;
@@ -1212,6 +1213,7 @@ public static ScannerSupplier warningChecks() {
12121213
EqualsBrokenForNull.class,
12131214
EqualsMissingNullable.class,
12141215
ExpectedExceptionChecker.class,
1216+
ExplicitArrayForVarargs.class,
12151217
ExtendsAutoValue.class,
12161218
FieldCanBeFinal.class,
12171219
FieldCanBeLocal.class,
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright 2025 The Error Prone 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.errorprone.bugpatterns;
18+
19+
import com.google.errorprone.CompilationTestHelper;
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.junit.runners.JUnit4;
23+
24+
@RunWith(JUnit4.class)
25+
public final class ExplicitArrayForVarargsTest {
26+
private final CompilationTestHelper helper =
27+
CompilationTestHelper.newInstance(ExplicitArrayForVarargs.class, getClass());
28+
29+
@Test
30+
public void positive() {
31+
helper
32+
.addSourceLines(
33+
"Test.java",
34+
"""
35+
class Test {
36+
void test() {
37+
// BUG: Diagnostic contains: String.format("foo %s", "foo")
38+
String a = String.format("foo %s", new Object[] {"foo"});
39+
// BUG: Diagnostic contains: String.format("foo %s")
40+
String b = String.format("foo %s", new Object[] {});
41+
}
42+
}
43+
""")
44+
.doTest();
45+
}
46+
47+
@Test
48+
public void multidimensionalArray() {
49+
helper
50+
.addSourceLines(
51+
"Test.java",
52+
"""
53+
import java.util.Arrays;
54+
import java.util.List;
55+
56+
class Test {
57+
List<Object[]> test() {
58+
return Arrays.asList(new Object[][] {{1, 2}, {3, 4}});
59+
}
60+
}
61+
""")
62+
.doTest();
63+
}
64+
65+
@Test
66+
public void notFinalArgument() {
67+
helper
68+
.addSourceLines(
69+
"Test.java",
70+
"""
71+
import java.util.Arrays;
72+
import java.util.List;
73+
74+
class Test {
75+
List<Object[]> test() {
76+
return Arrays.asList(new Object[] {1, 2}, new Object[] {3, 4});
77+
}
78+
}
79+
""")
80+
.doTest();
81+
}
82+
}

0 commit comments

Comments
 (0)