Skip to content

Commit

Permalink
Remove volatile when finalling static fields.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
graememorgan authored and Error Prone Team committed Oct 16, 2023
1 parent e8298e4 commit 159ae7f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
import static com.google.errorprone.fixes.SuggestedFix.merge;
import static com.google.errorprone.fixes.SuggestedFixes.addModifiers;
import static com.google.errorprone.fixes.SuggestedFixes.removeModifiers;
import static com.google.errorprone.matchers.Description.NO_MATCH;
import static com.google.errorprone.util.ASTHelpers.canBeRemoved;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
Expand All @@ -46,6 +47,7 @@
import com.sun.tools.javac.code.Symbol.VarSymbol;
import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.lang.model.element.Modifier;

/** A BugPattern; see the summary. */
@BugPattern(summary = "Static fields should almost always be final.", severity = WARNING)
Expand Down Expand Up @@ -75,7 +77,10 @@ public Description matchVariable(VariableTree tree, VisitorState state) {
return describeMatch(
tree,
merge(
addModifiers(tree, state, FINAL).orElse(emptyFix()),
addModifiers(tree, tree.getModifiers(), state, ImmutableSet.of(FINAL))
.orElse(emptyFix()),
removeModifiers(tree.getModifiers(), state, ImmutableSet.of(Modifier.VOLATILE))
.orElse(emptyFix()),
addDefaultInitializerIfNecessary(tree, state)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,20 @@ public void exemptedAnnotation_noFinding() {
"}")
.doTest();
}

@Test
public void volatileRemoved() {
refactoringTestHelper
.addInputLines(
"Test.java", //
"public class Test {",
" private volatile static String FOO = \"\";",
"}")
.addOutputLines(
"Test.java", //
"public class Test {",
" private static final String FOO = \"\";",
"}")
.doTest();
}
}

0 comments on commit 159ae7f

Please sign in to comment.