Skip to content

Commit 2b306b6

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Improve XorPower
* handle longs as well as ints * handle e.g. `x * 10 ^ 3`, which evalutes as `(x * 10) ^ 3`, but where the intent may have been `x * 1000` PiperOrigin-RevId: 652520871
1 parent 008cfb0 commit 2b306b6

2 files changed

Lines changed: 74 additions & 10 deletions

File tree

‎core/src/main/java/com/google/errorprone/bugpatterns/XorPower.java‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
2020
import static com.google.errorprone.matchers.Description.NO_MATCH;
21+
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
2122

22-
import com.google.common.base.Strings;
2323
import com.google.errorprone.BugPattern;
2424
import com.google.errorprone.VisitorState;
2525
import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher;
@@ -37,21 +37,31 @@ public Description matchBinary(BinaryTree tree, VisitorState state) {
3737
if (!tree.getKind().equals(Tree.Kind.XOR)) {
3838
return NO_MATCH;
3939
}
40-
Integer lhs = ASTHelpers.constValue(tree.getLeftOperand(), Integer.class);
40+
Tree lhsTree = tree.getLeftOperand();
41+
while (lhsTree instanceof BinaryTree) {
42+
lhsTree = ((BinaryTree) lhsTree).getRightOperand();
43+
}
44+
Number lhs = ASTHelpers.constValue(lhsTree, Number.class);
4145
if (lhs == null) {
4246
return NO_MATCH;
4347
}
48+
if (lhs.longValue() != lhs.intValue()) {
49+
return NO_MATCH;
50+
}
4451
switch (lhs.intValue()) {
4552
case 2:
4653
case 10:
4754
break;
4855
default:
4956
return NO_MATCH;
5057
}
51-
Integer rhs = ASTHelpers.constValue(tree.getRightOperand(), Integer.class);
58+
Number rhs = ASTHelpers.constValue(tree.getRightOperand(), Number.class);
5259
if (rhs == null) {
5360
return NO_MATCH;
5461
}
62+
if (rhs.longValue() != rhs.intValue()) {
63+
return NO_MATCH;
64+
}
5565
if (state.getSourceForNode(tree.getRightOperand()).startsWith("0")) {
5666
// hex and octal literals
5767
return NO_MATCH;
@@ -62,16 +72,24 @@ public Description matchBinary(BinaryTree tree, VisitorState state) {
6272
String.format(
6373
"The ^ operator is binary XOR, not a power operator, so '%s' will always"
6474
+ " evaluate to %d.",
65-
state.getSourceForNode(tree), lhs ^ rhs));
75+
state.getSourceForNode(tree), lhs.intValue() ^ rhs.intValue()));
76+
String suffix = lhs instanceof Long ? "L" : "";
77+
int start = getStartPosition(lhsTree);
78+
int end = state.getEndPosition(tree);
6679
switch (lhs.intValue()) {
6780
case 2:
68-
if (rhs <= 31) {
69-
description.addFix(SuggestedFix.replace(tree, String.format("1 << %d", rhs)));
81+
if (rhs.intValue() <= (lhs instanceof Long ? 63 : 31)) {
82+
String replacement = String.format("1%s << %d", suffix, rhs);
83+
if (start != getStartPosition(tree)) {
84+
replacement = "(" + replacement + ")";
85+
}
86+
description.addFix(SuggestedFix.replace(start, end, replacement));
7087
}
7188
break;
7289
case 10:
73-
if (rhs <= 9) {
74-
description.addFix(SuggestedFix.replace(tree, "1" + Strings.repeat("0", rhs)));
90+
if (rhs.intValue() <= (lhs instanceof Long ? 18 : 9)) {
91+
description.addFix(
92+
SuggestedFix.replace(start, end, "1" + "0".repeat(rhs.intValue()) + suffix));
7593
}
7694
break;
7795
default:

‎core/src/test/java/com/google/errorprone/bugpatterns/XorPowerTest.java‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ public void positive() {
3232
"Test.java",
3333
"class Test {",
3434
" static final int X = 2 ^ 16;",
35-
" static final int Y = 2 ^ 32;",
35+
" static final int TOO_BIG = 2 ^ 32;",
3636
" static final int Z = 2 ^ 31;",
3737
" static final int P = 10 ^ 6;",
3838
"}")
3939
.addOutputLines(
4040
"Test.java",
4141
"class Test {",
4242
" static final int X = 1 << 16;",
43-
" static final int Y = 2 ^ 32;",
43+
" static final int TOO_BIG = 2 ^ 32;",
4444
" static final int Z = 1 << 31;",
4545
" static final int P = 1000000;",
4646
"}")
@@ -59,4 +59,50 @@ public void negative() {
5959
"}")
6060
.doTest();
6161
}
62+
63+
@Test
64+
public void positiveLongs() {
65+
BugCheckerRefactoringTestHelper.newInstance(XorPower.class, getClass())
66+
.addInputLines(
67+
"Test.java",
68+
"class Test {",
69+
" static final long X = 2L ^ 16;",
70+
" static final long Y = 2L ^ 32;",
71+
" static final long Z = 2L ^ 31;",
72+
" static final long TOO_BIG = 2L ^ 64;",
73+
" static final long P = 10L ^ 6;",
74+
"}")
75+
.addOutputLines(
76+
"Test.java",
77+
"class Test {",
78+
" static final long X = 1L << 16;",
79+
" static final long Y = 1L << 32;",
80+
" static final long Z = 1L << 31;",
81+
" static final long TOO_BIG = 2L ^ 64;",
82+
" static final long P = 1000000L;",
83+
"}")
84+
.doTest();
85+
}
86+
87+
@Test
88+
public void precedence() {
89+
BugCheckerRefactoringTestHelper.newInstance(XorPower.class, getClass())
90+
.addInputLines(
91+
"Test.java", //
92+
"class Test {",
93+
" void f(int i) {",
94+
" int x = i * 2 ^ 16;",
95+
" int y = i * 10 ^ 3;",
96+
" }",
97+
"}")
98+
.addOutputLines(
99+
"Test.java", //
100+
"class Test {",
101+
" void f(int i) {",
102+
" int x = i * (1 << 16);",
103+
" int y = i * 1000;",
104+
" }",
105+
"}")
106+
.doTest();
107+
}
62108
}

0 commit comments

Comments
 (0)