Skip to content

Commit 10ae8a1

Browse files
nick-someoneError Prone Team
nick-someone
authored and
Error Prone Team
committed
Remove DISALLOW_ARGUMENT_REUSE flag for InlineMe - we won't support re-using a
method argument more than once in an inlined function. RELNOTES=n/a PiperOrigin-RevId: 390460434
1 parent 41ee638 commit 10ae8a1

File tree

5 files changed

+11
-127
lines changed

5 files changed

+11
-127
lines changed

‎core/src/main/java/com/google/errorprone/bugpatterns/inlineme/InlinabilityResult.java

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@
5555
@AutoValue
5656
abstract class InlinabilityResult {
5757

58-
static final String DISALLOW_ARGUMENT_REUSE = "InlineMe:DisallowArgumentReuse";
59-
6058
abstract @Nullable InlineValidationErrorReason error();
6159

6260
abstract @Nullable ExpressionTree body();
@@ -138,8 +136,7 @@ String getErrorMessage() {
138136
}
139137
}
140138

141-
static InlinabilityResult forMethod(
142-
MethodTree tree, VisitorState state, boolean checkForArgumentReuse) {
139+
static InlinabilityResult forMethod(MethodTree tree, VisitorState state) {
143140
if (tree.getBody() == null) {
144141
return fromError(InlineValidationErrorReason.NO_BODY);
145142
}
@@ -187,12 +184,10 @@ static InlinabilityResult forMethod(
187184
return fromError(InlineValidationErrorReason.COMPLEX_STATEMENT, body);
188185
}
189186

190-
if (checkForArgumentReuse) {
191-
Symbol usedMultipledTimes = usesVariablesMultipleTimes(body, methSymbol.params(), state);
192-
if (usedMultipledTimes != null) {
193-
return fromError(
194-
InlineValidationErrorReason.REUSE_OF_ARGUMENTS, body, usedMultipledTimes.toString());
195-
}
187+
Symbol usedMultipledTimes = usesVariablesMultipleTimes(body, methSymbol.params(), state);
188+
if (usedMultipledTimes != null) {
189+
return fromError(
190+
InlineValidationErrorReason.REUSE_OF_ARGUMENTS, body, usedMultipledTimes.toString());
196191
}
197192

198193
Tree privateOrDeprecatedApi = usesPrivateOrDeprecatedApis(body, state);

‎core/src/main/java/com/google/errorprone/bugpatterns/inlineme/Suggester.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
2121

2222
import com.google.errorprone.BugPattern;
23-
import com.google.errorprone.ErrorProneFlags;
2423
import com.google.errorprone.VisitorState;
2524
import com.google.errorprone.annotations.DoNotCall;
2625
import com.google.errorprone.annotations.InlineMe;
@@ -42,13 +41,6 @@
4241
severity = WARNING)
4342
public final class Suggester extends BugChecker implements MethodTreeMatcher {
4443

45-
private final boolean checkForArgumentReuse;
46-
47-
public Suggester(ErrorProneFlags flags) {
48-
checkForArgumentReuse =
49-
flags.getBoolean(InlinabilityResult.DISALLOW_ARGUMENT_REUSE).orElse(true);
50-
}
51-
5244
@Override
5345
public Description matchMethod(MethodTree tree, VisitorState state) {
5446
// only suggest @InlineMe on @Deprecated APIs
@@ -67,8 +59,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
6759
}
6860

6961
// if the body is not inlineable, then return no match
70-
InlinabilityResult inlinabilityResult =
71-
InlinabilityResult.forMethod(tree, state, checkForArgumentReuse);
62+
InlinabilityResult inlinabilityResult = InlinabilityResult.forMethod(tree, state);
7263
if (!inlinabilityResult.isValidForSuggester()) {
7364
return Description.NO_MATCH;
7465
}

‎core/src/main/java/com/google/errorprone/bugpatterns/inlineme/Validator.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import com.google.common.collect.ImmutableList;
2424
import com.google.common.collect.ImmutableSet;
2525
import com.google.errorprone.BugPattern;
26-
import com.google.errorprone.ErrorProneFlags;
2726
import com.google.errorprone.VisitorState;
2827
import com.google.errorprone.annotations.InlineMe;
2928
import com.google.errorprone.annotations.InlineMeValidationDisabled;
@@ -50,12 +49,6 @@
5049
documentSuppression = false,
5150
severity = ERROR)
5251
public final class Validator extends BugChecker implements MethodTreeMatcher {
53-
private final boolean checkForArgumentReuse;
54-
55-
public Validator(ErrorProneFlags flags) {
56-
checkForArgumentReuse =
57-
flags.getBoolean(InlinabilityResult.DISALLOW_ARGUMENT_REUSE).orElse(true);
58-
}
5952

6053
@Override
6154
public Description matchMethod(MethodTree tree, VisitorState state) {
@@ -67,7 +60,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
6760
return Description.NO_MATCH;
6861
}
6962

70-
InlinabilityResult result = InlinabilityResult.forMethod(tree, state, checkForArgumentReuse);
63+
InlinabilityResult result = InlinabilityResult.forMethod(tree, state);
7164
if (!result.isValidForValidator()) {
7265
return buildDescription(tree)
7366
.setMessage(result.errorMessage())

‎core/src/test/java/com/google/errorprone/bugpatterns/inlineme/InlinerTest.java

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,99 +1029,6 @@ public void testOrderOfOperationsWithTrailingOperand() {
10291029
.doTest();
10301030
}
10311031

1032-
@Test
1033-
public void testSublist() {
1034-
refactoringTestHelper
1035-
.setArgs("-XepOpt:" + InlinabilityResult.DISALLOW_ARGUMENT_REUSE + "=false")
1036-
.addInputLines(
1037-
"Client.java",
1038-
"import com.google.errorprone.annotations.InlineMe;",
1039-
"import java.util.List;",
1040-
"public final class Client {",
1041-
" @Deprecated",
1042-
" @InlineMe(replacement = \"list.subList(list.size() - n, list.size())\")",
1043-
" public List<String> last(List<String> list, int n) {",
1044-
" return list.subList(list.size() - n, list.size());",
1045-
" }",
1046-
"}")
1047-
.expectUnchanged()
1048-
.addInputLines(
1049-
"Caller.java",
1050-
"import java.util.ArrayList;",
1051-
"import java.util.List;",
1052-
"public final class Caller {",
1053-
" public void doTest() {",
1054-
" List<String> list = new ArrayList<>();",
1055-
" list.add(\"hi\");",
1056-
" Client client = new Client();",
1057-
" List<String> result = client.last(list, 1);",
1058-
" }",
1059-
"}")
1060-
.addOutputLines(
1061-
"out/Caller.java",
1062-
"import java.util.ArrayList;",
1063-
"import java.util.List;",
1064-
"public final class Caller {",
1065-
" public void doTest() {",
1066-
" List<String> list = new ArrayList<>();",
1067-
" list.add(\"hi\");",
1068-
" Client client = new Client();",
1069-
" List<String> result = list.subList(list.size() - 1, list.size());",
1070-
" }",
1071-
"}")
1072-
.doTest();
1073-
}
1074-
1075-
@Test
1076-
public void testSublistPassingMethod() {
1077-
refactoringTestHelper
1078-
.setArgs("-XepOpt:" + InlinabilityResult.DISALLOW_ARGUMENT_REUSE + "=false")
1079-
.addInputLines(
1080-
"Client.java",
1081-
"import com.google.errorprone.annotations.InlineMe;",
1082-
"import java.util.List;",
1083-
"public final class Client {",
1084-
" @Deprecated",
1085-
" @InlineMe(replacement = \"list.subList(list.size() - n, list.size())\")",
1086-
" public List<String> last(List<String> list, int n) {",
1087-
" return list.subList(list.size() - n, list.size());",
1088-
" }",
1089-
"}")
1090-
.expectUnchanged()
1091-
.addInputLines(
1092-
"Caller.java",
1093-
"import java.util.ArrayList;",
1094-
"import java.util.List;",
1095-
"public final class Caller {",
1096-
" public void doTest() {",
1097-
" Client client = new Client();",
1098-
" List<String> result = client.last(getList(), 1);",
1099-
" }",
1100-
" public List<String> getList() {",
1101-
" List<String> list = new ArrayList<>();",
1102-
" list.add(\"hi\");",
1103-
" return list;",
1104-
" }",
1105-
"}")
1106-
.addOutputLines(
1107-
"out/Caller.java",
1108-
"import java.util.ArrayList;",
1109-
"import java.util.List;",
1110-
"public final class Caller {",
1111-
" public void doTest() {",
1112-
" Client client = new Client();",
1113-
// TODO(b/188184784): this is a bug, as there's no guarantee that getList() returns the
1114-
// same list every time (or it may be expensive!)
1115-
" List<String> result = getList().subList(getList().size() - 1, getList().size());",
1116-
" }",
1117-
" public List<String> getList() {",
1118-
" List<String> list = new ArrayList<>();",
1119-
" list.add(\"hi\");",
1120-
" return list;",
1121-
" }",
1122-
"}")
1123-
.doTest();
1124-
}
11251032

11261033
@Test
11271034
public void testBooleanParameterWithInlineComment() {

‎core/src/test/java/com/google/errorprone/bugpatterns/inlineme/SuggesterTest.java

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -612,9 +612,7 @@ public void testNestedBlock() {
612612

613613
@Test
614614
public void testTernaryOverMultipleLines() {
615-
// This is suggested/works only if we allow users to use the argument more than once
616615
refactoringTestHelper
617-
.setArgs("-XepOpt:" + InlinabilityResult.DISALLOW_ARGUMENT_REUSE + "=false")
618616
.addInputLines(
619617
"Client.java",
620618
"package com.google.frobber;",
@@ -623,7 +621,7 @@ public void testTernaryOverMultipleLines() {
623621
" @Deprecated",
624622
" public Duration getDeadline(Duration deadline) {",
625623
" return deadline.compareTo(Duration.ZERO) > 0",
626-
" ? deadline",
624+
" ? Duration.ofSeconds(42)",
627625
" : Duration.ZERO;",
628626
" }",
629627
"}")
@@ -633,13 +631,13 @@ public void testTernaryOverMultipleLines() {
633631
"import com.google.errorprone.annotations.InlineMe;",
634632
"import java.time.Duration;",
635633
"public final class Client {",
636-
" @InlineMe(replacement = \""
637-
+ "deadline.compareTo(Duration.ZERO) > 0 ? deadline : Duration.ZERO\", ",
634+
" @InlineMe(replacement = \"deadline.compareTo(Duration.ZERO) > 0 ?"
635+
+ " Duration.ofSeconds(42) : Duration.ZERO\", ",
638636
"imports = \"java.time.Duration\")",
639637
" @Deprecated",
640638
" public Duration getDeadline(Duration deadline) {",
641639
" return deadline.compareTo(Duration.ZERO) > 0",
642-
" ? deadline",
640+
" ? Duration.ofSeconds(42)",
643641
" : Duration.ZERO;",
644642
" }",
645643
"}")

0 commit comments

Comments
 (0)