From e52d93a2a87a16722c6c0277dd586180e04919dc Mon Sep 17 00:00:00 2001 From: eviaaaaa <100466054+eviaaaaa@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:36:58 +0800 Subject: [PATCH] Recognize compile-time constants in LoopConditionChecker --- .../bugpatterns/LoopConditionChecker.java | 10 +- .../bugpatterns/LoopConditionCheckerTest.java | 106 ++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/LoopConditionChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/LoopConditionChecker.java index b9234056616..74b44c9d801 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/LoopConditionChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/LoopConditionChecker.java @@ -93,7 +93,7 @@ private Description check(ExpressionTree condition, ImmutableList loopBody .build(); } - /** Scan for loop conditions that are determined entirely by the state of local variables. */ + /** Scan for loop conditions determined entirely by local variables and compile-time constants. */ private static class LoopConditionVisitor extends SimpleTreeVisitor { static ImmutableSet scan(Tree tree) { @@ -112,6 +112,9 @@ static ImmutableSet scan(Tree tree) { @Override public Boolean visitIdentifier(IdentifierTree tree, Void unused) { + if (ASTHelpers.constValue(tree) != null) { + return true; + } Symbol sym = ASTHelpers.getSymbol(tree); if (sym instanceof Symbol.VarSymbol varSymbol) { switch (sym.getKind()) { @@ -130,6 +133,11 @@ public Boolean visitLiteral(LiteralTree tree, Void unused) { return true; } + @Override + protected Boolean defaultAction(Tree node, Void unused) { + return ASTHelpers.constValue(node) != null; + } + @Override public Boolean visitUnary(UnaryTree node, Void unused) { return node.getExpression().accept(this, null); diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/LoopConditionCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/LoopConditionCheckerTest.java index 170f06a4e7d..424c791b4d1 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/LoopConditionCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/LoopConditionCheckerTest.java @@ -175,6 +175,112 @@ void f() { .doTest(); } + @Test + public void compileTimeConstantBounds_areEquivalentToLiterals() { + compilationTestHelper + .expectErrorMessage( + "ONLY_I", + message -> + message.contains("condition variable(s) never modified in loop body: i") + && !message.contains("i,")) + .addSourceLines( + "Test.java", + """ + class Test { + static final int ZERO = 0; + static final int ONE = 1; + static final boolean TRUE = true; + static final boolean FALSE = false; + + void sink() {} + + void f() { + final int localOne = 1; + // BUG: Diagnostic matches: ONLY_I + for (int i = 0; i < 1; sink()) {} + // BUG: Diagnostic matches: ONLY_I + for (int i = 0; i < localOne; sink()) {} + // BUG: Diagnostic matches: ONLY_I + for (int i = 0; i < ONE; sink()) {} + // BUG: Diagnostic matches: ONLY_I + for (int i = 0; i < ZERO; sink()) {} + // BUG: Diagnostic matches: ONLY_I + for (int i = 0; TRUE && i < ONE; sink()) {} + // BUG: Diagnostic matches: ONLY_I + for (int i = 0; FALSE || i < ONE; sink()) {} + } + } + """) + .doTest(); + } + + @Test + public void compileTimeConstantExpressions() { + compilationTestHelper + .addSourceLines( + "Test.java", + """ + class Bounds { + static final int ZERO = 0; + static final int ONE = 1; + static final int TWO = ONE + 1; + } + + class Test { + static final boolean TRUE = true; + + void sink() {} + + void f() { + // BUG: Diagnostic contains: + for (int i = 0; i < Bounds.ONE; sink()) {} + // BUG: Diagnostic contains: + for (int i = 0; i < Bounds.TWO; sink()) {} + // BUG: Diagnostic contains: + for (int i = 0; i < Bounds.ONE + 1; sink()) {} + // BUG: Diagnostic contains: + for (int i = 0; i < (int) Bounds.ONE; sink()) {} + // BUG: Diagnostic contains: + for (int i = 0; i < (TRUE ? Bounds.ONE : Bounds.ZERO); sink()) {} + } + } + """) + .doTest(); + } + + @Test + public void nonConstantFieldsRemainOutOfScope() { + compilationTestHelper + .addSourceLines( + "Test.java", + """ + class Test { + static final Integer BOXED_ONE = 1; + static final int RUNTIME_ONE = Integer.parseInt("1"); + static final Boolean BOXED_TRUE = true; + static final boolean RUNTIME_TRUE = Boolean.parseBoolean("true"); + static final boolean RUNTIME_FALSE = Boolean.parseBoolean("false"); + static int mutableOne = 1; + static boolean mutableTrue = true; + static boolean mutableFalse = false; + + void sink() {} + + void f() { + for (int i = 0; i < BOXED_ONE; sink()) {} + for (int i = 0; i < RUNTIME_ONE; sink()) {} + for (int i = 0; i < mutableOne; sink()) {} + for (int i = 0; BOXED_TRUE && i < 1; sink()) {} + for (int i = 0; RUNTIME_TRUE && i < 1; sink()) {} + for (int i = 0; RUNTIME_FALSE || i < 1; sink()) {} + for (int i = 0; mutableTrue && i < 1; sink()) {} + for (int i = 0; mutableFalse || i < 1; sink()) {} + } + } + """) + .doTest(); + } + @Test public void negative_field() { compilationTestHelper