Skip to content

Commit

Permalink
Introduce CanonicalConstantFieldName bug checker
Browse files Browse the repository at this point in the history
  • Loading branch information
mohamedsamehsalah committed Sep 17, 2023
1 parent 87f79da commit 7fff49f
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package tech.picnic.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;

import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.VariableTreeMatcher;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Symbol.VarSymbol;
import java.util.Locale;
import javax.inject.Inject;
import javax.lang.model.element.Modifier;
import tech.picnic.errorprone.bugpatterns.util.Flags;

/**
* A {@link BugChecker} that warns and suggests the fix when a constant variable does not follow the
* upper snake case naming convention.
*
* <p>Example:
*
* <p>This checker will re-write the following variables with all its references:
*
* <ul>
* <li>private static final int number = 1;
* <li>static final int otherNumber = 2;
* </ul>
*
* <p>To the following:
*
* <ul>
* <li>private static final int NUMBER = 1;
* <li>static final int OTHER_NUMBER = 2;
* </ul>
*/
@AutoService(BugChecker.class)
@BugPattern(
summary =
"Make sure that all constant variables follow the `UPPER_SNAKE_CASE` naming convention.",
link = BUG_PATTERNS_BASE_URL + "CanonicalConstantFieldName",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class CanonicalConstantFieldName extends BugChecker implements VariableTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<Tree> IS_CONSTANT =
allOf(hasModifier(Modifier.STATIC), hasModifier(Modifier.FINAL));
private static final String EXCLUDED_CONSTANT_FIELD_NAMES =
"CanonicalConstantFieldName:ExcludedConstantFliedNames";

private final ImmutableList<String> excludedConstantFliedNames;

/** Instantiates a default {@link CanonicalConstantFieldName} instance. */
public CanonicalConstantFieldName() {
this(ErrorProneFlags.empty());
}

/**
* Instantiates a customized {@link CanonicalConstantFieldName}.
*
* @param flags Any provided command line flags.
*/
@Inject
CanonicalConstantFieldName(ErrorProneFlags flags) {
excludedConstantFliedNames = getCanonicalizedLoggerName(flags);
}

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
if (!IS_CONSTANT.matches(tree, state)) {

Check warning on line 84 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 84 without causing a test to fail

removed conditional - replaced equality check with false (covered by 1 tests RemoveConditionalMutator_EQUAL_ELSE)
return Description.NO_MATCH;

Check warning on line 85 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 85 without causing a test to fail

replaced return value with null for matchVariable (no tests cover this line NullReturnValsMutator)
}
SuggestedFix.Builder fixBuilder = SuggestedFix.builder();

VarSymbol variableSymbol = ASTHelpers.getSymbol(tree);
String variableName = variableSymbol.getSimpleName().toString();

if (!isVariableUpperSnakeCase(variableName) && !isVariableNameExcluded(variableName)) {
fixBuilder.merge(SuggestedFixes.renameVariable(tree, toUpperSnakeCase(variableName), state));
}

return fixBuilder.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fixBuilder.build());

Check warning on line 96 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 96 without causing a test to fail

removed conditional - replaced equality check with false (covered by 1 tests RemoveConditionalMutator_EQUAL_ELSE)
}

private static boolean isVariableUpperSnakeCase(String variableName) {
return variableName.equals(toUpperSnakeCase(variableName));

Check warning on line 100 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 100 without causing a test to fail

replaced boolean return with false for isVariableUpperSnakeCase (covered by 1 tests BooleanFalseReturnValsMutator)
}

private boolean isVariableNameExcluded(String variableName) {
return excludedConstantFliedNames.contains(variableName);

Check warning on line 104 in error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantFieldName.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 104 without causing a test to fail

replaced boolean return with false for isVariableNameExcluded (covered by 1 tests BooleanFalseReturnValsMutator)
}

private static String toUpperSnakeCase(String variableName) {
return variableName.replaceAll("([a-z])([A-Z])", "$1_$2").toUpperCase(Locale.ROOT);
}

private static ImmutableList<String> getCanonicalizedLoggerName(ErrorProneFlags flags) {
return Flags.getList(flags, EXCLUDED_CONSTANT_FIELD_NAMES);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package tech.picnic.errorprone.bugpatterns;

import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import org.junit.jupiter.api.Test;

final class CanonicalConstantFieldNameTest {
@Test
void replacement() {
BugCheckerRefactoringTestHelper.newInstance(CanonicalConstantFieldName.class, getClass())
.addInputLines(
"A.java",
"class A {",
" private static final int number = 1;",
"",
" static final int otherNumber = 2;",
"",
" static final int ANOTHER_NUMBER = 3;",
"",
" static int getNumber() {",
" return number;",
" }",
"}")
.addOutputLines(
"A.java",
"class A {",
" private static final int NUMBER = 1;",
"",
" static final int OTHER_NUMBER = 2;",
"",
" static final int ANOTHER_NUMBER = 3;",
"",
" static int getNumber() {",
" return NUMBER;",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,7 @@
readable than the alternative. -->
-Xep:YodaCondition:OFF
-XepOpt:CheckReturnValue:CheckAllConstructors=true
-XepOpt:CanonicalConstantFieldName:ExcludedConstantFliedNames=serialVersionUID
<!-- XXX: Enable once there are fewer
false-positives.
-XepOpt:CheckReturnValue:CheckAllMethods=true -->
Expand Down

0 comments on commit 7fff49f

Please sign in to comment.