Skip to content

Commit

Permalink
Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan202 committed Oct 27, 2024
1 parent bd32e7e commit 94fb2a4
Show file tree
Hide file tree
Showing 9 changed files with 243 additions and 293 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
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.STYLE;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.hasModifier;
import static tech.picnic.errorprone.utils.Documentation.BUG_PATTERNS_BASE_URL;

import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
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.SuggestedFixes;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.VariableTree;
import com.sun.source.util.TreeScanner;
import java.util.Locale;
import java.util.regex.Pattern;
import javax.inject.Inject;
import javax.lang.model.element.Modifier;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.utils.Flags;

/**
* A {@link BugChecker} that flags static constants that do not follow the upper snake case naming
* convention.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Constant variables should adhere to the `UPPER_SNAKE_CASE` naming convention",
link = BUG_PATTERNS_BASE_URL + "ConstantNaming",
linkType = CUSTOM,
severity = WARNING,
tags = STYLE)
@SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
public final class ConstantNaming extends BugChecker implements VariableTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<VariableTree> IS_CONSTANT =
allOf(hasModifier(Modifier.STATIC), hasModifier(Modifier.FINAL));
private static final Matcher<VariableTree> IS_PRIVATE = hasModifier(Modifier.PRIVATE);
private static final Pattern SNAKE_CASE = Pattern.compile("([a-z])([A-Z])");
private static final ImmutableSet<String> DEFAULT_EXEMPTED_NAMES =
ImmutableSet.of("serialVersionUID");

/**
* Flag using which constant names that must not be flagged (in addition to those defined by
* {@link #DEFAULT_EXEMPTED_NAMES}) can be specified.
*/
private static final String ADDITIONAL_EXEMPTED_NAMES_FLAG =
"CanonicalConstantNaming:ExemptedNames";

private final ImmutableSet<String> exemptedNames;

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

/**
* Instantiates a customized {@link ConstantNaming}.
*
* @param flags Any provided command line flags.
*/
@Inject
ConstantNaming(ErrorProneFlags flags) {
exemptedNames =
Sets.union(DEFAULT_EXEMPTED_NAMES, Flags.getSet(flags, ADDITIONAL_EXEMPTED_NAMES_FLAG))

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

View workflow job for this annotation

GitHub Actions / pitest

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

swapped parameters 1 and 2 in call to union (covered by 3 tests ParamSwapMutator)
.immutableCopy();
}

@Override
public Description matchVariable(VariableTree tree, VisitorState state) {
String variableName = tree.getName().toString();
if (!IS_CONSTANT.matches(tree, state) || exemptedNames.contains(variableName)) {
return Description.NO_MATCH;
}

String replacement = toUpperSnakeCase(variableName);
if (replacement.equals(variableName)) {
return Description.NO_MATCH;
}

Description.Builder description = buildDescription(tree);
if (!IS_PRIVATE.matches(tree, state)) {
description.setMessage(
"%s; consider renaming to '%s', though note that this is not a private constant"
.formatted(message(), replacement));
} else if (isVariableNameInUse(replacement, state)) {
description.setMessage(
"%s; consider renaming to '%s', though note that a variable with this name is already declared"
.formatted(message(), replacement));
} else {
description.addFix(SuggestedFixes.renameVariable(tree, replacement, state));
}

return description.build();
}

private static String toUpperSnakeCase(String variableName) {
return SNAKE_CASE.matcher(variableName).replaceAll("$1_$2").toUpperCase(Locale.ROOT);
}

private static boolean isVariableNameInUse(String name, VisitorState state) {
return Boolean.TRUE.equals(
new TreeScanner<Boolean, @Nullable Void>() {
@Override
public Boolean visitVariable(VariableTree tree, @Nullable Void unused) {
return ASTHelpers.getSymbol(tree).getSimpleName().toString().equals(name)
|| super.visitVariable(tree, null);
}

@Override
public Boolean reduce(Boolean r1, Boolean r2) {
return Boolean.TRUE.equals(r1) || Boolean.TRUE.equals(r2);
}
}.scan(state.getPath().getCompilationUnit(), null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
Expand All @@ -34,10 +35,8 @@
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.inject.Inject;
import org.jspecify.annotations.Nullable;
Expand Down Expand Up @@ -230,10 +229,8 @@ private static AnnotationAttributeMatcher createAnnotationAttributeMatcher(
excludedAnnotations(flags));
}

private static ImmutableList<String> excludedAnnotations(ErrorProneFlags flags) {
Set<String> exclusions = new HashSet<>();
exclusions.addAll(Flags.getList(flags, EXCLUDED_ANNOTATIONS_FLAG));
exclusions.addAll(BLACKLISTED_ANNOTATIONS);
return ImmutableList.copyOf(exclusions);
private static ImmutableSet<String> excludedAnnotations(ErrorProneFlags flags) {
return Sets.union(BLACKLISTED_ANNOTATIONS, Flags.getSet(flags, EXCLUDED_ANNOTATIONS_FLAG))

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

View workflow job for this annotation

GitHub Actions / pitest

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

swapped parameters 1 and 2 in call to union (covered by 3 tests ParamSwapMutator)
.immutableCopy();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -378,11 +378,11 @@ private Description createDescription(Tree tree, Optional<SuggestedFix.Builder>

private static Matcher<MethodInvocationTree> createConversionMethodMatcher(
ErrorProneFlags flags) {
// XXX: ErrorProneFlags#getList splits by comma, but method signatures may also contain commas.
// For this class methods accepting more than one argument are not valid, but still: not nice.
// XXX: `Flags#getSet` splits by comma, but method signatures may also contain commas. For this
// class methods accepting more than one argument are not valid, but still: not nice.
return anyOf(
WELL_KNOWN_STRING_CONVERSION_METHODS,
new MethodMatcherFactory()
.create(Flags.getList(flags, EXTRA_STRING_CONVERSION_METHODS_FLAG)));
.create(Flags.getSet(flags, EXTRA_STRING_CONVERSION_METHODS_FLAG)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.ErrorProneFlags;
import com.google.errorprone.VisitorState;
Expand Down Expand Up @@ -74,10 +74,10 @@ private static Matcher<VariableTree> hasUnsupportedRequestParamType(ErrorProneFl
return allOf(
annotations(AT_LEAST_ONE, isType("org.springframework.web.bind.annotation.RequestParam")),
anyOf(isSubtypeOf(ImmutableCollection.class), isSubtypeOf(ImmutableMap.class)),
not(isSubtypeOfAny(Flags.getList(flags, SUPPORTED_CUSTOM_TYPES_FLAG))));
not(isSubtypeOfAny(Flags.getSet(flags, SUPPORTED_CUSTOM_TYPES_FLAG))));
}

private static Matcher<Tree> isSubtypeOfAny(ImmutableList<String> inclusions) {
private static Matcher<Tree> isSubtypeOfAny(ImmutableSet<String> inclusions) {
return anyOf(
inclusions.stream()
.map(inclusion -> isSubtypeOf(Suppliers.typeFromString(inclusion)))
Expand Down
Loading

0 comments on commit 94fb2a4

Please sign in to comment.