diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantNaming.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantNaming.java deleted file mode 100644 index 0e5afd8d9c0..00000000000 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantNaming.java +++ /dev/null @@ -1,157 +0,0 @@ -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.utils.Documentation.BUG_PATTERNS_BASE_URL; - -import com.google.auto.service.AutoService; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -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.CompilationUnitTree; -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 constant variables that do not follow the upper snake case naming - * convention. - * - *

This check will rewrite the following variables with all its references: - * - * 

{@code
- * private static final int simpleNumber = 1;
- * }
- * - *

To the following: - * - * 

{@code
- * private static final int SIMPLE_NUMBER = 1;
- * }
- * - * @apiNote This check has an optional flag `AllowedConstantNames` that represents a list of field - * names to exclude from this check. - */ -@AutoService(BugChecker.class) -@BugPattern( - summary = "Constant variables should adhere to the `UPPER_SNAKE_CASE` naming convention", - link = BUG_PATTERNS_BASE_URL + "CanonicalConstantNaming", - linkType = CUSTOM, - severity = WARNING, - tags = LIKELY_ERROR) -public final class CanonicalConstantNaming extends BugChecker implements VariableTreeMatcher { - private static final long serialVersionUID = 1L; - private static final Matcher IS_CONSTANT = - allOf( - hasModifier(Modifier.STATIC), hasModifier(Modifier.PRIVATE), hasModifier(Modifier.FINAL)); - private static final Pattern SNAKE_CASE = Pattern.compile("([a-z])([A-Z])"); - private static final ImmutableSet DEFAULT_ALLOWED_CONSTANT_NAMES = - ImmutableSet.of("serialVersionUID"); - private static final String ALLOWED_CONSTANT_NAMES_FLAG = - "CanonicalConstantNaming:AllowedConstantNames"; - - private final ImmutableList allowedConstantNames; - - /** Instantiates a default {@link CanonicalConstantNaming} instance. */ - public CanonicalConstantNaming() { - this(ErrorProneFlags.empty()); - } - - /** - * Instantiates a customized {@link CanonicalConstantNaming}. - * - * @param flags Any provided command line flags. - */ - @Inject - CanonicalConstantNaming(ErrorProneFlags flags) { - allowedConstantNames = getAllowedFieldNames(flags); - } - - @Override - public Description matchVariable(VariableTree tree, VisitorState state) { - String variableName = tree.getName().toString(); - if (!IS_CONSTANT.matches(tree, state) - || isUpperSnakeCase(variableName) - || isVariableNameAllowed(variableName)) { - return Description.NO_MATCH; - } - - SuggestedFix.Builder fixBuilder = SuggestedFix.builder(); - - ImmutableList variablesInCompilationUnit = - getVariablesInCompilationUnit(state.getPath().getCompilationUnit()); - String replacement = toUpperSnakeCase(variableName); - if (isVariableNameInUse(variablesInCompilationUnit, replacement)) { - reportConstantRenameBlocker(tree, replacement, state); - } else { - fixBuilder.merge(SuggestedFixes.renameVariable(tree, replacement, state)); - } - - return describeMatch(tree, fixBuilder.build()); - } - - private static ImmutableList getAllowedFieldNames(ErrorProneFlags flags) { - return Flags.getList(flags, ALLOWED_CONSTANT_NAMES_FLAG); - } - - private static boolean isUpperSnakeCase(String name) { - return name.contentEquals(toUpperSnakeCase(name)); - } - - private boolean isVariableNameAllowed(String variableName) { - return allowedConstantNames.contains(variableName) - || DEFAULT_ALLOWED_CONSTANT_NAMES.contains(variableName); - } - - private static ImmutableList getVariablesInCompilationUnit( - CompilationUnitTree tree) { - ImmutableList.Builder variablesInFileBuilder = ImmutableList.builder(); - new TreeScanner<@Nullable Void, @Nullable Void>() { - @Override - public @Nullable Void visitVariable(VariableTree variableTree, @Nullable Void unused) { - variablesInFileBuilder.add(variableTree); - return super.visitVariable(variableTree, null); - } - }.scan(tree, null); - - return variablesInFileBuilder.build(); - } - - private static String toUpperSnakeCase(String variableName) { - return SNAKE_CASE.matcher(variableName).replaceAll("$1_$2").toUpperCase(Locale.ROOT); - } - - private static boolean isVariableNameInUse( - ImmutableList variablesInCompilationUnit, String replacement) { - return variablesInCompilationUnit.stream() - .map(ASTHelpers::getSymbol) - .anyMatch(s -> s.getSimpleName().toString().equals(replacement)); - } - - private void reportConstantRenameBlocker( - VariableTree tree, String replacement, VisitorState state) { - state.reportMatch( - buildDescription(tree) - .setMessage( - String.format( - "a variable named `%s` is already defined in this scope", replacement)) - .build()); - } -} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ConstantNaming.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ConstantNaming.java new file mode 100644 index 00000000000..a0354a83c61 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ConstantNaming.java @@ -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 IS_CONSTANT = + allOf(hasModifier(Modifier.STATIC), hasModifier(Modifier.FINAL)); + private static final Matcher IS_PRIVATE = hasModifier(Modifier.PRIVATE); + private static final Pattern SNAKE_CASE = Pattern.compile("([a-z])([A-Z])"); + private static final ImmutableSet 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 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)) + .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() { + @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)); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java index fca196b4647..a788c6991fc 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListing.java @@ -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; @@ -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; @@ -230,10 +229,8 @@ private static AnnotationAttributeMatcher createAnnotationAttributeMatcher( excludedAnnotations(flags)); } - private static ImmutableList excludedAnnotations(ErrorProneFlags flags) { - Set exclusions = new HashSet<>(); - exclusions.addAll(Flags.getList(flags, EXCLUDED_ANNOTATIONS_FLAG)); - exclusions.addAll(BLACKLISTED_ANNOTATIONS); - return ImmutableList.copyOf(exclusions); + private static ImmutableSet excludedAnnotations(ErrorProneFlags flags) { + return Sets.union(BLACKLISTED_ANNOTATIONS, Flags.getSet(flags, EXCLUDED_ANNOTATIONS_FLAG)) + .immutableCopy(); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java index bc6dadbd2f7..689996a06c9 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversion.java @@ -378,11 +378,11 @@ private Description createDescription(Tree tree, Optional private static Matcher 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))); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestParamType.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestParamType.java index ee736da113a..3d2aef0c924 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestParamType.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RequestParamType.java @@ -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; @@ -74,10 +74,10 @@ private static Matcher 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 isSubtypeOfAny(ImmutableList inclusions) { + private static Matcher isSubtypeOfAny(ImmutableSet inclusions) { return anyOf( inclusions.stream() .map(inclusion -> isSubtypeOf(Suppliers.typeFromString(inclusion))) diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantNamingTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantNamingTest.java deleted file mode 100644 index 4ddd2f00d98..00000000000 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalConstantNamingTest.java +++ /dev/null @@ -1,118 +0,0 @@ -package tech.picnic.errorprone.bugpatterns; - -import com.google.errorprone.BugCheckerRefactoringTestHelper; -import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; -import com.google.errorprone.CompilationTestHelper; -import org.junit.jupiter.api.Test; - -final class CanonicalConstantNamingTest { - @Test - void identification() { - CompilationTestHelper.newInstance(CanonicalConstantNaming.class, getClass()) - .addSourceLines( - "A.java", - "class A {", - " // BUG: Diagnostic contains:", - " private static final int foo = 1;", - "", - " // BUG: Diagnostic contains:", - " private static final int bar_BAR = 2;", - "", - " private final int baz = 2;", - "", - " public static final int qux = 3;", - "", - " static final int quux = 4;", - "", - " private final int quuz = 5;", - "", - " int corge = 6;", - "", - " private static final long serialVersionUID = 1L;", - "", - " // BUG: Diagnostic contains: a variable named `NUMBER` is already defined in this scope", - " private static final int number = B.NUMBER;", - "", - " class B {", - " private static final int NUMBER = 1;", - "", - " // BUG: Diagnostic contains:", - " private static final int foo = 2;", - " }", - "}") - .doTest(); - } - - @Test - void replacement() { - BugCheckerRefactoringTestHelper.newInstance(CanonicalConstantNaming.class, getClass()) - .addInputLines( - "A.java", - "class A {", - " private static final int number = 1;", - "", - " int referenceToNumberFromAnotherClass = B.numberFromAnotherClass;", - "", - " static int getConstantNumber() {", - " return number;", - " }", - "", - " static int getLocalNumber() {", - " int number = 3;", - "", - " return number;", - " }", - "", - " class B {", - " private static final int number = 4;", - "", - " private static final int numberFromAnotherClass = 5;", - " }", - "}") - .addOutputLines( - "A.java", - "class A {", - " private static final int NUMBER = 1;", - "", - " int referenceToNumberFromAnotherClass = B.NUMBER_FROM_ANOTHER_CLASS;", - "", - " static int getConstantNumber() {", - " return NUMBER;", - " }", - "", - " static int getLocalNumber() {", - " int number = 3;", - "", - " return number;", - " }", - "", - " class B {", - " private static final int NUMBER = 4;", - "", - " private static final int NUMBER_FROM_ANOTHER_CLASS = 5;", - " }", - "}") - .doTest(TestMode.TEXT_MATCH); - } - - @Test - void allowedConstantsFlag() { - BugCheckerRefactoringTestHelper.newInstance(CanonicalConstantNaming.class, getClass()) - .setArgs("-XepOpt:CanonicalConstantNaming:AllowedConstantNames=foo") - .addInputLines( - "A.java", - "class A {", - " private static final int number = 1;", - "", - " private static final int foo = 3;", - "}") - .addOutputLines( - "A.java", - "class A {", - " private static final int NUMBER = 1;", - "", - " private static final int foo = 3;", - "}") - .doTest(TestMode.TEXT_MATCH); - } -} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ConstantNamingTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ConstantNamingTest.java new file mode 100644 index 00000000000..889f8599a34 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ConstantNamingTest.java @@ -0,0 +1,78 @@ +package tech.picnic.errorprone.bugpatterns; + +import com.google.errorprone.BugCheckerRefactoringTestHelper; +import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +final class ConstantNamingTest { + @Test + void identification() { + CompilationTestHelper.newInstance(ConstantNaming.class, getClass()) + .addSourceLines( + "A.java", + "class A {", + " private static final long serialVersionUID = 1L;", + " private static final int FOO = 1;", + " // BUG: Diagnostic contains: consider renaming to 'BAR', though note that this is not a private", + " // constant", + " static final int bar = 2;", + " // BUG: Diagnostic contains:", + " private static final int baz = 3;", + " // BUG: Diagnostic contains: consider renaming to 'QUX_QUUX', though note that a variable with", + " // this name is already declared", + " private static final int qux_QUUX = 4;", + " // BUG: Diagnostic contains: consider renaming to 'QUUZ', though note that a variable with", + " // this name is already declared", + " private static final int quuz = 3;", + "", + " private final int foo = 4;", + " private final Runnable QUX_QUUX =", + " new Runnable() {", + " private static final int QUUZ = 1;", + "", + " @Override", + " public void run() {}", + " };", + "}") + .doTest(); + } + + @Test + void identificationWithCustomExemption() { + CompilationTestHelper.newInstance(ConstantNaming.class, getClass()) + .setArgs("-XepOpt:CanonicalConstantNaming:ExemptedNames=foo,baz") + .addSourceLines( + "A.java", + "class A {", + " private static final long serialVersionUID = 1L;", + " private static final int foo = 1;", + " // BUG: Diagnostic contains:", + " private static final int bar = 2;", + " private static final int baz = 3;", + "}") + .doTest(); + } + + @Test + void replacement() { + BugCheckerRefactoringTestHelper.newInstance(ConstantNaming.class, getClass()) + .addInputLines( + "A.java", + "class A {", + " static final int foo = 1;", + " private static final int bar = 2;", + " private static final int baz = 3;", + " private static final int BAZ = 4;", + "}") + .addOutputLines( + "A.java", + "class A {", + " static final int foo = 1;", + " private static final int BAR = 2;", + " private static final int baz = 3;", + " private static final int BAZ = 4;", + "}") + .doTest(TestMode.TEXT_MATCH); + } +} diff --git a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/Flags.java b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/Flags.java index df2787c7f40..b8e263404f7 100644 --- a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/Flags.java +++ b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/Flags.java @@ -1,6 +1,7 @@ package tech.picnic.errorprone.utils; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.ErrorProneFlags; /** Helper methods for working with {@link ErrorProneFlags}. */ @@ -19,4 +20,19 @@ public static ImmutableList getList(ErrorProneFlags errorProneFlags, Str ImmutableList list = errorProneFlags.getListOrEmpty(name); return list.equals(ImmutableList.of("")) ? ImmutableList.of() : list; } + + /** + * Returns the set of (comma-separated) arguments passed using the given Error Prone flag. + * + * @param errorProneFlags The full set of flags provided. + * @param name The name of the flag of interest. + * @return A non-{@code null} set of provided arguments; this set is empty if the flag was not + * provided, or if the flag's value is the empty string. + * @implNote This method does not delegate to {@link ErrorProneFlags#getSetOrEmpty(String)}, as + * that method wouldn't allow us to identify a non-singleton set of empty strings; such a set + * should not be treated as empty. + */ + public static ImmutableSet getSet(ErrorProneFlags errorProneFlags, String name) { + return ImmutableSet.copyOf(getList(errorProneFlags, name)); + } } diff --git a/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/FlagsTest.java b/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/FlagsTest.java index bc7658ce84a..dcc217c8f7c 100644 --- a/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/FlagsTest.java +++ b/error-prone-utils/src/test/java/tech/picnic/errorprone/utils/FlagsTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.params.provider.Arguments.arguments; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.ErrorProneOptions; import java.util.stream.Stream; import org.junit.jupiter.params.ParameterizedTest; @@ -11,21 +12,29 @@ import org.junit.jupiter.params.provider.MethodSource; final class FlagsTest { - private static Stream getListTestCases() { - /* { args, flag, expected } */ + private static Stream getCollectionTestCases() { + /* { args, flag, listed } */ return Stream.of( arguments(ImmutableList.of(), "Foo", ImmutableList.of()), arguments(ImmutableList.of("-XepOpt:Foo=bar,baz"), "Qux", ImmutableList.of()), arguments(ImmutableList.of("-XepOpt:Foo="), "Foo", ImmutableList.of()), arguments(ImmutableList.of("-XepOpt:Foo=bar"), "Foo", ImmutableList.of("bar")), + arguments(ImmutableList.of("-XepOpt:Foo=bar,bar"), "Foo", ImmutableList.of("bar", "bar")), arguments(ImmutableList.of("-XepOpt:Foo=bar,baz"), "Foo", ImmutableList.of("bar", "baz")), arguments(ImmutableList.of("-XepOpt:Foo=,"), "Foo", ImmutableList.of("", ""))); } - @MethodSource("getListTestCases") + @MethodSource("getCollectionTestCases") @ParameterizedTest - void getList(ImmutableList args, String flag, ImmutableList expected) { + void getList(ImmutableList args, String flag, ImmutableList listed) { assertThat(Flags.getList(ErrorProneOptions.processArgs(args).getFlags(), flag)) - .containsExactlyElementsOf(expected); + .containsExactlyElementsOf(listed); + } + + @MethodSource("getCollectionTestCases") + @ParameterizedTest + void getSet(ImmutableList args, String flag, ImmutableList listed) { + assertThat(Flags.getSet(ErrorProneOptions.processArgs(args).getFlags(), flag)) + .containsExactlyElementsOf(ImmutableSet.copyOf(listed)); } }