Skip to content

Commit ad813c3

Browse files
Stephan202Error Prone Team
authored and
Error Prone Team
committed
Optimize VisitorState#getConstantExpression
By avoiding a second pass over the string. Fixes #4586 FUTURE_COPYBARA_INTEGRATE_REVIEW=#4586 from PicnicSupermarket:sschroevers/optimize-getConstantExpression 45f3401 PiperOrigin-RevId: 711440462
1 parent 85af056 commit ad813c3

File tree

8 files changed

+113
-23
lines changed

8 files changed

+113
-23
lines changed

check_api/pom.xml

+27
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,22 @@
181181
</jdkToolchain>
182182
</configuration>
183183
</execution>
184+
<execution>
185+
<id>java23</id>
186+
<goals>
187+
<goal>compile</goal>
188+
</goals>
189+
<configuration>
190+
<jdkToolchain>
191+
<version>23</version>
192+
</jdkToolchain>
193+
<compileSourceRoots>
194+
<compileSourceRoot>${basedir}/src/main/java23</compileSourceRoot>
195+
</compileSourceRoots>
196+
<!-- multiReleaseOutput requires setting release -->
197+
<outputDirectory>${project.build.outputDirectory}/META-INF/versions/23</outputDirectory>
198+
</configuration>
199+
</execution>
184200
<execution>
185201
<id>java24</id>
186202
<configuration>
@@ -197,6 +213,17 @@
197213
</executions>
198214

199215
</plugin>
216+
<plugin>
217+
<groupId>org.apache.maven.plugins</groupId>
218+
<artifactId>maven-jar-plugin</artifactId>
219+
<configuration>
220+
<archive>
221+
<manifestEntries>
222+
<Multi-Release>true</Multi-Release>
223+
</manifestEntries>
224+
</archive>
225+
</configuration>
226+
</plugin>
200227
</plugins>
201228
</build>
202229

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Copyright 2024 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;
18+
19+
import com.sun.tools.javac.util.Convert;
20+
import javax.lang.model.util.Elements;
21+
22+
/**
23+
* @see VisitorState#getConstantExpression(Object)
24+
*/
25+
final class ConstantStringExpressions {
26+
private ConstantStringExpressions() {}
27+
28+
static String toConstantStringExpression(Object value, Elements elements) {
29+
if (!(value instanceof CharSequence)) {
30+
return elements.getConstantExpression(value);
31+
}
32+
33+
// Don't escape single-quotes in string literals.
34+
CharSequence str = (CharSequence) value;
35+
StringBuilder sb = new StringBuilder("\"");
36+
for (int i = 0; i < str.length(); i++) {
37+
char c = str.charAt(i);
38+
if (c == '\'') {
39+
sb.append('\'');
40+
} else {
41+
sb.append(Convert.quote(c));
42+
}
43+
}
44+
return sb.append('"').toString();
45+
}
46+
}

check_api/src/main/java/com/google/errorprone/VisitorState.java

+4-20
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.common.base.Preconditions.checkArgument;
2020
import static com.google.common.base.Preconditions.checkNotNull;
21+
import static com.google.errorprone.ConstantStringExpressions.toConstantStringExpression;
2122
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
2223

2324
import com.google.common.annotations.VisibleForTesting;
@@ -757,27 +758,10 @@ private static final class SharedState {
757758

758759
/**
759760
* Returns the Java source code for a constant expression representing the given constant value.
760-
* Like {@link Elements#getConstantExpression}, but doesn't over-escape single quotes in strings.
761+
* Like {@link Elements#getConstantExpression}, but (a) before JDK 23, doesn't over-escape single
762+
* quotes in strings and (b) treats any {@link CharSequence} as a {@link String}.
761763
*/
762764
public String getConstantExpression(Object value) {
763-
String escaped = getElements().getConstantExpression(value);
764-
if (value instanceof String) {
765-
// Don't escape single-quotes in string literals
766-
StringBuilder sb = new StringBuilder();
767-
for (int i = 0; i < escaped.length(); i++) {
768-
char c = escaped.charAt(i);
769-
if (c == '\\' && i + 1 < escaped.length()) {
770-
char next = escaped.charAt(++i);
771-
if (next != '\'') {
772-
sb.append(c);
773-
}
774-
sb.append(next);
775-
} else {
776-
sb.append(c);
777-
}
778-
}
779-
return sb.toString();
780-
}
781-
return escaped;
765+
return toConstantStringExpression(value, this.getElements());
782766
}
783767
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright 2024 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;
18+
19+
import javax.lang.model.util.Elements;
20+
21+
/**
22+
* @see VisitorState#getConstantExpression(Object)
23+
*/
24+
final class ConstantStringExpressions {
25+
private ConstantStringExpressions() {}
26+
27+
static String toConstantStringExpression(Object value, Elements elements) {
28+
return elements.getConstantExpression(
29+
value instanceof CharSequence charSequence ? charSequence.toString() : value);
30+
}
31+
}

core/src/main/java/com/google/errorprone/bugpatterns/RobolectricShadowDirectlyOn.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
7373
MethodSymbol symbol = getSymbol(parent);
7474
String argReplacement =
7575
Streams.concat(
76-
Stream.of(state.getConstantExpression(symbol.getSimpleName().toString())),
76+
Stream.of(state.getConstantExpression(symbol.getSimpleName())),
7777
Streams.zip(
7878
symbol.getParameters().stream(),
7979
parent.getArguments().stream(),

core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerArgumentToString.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ Description unwrapArguments(
352352
if (!fixed) {
353353
return NO_MATCH;
354354
}
355-
fix.replace(tree.getArguments().get(0), state.getConstantExpression(sb.toString()));
355+
fix.replace(tree.getArguments().get(0), state.getConstantExpression(sb));
356356
return describeMatch(tree, fix.build());
357357
}
358358

core/src/main/java/com/google/errorprone/bugpatterns/flogger/FloggerStringConcatenation.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ protected Void defaultAction(Tree tree, Void unused) {
112112
tree,
113113
SuggestedFix.replace(
114114
argument,
115-
state.getConstantExpression(formatString.toString())
115+
state.getConstantExpression(formatString)
116116
+ ", "
117117
+ formatArguments.stream().map(state::getSourceForNode).collect(joining(", "))));
118118
}

core/src/test/java/com/google/errorprone/VisitorStateTest.java

+2
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ public void getConstantExpression() {
106106
assertThat(visitorState.getConstantExpression("hello \n world"))
107107
.isEqualTo("\"hello \\n world\"");
108108
assertThat(visitorState.getConstantExpression('\'')).isEqualTo("'\\''");
109+
assertThat(visitorState.getConstantExpression(new StringBuilder("hello ' world")))
110+
.isEqualTo("\"hello ' world\"");
109111
}
110112

111113
// The following is taken from ErrorProneJavacPluginTest. There may be an easier way.

0 commit comments

Comments
 (0)