Skip to content

Commit 159ae7f

Browse files
graememorganError Prone Team
authored and
Error Prone Team
committed
Remove volatile when finalling static fields.
Please do validate my claim, but I think there's simply no point in ever putting volatile on a static final field. PiperOrigin-RevId: 573830549
1 parent e8298e4 commit 159ae7f

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
2121
import static com.google.errorprone.fixes.SuggestedFix.merge;
2222
import static com.google.errorprone.fixes.SuggestedFixes.addModifiers;
23+
import static com.google.errorprone.fixes.SuggestedFixes.removeModifiers;
2324
import static com.google.errorprone.matchers.Description.NO_MATCH;
2425
import static com.google.errorprone.util.ASTHelpers.canBeRemoved;
2526
import static com.google.errorprone.util.ASTHelpers.getSymbol;
@@ -46,6 +47,7 @@
4647
import com.sun.tools.javac.code.Symbol.VarSymbol;
4748
import java.util.Objects;
4849
import java.util.concurrent.atomic.AtomicBoolean;
50+
import javax.lang.model.element.Modifier;
4951

5052
/** A BugPattern; see the summary. */
5153
@BugPattern(summary = "Static fields should almost always be final.", severity = WARNING)
@@ -75,7 +77,10 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
7577
return describeMatch(
7678
tree,
7779
merge(
78-
addModifiers(tree, state, FINAL).orElse(emptyFix()),
80+
addModifiers(tree, tree.getModifiers(), state, ImmutableSet.of(FINAL))
81+
.orElse(emptyFix()),
82+
removeModifiers(tree.getModifiers(), state, ImmutableSet.of(Modifier.VOLATILE))
83+
.orElse(emptyFix()),
7984
addDefaultInitializerIfNecessary(tree, state)));
8085
}
8186

core/src/test/java/com/google/errorprone/bugpatterns/NonFinalStaticFieldTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,4 +195,20 @@ public void exemptedAnnotation_noFinding() {
195195
"}")
196196
.doTest();
197197
}
198+
199+
@Test
200+
public void volatileRemoved() {
201+
refactoringTestHelper
202+
.addInputLines(
203+
"Test.java", //
204+
"public class Test {",
205+
" private volatile static String FOO = \"\";",
206+
"}")
207+
.addOutputLines(
208+
"Test.java", //
209+
"public class Test {",
210+
" private static final String FOO = \"\";",
211+
"}")
212+
.doTest();
213+
}
198214
}

0 commit comments

Comments
 (0)