diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index 15460b3565..4fd14a1f28 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -76,6 +76,10 @@ javac provided + + com.google.googlejavaformat + google-java-format + com.google.guava guava diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ErrorProneTestHelperSourceFormatCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ErrorProneTestHelperSourceFormatCheck.java new file mode 100644 index 0000000000..fab7db6451 --- /dev/null +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/ErrorProneTestHelperSourceFormatCheck.java @@ -0,0 +1,161 @@ +package tech.picnic.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.LinkType.NONE; +import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION; +import static com.google.errorprone.BugPattern.StandardTags.STYLE; +import static com.google.errorprone.matchers.Matchers.anyOf; +import static com.google.errorprone.matchers.Matchers.instanceMethod; +import static java.util.stream.Collectors.joining; + +import com.google.auto.service.AutoService; +import com.google.common.base.Splitter; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.util.ASTHelpers; +import com.google.googlejavaformat.java.Formatter; +import com.google.googlejavaformat.java.FormatterException; +import com.google.googlejavaformat.java.ImportOrderer; +import com.google.googlejavaformat.java.JavaFormatterOptions.Style; +import com.google.googlejavaformat.java.RemoveUnusedImports; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; +import com.sun.tools.javac.util.Position; +import java.util.List; +import java.util.Optional; + +/** + * A {@link BugChecker} which flags improperly formatted Error Prone test code. + * + *

All test code should be formatted in accordance with Google Java Format's {@link Formatter} + * output, and imports should be ordered according to the {@link Style#GOOGLE Google} style. + * + *

This checker inspects inline code passed to {@code + * com.google.errorprone.CompilationTestHelper} and {@code + * com.google.errorprone.BugCheckerRefactoringTestHelper}. It requires that this code is properly + * formatted and that its imports are organized. Only code that represents the expected output of a + * refactoring operation is allowed to have unused imports, as most {@link BugChecker}s do not (and + * are not able to) remove imports that become obsolete as a result of applying their suggested + * fix(es). + */ +// XXX: Once we target JDK 17 (optionally?) suggest text block fixes. +// XXX: GJF guesses the line separator to be used by inspecting the source. When using text blocks +// this may cause the current unconditional use of `\n` not to be sufficient when building on +// Windows; TBD. +@AutoService(BugChecker.class) +@BugPattern( + name = "ErrorProneTestHelperSourceFormat", + summary = "Test code should follow the Google Java style", + linkType = NONE, + severity = SUGGESTION, + tags = STYLE) +public final class ErrorProneTestHelperSourceFormatCheck extends BugChecker + implements MethodInvocationTreeMatcher { + private static final long serialVersionUID = 1L; + private static final Formatter FORMATTER = new Formatter(); + private static final Matcher INPUT_SOURCE_ACCEPTING_METHOD = + anyOf( + instanceMethod() + .onDescendantOf("com.google.errorprone.CompilationTestHelper") + .named("addSourceLines"), + instanceMethod() + .onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper") + .named("addInputLines")); + private static final Matcher OUTPUT_SOURCE_ACCEPTING_METHOD = + instanceMethod() + .onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput") + .named("addOutputLines"); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + boolean isOutputSource = OUTPUT_SOURCE_ACCEPTING_METHOD.matches(tree, state); + if (!isOutputSource && !INPUT_SOURCE_ACCEPTING_METHOD.matches(tree, state)) { + return Description.NO_MATCH; + } + + List sourceLines = + tree.getArguments().subList(1, tree.getArguments().size()); + if (sourceLines.isEmpty()) { + return buildDescription(tree).setMessage("No source code provided").build(); + } + + int startPos = ASTHelpers.getStartPosition(sourceLines.get(0)); + int endPos = state.getEndPosition(sourceLines.get(sourceLines.size() - 1)); + + /* Attempt to format the source code only if it fully consists of constant expressions. */ + return getConstantSourceCode(sourceLines) + .map(source -> flagFormattingIssues(startPos, endPos, source, isOutputSource, state)) + .orElse(Description.NO_MATCH); + } + + private Description flagFormattingIssues( + int startPos, int endPos, String source, boolean retainUnusedImports, VisitorState state) { + Tree methodInvocation = state.getPath().getLeaf(); + + String formatted; + try { + formatted = formatSourceCode(source, retainUnusedImports).trim(); + } catch (FormatterException e) { + return buildDescription(methodInvocation) + .setMessage(String.format("Source code is malformed: %s", e.getMessage())) + .build(); + } + + if (source.trim().equals(formatted)) { + return Description.NO_MATCH; + } + + if (startPos == Position.NOPOS || endPos == Position.NOPOS) { + /* + * We have insufficient source information to emit a fix, so we only flag the fact that the + * code isn't properly formatted. + */ + return describeMatch(methodInvocation); + } + + /* + * The code isn't properly formatted; replace all lines with the properly formatted + * alternatives. + */ + return describeMatch( + methodInvocation, + SuggestedFix.replace( + startPos, + endPos, + Splitter.on('\n') + .splitToStream(formatted) + .map(state::getConstantExpression) + .collect(joining(", ")))); + } + + private static String formatSourceCode(String source, boolean retainUnusedImports) + throws FormatterException { + String withReorderedImports = ImportOrderer.reorderImports(source, Style.GOOGLE); + String withOptionallyRemovedImports = + retainUnusedImports + ? withReorderedImports + : RemoveUnusedImports.removeUnusedImports(withReorderedImports); + return FORMATTER.formatSource(withOptionallyRemovedImports); + } + + private static Optional getConstantSourceCode( + List sourceLines) { + StringBuilder source = new StringBuilder(); + + for (ExpressionTree sourceLine : sourceLines) { + Object value = ASTHelpers.constValue(sourceLine); + if (value == null) { + return Optional.empty(); + } + + source.append(value).append('\n'); + } + + return Optional.of(source.toString()); + } +} diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheck.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheck.java index 37077f7f55..a5a9879f6a 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheck.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheck.java @@ -52,7 +52,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) { Optional fix = tryFixOrdering(originalOrdering, sortedAnnotations, state); - Description.Builder description = buildDescription(tree); + Description.Builder description = buildDescription(originalOrdering.get(0)); fix.ifPresent(description::addFix); return description.build(); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreatorCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreatorCheckTest.java index 5770cca15d..bc33bc169f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreatorCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AmbiguousJsonCreatorCheckTest.java @@ -112,7 +112,6 @@ void identification() { " return new F(s);", " }", " }", - "", "}") .doTest(); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorCheckTest.java index 78b1a56a53..6152058e77 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/AutowiredConstructorCheckTest.java @@ -27,27 +27,34 @@ void identification() { " }", "", " class B {", - " @Autowired void setProperty(Object o) {}", + " @Autowired", + " void setProperty(Object o) {}", " }", "", " class C {", " // BUG: Diagnostic contains:", - " @Autowired C() {}", + " @Autowired", + " C() {}", " }", "", " class D {", " // BUG: Diagnostic contains:", - " @Autowired D(String x) {}", + " @Autowired", + " D(String x) {}", " }", "", " class E {", - " @Autowired E() {}", + " @Autowired", + " E() {}", + "", " E(String x) {}", " }", "", " class F {", " F() {}", - " @Autowired F(String x) {}", + "", + " @Autowired", + " F(String x) {}", " }", "", " class G {", @@ -55,7 +62,8 @@ void identification() { " }", "", " class H {", - " @SafeVarargs H(List... lists) {}", + " @SafeVarargs", + " H(List... lists) {}", " }", "}") .doTest(); @@ -70,11 +78,14 @@ void replacement() { "", "interface Container {", " class A {", - " @Autowired @Deprecated A() {}", + " @Autowired", + " @Deprecated", + " A() {}", " }", "", " class B {", - " @Autowired B(String x) {}", + " @Autowired", + " B(String x) {}", " }", "}") .addOutputLines( @@ -83,10 +94,13 @@ void replacement() { "", "interface Container {", " class A {", - " @Deprecated A() {}", + "", + " @Deprecated", + " A() {}", " }", "", " class B {", + "", " B(String x) {}", " }", "}") diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxCheckTest.java index 18bd06182d..6863e32b23 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalAnnotationSyntaxCheckTest.java @@ -23,63 +23,110 @@ void identification() { "interface A {", " @interface Foo {", " int[] value() default {};", + "", " int[] value2() default {};", " }", "", - " @pkg.A.Foo A minimal1();", - " @A.Foo A minimal2();", - " @Foo A minimal3();", + " @pkg.A.Foo", + " A minimal1();", + "", + " @A.Foo", + " A minimal2();", + "", + " @Foo", + " A minimal3();", "", " // BUG: Diagnostic contains:", - " @pkg.A.Foo() A functional1();", + " @pkg.A.Foo()", + " A functional1();", " // BUG: Diagnostic contains:", - " @A.Foo() A functional2();", + " @A.Foo()", + " A functional2();", " // BUG: Diagnostic contains:", - " @Foo() A functional3();", + " @Foo()", + " A functional3();", + "", + " @pkg.A.Foo(1)", + " A simple1();", + "", + " @A.Foo(1)", + " A simple2();", "", - " @pkg.A.Foo(1) A simple1();", - " @A.Foo(1) A simple2();", - " @Foo(1) A simple3();", + " @Foo(1)", + " A simple3();", "", " // BUG: Diagnostic contains:", - " @pkg.A.Foo({1}) A singleton1();", + " @pkg.A.Foo({1})", + " A singleton1();", " // BUG: Diagnostic contains:", - " @A.Foo({1}) A singleton2();", + " @A.Foo({1})", + " A singleton2();", " // BUG: Diagnostic contains:", - " @Foo({1}) A singleton3();", + " @Foo({1})", + " A singleton3();", "", " // BUG: Diagnostic contains:", - " @pkg.A.Foo(value = 1) A verbose1();", + " @pkg.A.Foo(value = 1)", + " A verbose1();", " // BUG: Diagnostic contains:", - " @A.Foo(value = 1) A verbose2();", + " @A.Foo(value = 1)", + " A verbose2();", " // BUG: Diagnostic contains:", - " @Foo(value = 1) A verbose3();", + " @Foo(value = 1)", + " A verbose3();", "", - " @pkg.A.Foo(value2 = 2) A custom1();", - " @A.Foo(value2 = 2) A custom2();", - " @Foo(value2 = 2) A custom3();", + " @pkg.A.Foo(value2 = 2)", + " A custom1();", + "", + " @A.Foo(value2 = 2)", + " A custom2();", + "", + " @Foo(value2 = 2)", + " A custom3();", "", " // BUG: Diagnostic contains:", - " @pkg.A.Foo(value2 = {2}) A customSingleton1();", + " @pkg.A.Foo(value2 = {2})", + " A customSingleton1();", " // BUG: Diagnostic contains:", - " @A.Foo(value2 = {2}) A customSingleton2();", + " @A.Foo(value2 = {2})", + " A customSingleton2();", " // BUG: Diagnostic contains:", - " @Foo(value2 = {2}) A customSingleton3();", + " @Foo(value2 = {2})", + " A customSingleton3();", + "", + " @pkg.A.Foo(value2 = {2, 2})", + " A customPair1();", + "", + " @A.Foo(value2 = {2, 2})", + " A customPair2();", "", - " @pkg.A.Foo(value2 = {2, 2}) A customPair1();", - " @A.Foo(value2 = {2, 2}) A customPair2();", - " @Foo(value2 = {2, 2}) A customPair3();", + " @Foo(value2 = {2, 2})", + " A customPair3();", "", - " @pkg.A.Foo(value = 1, value2 = 2) A extended1();", - " @A.Foo(value = 1, value2 = 2) A extended2();", - " @Foo(value = 1, value2 = 2) A extended3();", + " @pkg.A.Foo(value = 1, value2 = 2)", + " A extended1();", + "", + " @A.Foo(value = 1, value2 = 2)", + " A extended2();", + "", + " @Foo(value = 1, value2 = 2)", + " A extended3();", "", " // BUG: Diagnostic contains:", - " @pkg.A.Foo({1, 1,}) A trailingComma1();", + " @pkg.A.Foo({", + " 1, 1,", + " })", + " A trailingComma1();", " // BUG: Diagnostic contains:", - " @A.Foo({1, 1,}) A trailingComma2();", + " @A.Foo({", + " 1, 1,", + " })", + " A trailingComma2();", " // BUG: Diagnostic contains:", - " @Foo({1, 1,}) A trailingComma3();", + " @Foo({", + " 1, 1,", + " })", + " A trailingComma3();", "}") .doTest(); } @@ -96,28 +143,67 @@ void replacement() { "interface A {", " @interface Foo {", " String[] value() default {};", + "", " int[] value2() default {};", " }", "", - " @pkg.A.Foo() A functional1();", - " @A.Foo() A functional2();", - " @Foo() A functional3();", + " @pkg.A.Foo()", + " A functional1();", + "", + " @A.Foo()", + " A functional2();", + "", + " @Foo()", + " A functional3();", + "", + " @pkg.A.Foo(value = \"foo\")", + " A verbose1();", + "", + " @A.Foo(value = \"a'b\")", + " A verbose2();", + "", + " @Foo(value = \"a\" + \"\\nb\")", + " A verbose3();", "", - " @pkg.A.Foo(value = \"foo\") A verbose1();", - " @A.Foo(value = \"a'b\") A verbose2();", - " @Foo(value = \"a\" + \"\\nb\") A verbose3();", + " @pkg.A.Foo(value = {\"foo\"})", + " A moreVerbose1();", "", - " @pkg.A.Foo(value = {\"foo\"}) A moreVerbose1();", - " @A.Foo(value = {\"a'b\"}) A moreVerbose2();", - " @Foo(value = {\"a\" + \"\\nb\"}) A moreVerbose3();", + " @A.Foo(value = {\"a'b\"})", + " A moreVerbose2();", "", - " @pkg.A.Foo(value = {\"foo\", \"bar\"}, value2 = {2}) A extended1();", - " @A.Foo(value = {\"a'b\", \"c'd\"}, value2 = {2}) A extended2();", - " @Foo(value = {\"a\" + \"\\nb\", \"c\" + \"\\nd\"}, value2 = {2}) A extended3();", + " @Foo(value = {\"a\" + \"\\nb\"})", + " A moreVerbose3();", "", - " @pkg.A.Foo({\"foo\", \"bar\",}) A trailingComma1();", - " @A.Foo({\"a'b\", \"c'd\",}) A trailingComma2();", - " @Foo({\"a\" + \"\\nb\", \"c\" + \"\\nd\",}) A trailingComma3();", + " @pkg.A.Foo(", + " value = {\"foo\", \"bar\"},", + " value2 = {2})", + " A extended1();", + "", + " @A.Foo(", + " value = {\"a'b\", \"c'd\"},", + " value2 = {2})", + " A extended2();", + "", + " @Foo(", + " value = {\"a\" + \"\\nb\", \"c\" + \"\\nd\"},", + " value2 = {2})", + " A extended3();", + "", + " @pkg.A.Foo({", + " \"foo\", \"bar\",", + " })", + " A trailingComma1();", + "", + " @A.Foo({", + " \"a'b\", \"c'd\",", + " })", + " A trailingComma2();", + "", + " @Foo({", + " \"a\" + \"\\nb\",", + " \"c\" + \"\\nd\",", + " })", + " A trailingComma3();", "}") .addOutputLines( "out/pkg/A.java", @@ -128,28 +214,60 @@ void replacement() { "interface A {", " @interface Foo {", " String[] value() default {};", + "", " int[] value2() default {};", " }", "", - " @pkg.A.Foo A functional1();", - " @A.Foo A functional2();", - " @Foo A functional3();", + " @pkg.A.Foo", + " A functional1();", + "", + " @A.Foo", + " A functional2();", + "", + " @Foo", + " A functional3();", + "", + " @pkg.A.Foo(\"foo\")", + " A verbose1();", + "", + " @A.Foo(\"a'b\")", + " A verbose2();", + "", + " @Foo(\"a\" + \"\\nb\")", + " A verbose3();", + "", + " @pkg.A.Foo(\"foo\")", + " A moreVerbose1();", + "", + " @A.Foo(\"a'b\")", + " A moreVerbose2();", + "", + " @Foo(\"a\" + \"\\nb\")", + " A moreVerbose3();", + "", + " @pkg.A.Foo(", + " value = {\"foo\", \"bar\"},", + " value2 = 2)", + " A extended1();", + "", + " @A.Foo(", + " value = {\"a'b\", \"c'd\"},", + " value2 = 2)", + " A extended2();", "", - " @pkg.A.Foo(\"foo\") A verbose1();", - " @A.Foo(\"a'b\") A verbose2();", - " @Foo(\"a\" + \"\\nb\") A verbose3();", + " @Foo(", + " value = {\"a\" + \"\\nb\", \"c\" + \"\\nd\"},", + " value2 = 2)", + " A extended3();", "", - " @pkg.A.Foo(\"foo\") A moreVerbose1();", - " @A.Foo(\"a'b\") A moreVerbose2();", - " @Foo(\"a\" + \"\\nb\") A moreVerbose3();", + " @pkg.A.Foo({\"foo\", \"bar\"})", + " A trailingComma1();", "", - " @pkg.A.Foo(value = {\"foo\", \"bar\"}, value2 = 2) A extended1();", - " @A.Foo(value = {\"a'b\", \"c'd\"}, value2 = 2) A extended2();", - " @Foo(value = {\"a\" + \"\\nb\", \"c\" + \"\\nd\"}, value2 = 2) A extended3();", + " @A.Foo({\"a'b\", \"c'd\"})", + " A trailingComma2();", "", - " @pkg.A.Foo({\"foo\", \"bar\"}) A trailingComma1();", - " @A.Foo({\"a'b\", \"c'd\"}) A trailingComma2();", - " @Foo({\"a\" + \"\\nb\", \"c\" + \"\\nd\"}) A trailingComma3();", + " @Foo({\"a\" + \"\\nb\", \"c\" + \"\\nd\"})", + " A trailingComma3();", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CollectorMutabilityCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CollectorMutabilityCheckTest.java index adc1cec135..8c9f9e76a0 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CollectorMutabilityCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CollectorMutabilityCheckTest.java @@ -19,12 +19,12 @@ void identification() { .addSourceLines( "A.java", "import static com.google.common.collect.ImmutableList.toImmutableList;", - "import static com.google.common.collect.ImmutableSet.toImmutableSet;", "import static com.google.common.collect.ImmutableMap.toImmutableMap;", + "import static com.google.common.collect.ImmutableSet.toImmutableSet;", "import static java.util.stream.Collectors.toCollection;", "import static java.util.stream.Collectors.toList;", - "import static java.util.stream.Collectors.toSet;", "import static java.util.stream.Collectors.toMap;", + "import static java.util.stream.Collectors.toSet;", "", "import java.util.ArrayList;", "import java.util.HashMap;", @@ -137,7 +137,7 @@ void replacementSecondSuggestedFix() { "import reactor.core.publisher.Flux;", "", "class A {", - " void m() {", + " void m() {", " Flux.just(1).collect(Collectors.toList());", " Flux.just(2).collect(toList());", "", @@ -148,7 +148,7 @@ void replacementSecondSuggestedFix() { "", " Stream.of(1).collect(Collectors.toSet());", " Stream.of(2).collect(toSet());", - " }", + " }", "}") .addOutputLines( "A.java", @@ -165,18 +165,35 @@ void replacementSecondSuggestedFix() { "import reactor.core.publisher.Flux;", "", "class A {", - " void m() {", - " Flux.just(1).collect(toCollection(ArrayList::new));", - " Flux.just(2).collect(toCollection(ArrayList::new));", - "", - " Stream.of(\"foo\").collect(Collectors.toMap(String::getBytes, String::length, (a, b) -> { throw new IllegalStateException(); }, HashMap::new));", - " Stream.of(\"bar\").collect(toMap(String::getBytes, String::length, (a, b) -> { throw new IllegalStateException(); }, HashMap::new));", - " Flux.just(\"baz\").collect(Collectors.toMap(String::getBytes, String::length, (a, b) -> b, HashMap::new));", - " Flux.just(\"qux\").collect(toMap(String::getBytes, String::length, (a, b) -> b, HashMap::new));", - "", - " Stream.of(1).collect(toCollection(HashSet::new));", - " Stream.of(2).collect(toCollection(HashSet::new));", - " }", + " void m() {", + " Flux.just(1).collect(toCollection(ArrayList::new));", + " Flux.just(2).collect(toCollection(ArrayList::new));", + "", + " Stream.of(\"foo\")", + " .collect(", + " Collectors.toMap(", + " String::getBytes,", + " String::length,", + " (a, b) -> {", + " throw new IllegalStateException();", + " },", + " HashMap::new));", + " Stream.of(\"bar\")", + " .collect(", + " toMap(", + " String::getBytes,", + " String::length,", + " (a, b) -> {", + " throw new IllegalStateException();", + " },", + " HashMap::new));", + " Flux.just(\"baz\")", + " .collect(Collectors.toMap(String::getBytes, String::length, (a, b) -> b, HashMap::new));", + " Flux.just(\"qux\").collect(toMap(String::getBytes, String::length, (a, b) -> b, HashMap::new));", + "", + " Stream.of(1).collect(toCollection(HashSet::new));", + " Stream.of(2).collect(toCollection(HashSet::new));", + " }", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ErrorProneTestHelperSourceFormatCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ErrorProneTestHelperSourceFormatCheckTest.java new file mode 100644 index 0000000000..c774fe5069 --- /dev/null +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ErrorProneTestHelperSourceFormatCheckTest.java @@ -0,0 +1,151 @@ +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 ErrorProneTestHelperSourceFormatCheckTest { + private final CompilationTestHelper compilationTestHelper = + CompilationTestHelper.newInstance(ErrorProneTestHelperSourceFormatCheck.class, getClass()); + private final BugCheckerRefactoringTestHelper refactoringTestHelper = + BugCheckerRefactoringTestHelper.newInstance( + ErrorProneTestHelperSourceFormatCheck.class, getClass()); + + @Test + void identification() { + compilationTestHelper + .addSourceLines( + "A.java", + "import com.google.errorprone.BugCheckerRefactoringTestHelper;", + "import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;", + "import com.google.errorprone.CompilationTestHelper;", + "import tech.picnic.errorprone.bugpatterns.EmptyMethodCheck;", + "", + "class A {", + " private final CompilationTestHelper compilationTestHelper =", + " CompilationTestHelper.newInstance(EmptyMethodCheck.class, getClass());", + " private final BugCheckerRefactoringTestHelper refactoringTestHelper =", + " BugCheckerRefactoringTestHelper.newInstance(EmptyMethodCheck.class, getClass());", + "", + " void m() {", + " compilationTestHelper", + " // BUG: Diagnostic contains: No source code provided", + " .addSourceLines(\"A.java\")", + " // BUG: Diagnostic contains: Source code is malformed:", + " .addSourceLines(\"B.java\", \"class B {\")", + " // Well-formed code, so not flagged.", + " .addSourceLines(\"C.java\", \"class C {}\")", + " // Malformed code, but not compile-time constant, so not flagged.", + " .addSourceLines(\"D.java\", \"class D {\" + getClass())", + " // BUG: Diagnostic contains: Test code should follow the Google Java style", + " .addSourceLines(\"E.java\", \"class E { }\")", + " .doTest();", + "", + " refactoringTestHelper", + " // BUG: Diagnostic contains: Test code should follow the Google Java style", + " .addInputLines(\"in/A.java\", \"class A { }\")", + " // BUG: Diagnostic contains: Test code should follow the Google Java style", + " .addOutputLines(\"out/A.java\", \"class A { }\")", + " // BUG: Diagnostic contains: Test code should follow the Google Java style", + " .addInputLines(\"in/B.java\", \"import java.util.Map;\", \"\", \"class B {}\")", + " // Unused import, but in an output file, so not flagged.", + " .addOutputLines(\"out/B.java\", \"import java.util.Map;\", \"\", \"class B {}\")", + " .doTest(TestMode.TEXT_MATCH);", + " }", + "}") + .doTest(); + } + + @Test + void replacement() { + /* + * Verifies that import sorting and code formatting is performed unconditionally, while unused + * imports are removed unless part of a `BugCheckerRefactoringTestHelper` expected output file. + */ + refactoringTestHelper + .addInputLines( + "in/A.java", + "import com.google.errorprone.BugCheckerRefactoringTestHelper;", + "import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;", + "import com.google.errorprone.CompilationTestHelper;", + "import tech.picnic.errorprone.bugpatterns.EmptyMethodCheck;", + "", + "class A {", + " private final CompilationTestHelper compilationTestHelper =", + " CompilationTestHelper.newInstance(EmptyMethodCheck.class, getClass());", + " private final BugCheckerRefactoringTestHelper refactoringTestHelper =", + " BugCheckerRefactoringTestHelper.newInstance(EmptyMethodCheck.class, getClass());", + "", + " void m() {", + " compilationTestHelper", + " .addSourceLines(", + " \"A.java\",", + " \"import java.util.Map;\",", + " \"import java.util.Collection;\",", + " \"import java.util.List;\",", + " \"\",", + " \"interface A extends List, Map { }\")", + " .doTest();", + "", + " refactoringTestHelper", + " .addInputLines(", + " \"in/A.java\",", + " \"import java.util.Map;\",", + " \"import java.util.Collection;\",", + " \"import java.util.List;\",", + " \"\",", + " \"interface A extends List, Map { }\")", + " .addOutputLines(", + " \"out/A.java\",", + " \"import java.util.Map;\",", + " \"import java.util.Collection;\",", + " \"import java.util.List;\",", + " \"\",", + " \"interface A extends List, Map { }\")", + " .doTest(TestMode.TEXT_MATCH);", + " }", + "}") + .addOutputLines( + "out/A.java", + "import com.google.errorprone.BugCheckerRefactoringTestHelper;", + "import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;", + "import com.google.errorprone.CompilationTestHelper;", + "import tech.picnic.errorprone.bugpatterns.EmptyMethodCheck;", + "", + "class A {", + " private final CompilationTestHelper compilationTestHelper =", + " CompilationTestHelper.newInstance(EmptyMethodCheck.class, getClass());", + " private final BugCheckerRefactoringTestHelper refactoringTestHelper =", + " BugCheckerRefactoringTestHelper.newInstance(EmptyMethodCheck.class, getClass());", + "", + " void m() {", + " compilationTestHelper", + " .addSourceLines(", + " \"A.java\",", + " \"import java.util.List;\",", + " \"import java.util.Map;\",", + " \"\",", + " \"interface A extends List, Map {}\")", + " .doTest();", + "", + " refactoringTestHelper", + " .addInputLines(", + " \"in/A.java\",", + " \"import java.util.List;\",", + " \"import java.util.Map;\",", + " \"\",", + " \"interface A extends List, Map {}\")", + " .addOutputLines(", + " \"out/A.java\",", + " \"import java.util.Collection;\",", + " \"import java.util.List;\",", + " \"import java.util.Map;\",", + " \"\",", + " \"interface A extends List, Map {}\")", + " .doTest(TestMode.TEXT_MATCH);", + " }", + "}") + .doTest(TestMode.TEXT_MATCH); + } +} diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java index 4e91ab1eb1..5354b1be99 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/ExplicitEnumOrderingCheckTest.java @@ -12,14 +12,14 @@ void Identification() { compilationTestHelper .addSourceLines( "A.java", - "import static java.lang.annotation.RetentionPolicy.SOURCE;", "import static java.lang.annotation.RetentionPolicy.CLASS;", "import static java.lang.annotation.RetentionPolicy.RUNTIME;", + "import static java.lang.annotation.RetentionPolicy.SOURCE;", "import static java.time.chrono.IsoEra.BCE;", "import static java.time.chrono.IsoEra.CE;", "", - "import com.google.common.collect.Ordering;", "import com.google.common.collect.ImmutableList;", + "import com.google.common.collect.Ordering;", "import java.lang.annotation.RetentionPolicy;", "import java.time.chrono.IsoEra;", "", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java index e6dff10e8a..235e0b029c 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FluxFlatMapUsageCheckTest.java @@ -20,8 +20,8 @@ void identification() { "A.java", "import java.util.function.BiFunction;", "import java.util.function.Function;", - "import reactor.core.publisher.Mono;", "import reactor.core.publisher.Flux;", + "import reactor.core.publisher.Mono;", "", "class A {", " void m() {", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenationCheckTest.java index 68c6e0adea..79624adcf7 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenationCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenationCheckTest.java @@ -23,8 +23,8 @@ void identification() { "import static org.assertj.core.api.Assertions.assertThat;", "import static org.assertj.core.api.SoftAssertions.assertSoftly;", "", - "import java.util.Locale;", "import java.util.Formatter;", + "import java.util.Locale;", "import org.assertj.core.api.Assertions;", "import org.assertj.core.api.BDDAssertions;", "import org.assertj.core.api.Fail;", @@ -269,9 +269,9 @@ void identification() { " LOG.error(\"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", - " LOG.error((Marker) null,\"str \" + toString());", + " LOG.error((Marker) null, \"str \" + toString());", " // BUG: Diagnostic contains:", - " LOG.error((Marker) null,\"{} \" + toString(), \"arg\");", + " LOG.error((Marker) null, \"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", " LOG.info(\"str \" + toString());", @@ -279,9 +279,9 @@ void identification() { " LOG.info(\"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", - " LOG.info((Marker) null,\"str \" + toString());", + " LOG.info((Marker) null, \"str \" + toString());", " // BUG: Diagnostic contains:", - " LOG.info((Marker) null,\"{} \" + toString(), \"arg\");", + " LOG.info((Marker) null, \"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", " LOG.trace(\"str \" + toString());", @@ -289,9 +289,9 @@ void identification() { " LOG.trace(\"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", - " LOG.trace((Marker) null,\"str \" + toString());", + " LOG.trace((Marker) null, \"str \" + toString());", " // BUG: Diagnostic contains:", - " LOG.trace((Marker) null,\"{} \" + toString(), \"arg\");", + " LOG.trace((Marker) null, \"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", " LOG.warn(\"str \" + toString());", @@ -299,9 +299,9 @@ void identification() { " LOG.warn(\"{} \" + toString(), \"arg\");", "", " // BUG: Diagnostic contains:", - " LOG.warn((Marker) null,\"str \" + toString());", + " LOG.warn((Marker) null, \"str \" + toString());", " // BUG: Diagnostic contains:", - " LOG.warn((Marker) null,\"{} \" + toString(), \"arg\");", + " LOG.warn((Marker) null, \"{} \" + toString(), \"arg\");", " }", "}") .doTest(); @@ -316,7 +316,6 @@ void replacement() { "import static org.assertj.core.api.Assertions.assertThat;", "", "import java.util.Locale;", - "import java.util.Formatter;", "import org.slf4j.Logger;", "import org.slf4j.LoggerFactory;", "import org.slf4j.Marker;", @@ -366,7 +365,6 @@ void replacement() { "import static org.assertj.core.api.Assertions.assertThat;", "", "import java.util.Locale;", - "import java.util.Formatter;", "import org.slf4j.Logger;", "import org.slf4j.LoggerFactory;", "import org.slf4j.Marker;", @@ -400,6 +398,7 @@ void replacement() { " String.format(\"{} \" + toString(), \"arg\");", " String.format(Locale.ROOT, \"{} \" + toString(), \"arg\");", " }", + "", " void slf4j() {", " LOG.debug(\"str {}\", toString());", " LOG.debug((Marker) null, \"str {}\", toString());", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionCheckTest.java index f71546197c..04ae2b959e 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/IdentityConversionCheckTest.java @@ -121,8 +121,9 @@ void identification() { " ImmutableBiMap o1 = ImmutableBiMap.copyOf(ImmutableBiMap.of());", " // BUG: Diagnostic contains:", " ImmutableList o2 = ImmutableList.copyOf(ImmutableList.of());", - " // BUG: Diagnostic contains:", - " ImmutableListMultimap o3 = ImmutableListMultimap.copyOf(ImmutableListMultimap.of());", + " ImmutableListMultimap o3 =", + " // BUG: Diagnostic contains:", + " ImmutableListMultimap.copyOf(ImmutableListMultimap.of());", " // BUG: Diagnostic contains:", " ImmutableMap o4 = ImmutableMap.copyOf(ImmutableMap.of());", " // BUG: Diagnostic contains:", @@ -135,8 +136,9 @@ void identification() { " ImmutableRangeSet o8 = ImmutableRangeSet.copyOf(ImmutableRangeSet.of());", " // BUG: Diagnostic contains:", " ImmutableSet o9 = ImmutableSet.copyOf(ImmutableSet.of());", - " // BUG: Diagnostic contains:", - " ImmutableSetMultimap o10 = ImmutableSetMultimap.copyOf(ImmutableSetMultimap.of());", + " ImmutableSetMultimap o10 =", + " // BUG: Diagnostic contains:", + " ImmutableSetMultimap.copyOf(ImmutableSetMultimap.of());", " // BUG: Diagnostic contains:", " ImmutableTable o11 = ImmutableTable.copyOf(ImmutableTable.of());", "", @@ -171,8 +173,8 @@ void replacementFirstSuggestedFix() { "import com.google.common.collect.ImmutableCollection;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", - "import java.util.Collection;", "import java.util.ArrayList;", + "import java.util.Collection;", "import org.reactivestreams.Publisher;", "import reactor.adapter.rxjava.RxJava2Adapter;", "import reactor.core.publisher.Flux;", @@ -204,8 +206,7 @@ void replacementFirstSuggestedFix() { " Object o1 = ImmutableSet.copyOf(ImmutableList.of());", " Object o2 = ImmutableSet.copyOf(ImmutableSet.of());", "", - " when(\"foo\".contains(\"f\"))", - " .thenAnswer(inv-> ImmutableSet.copyOf(ImmutableList.of(1)));", + " when(\"foo\".contains(\"f\")).thenAnswer(inv -> ImmutableSet.copyOf(ImmutableList.of(1)));", " }", "", " void bar(Publisher publisher) {}", @@ -217,8 +218,8 @@ void replacementFirstSuggestedFix() { "import com.google.common.collect.ImmutableCollection;", "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", - "import java.util.Collection;", "import java.util.ArrayList;", + "import java.util.Collection;", "import org.reactivestreams.Publisher;", "import reactor.adapter.rxjava.RxJava2Adapter;", "import reactor.core.publisher.Flux;", @@ -250,8 +251,7 @@ void replacementFirstSuggestedFix() { " Object o1 = ImmutableSet.copyOf(ImmutableList.of());", " Object o2 = ImmutableSet.of();", "", - " when(\"foo\".contains(\"f\"))", - " .thenAnswer(inv-> ImmutableSet.copyOf(ImmutableList.of(1)));", + " when(\"foo\".contains(\"f\")).thenAnswer(inv -> ImmutableSet.copyOf(ImmutableList.of(1)));", " }", "", " void bar(Publisher publisher) {}", @@ -269,9 +269,6 @@ void replacementSecondSuggestedFix() { "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", "import java.util.ArrayList;", - "import reactor.adapter.rxjava.RxJava2Adapter;", - "import reactor.core.publisher.Flux;", - "import reactor.core.publisher.Mono;", "", "public final class Foo {", " public void foo() {", @@ -288,9 +285,6 @@ void replacementSecondSuggestedFix() { "import com.google.common.collect.ImmutableList;", "import com.google.common.collect.ImmutableSet;", "import java.util.ArrayList;", - "import reactor.adapter.rxjava.RxJava2Adapter;", - "import reactor.core.publisher.Flux;", - "import reactor.core.publisher.Mono;", "", "public final class Foo {", " public void foo() {", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java index 088d5cbe1e..a6c66bf46f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/JUnitMethodDeclarationCheckTest.java @@ -26,71 +26,137 @@ void identification() { "import org.junit.jupiter.params.ParameterizedTest;", "", "class A {", - " @BeforeAll void setUp1() {}", + " {", + " arguments();", + " }", + "", + " @BeforeAll", + " void setUp1() {}", + "", + " @BeforeAll", " // BUG: Diagnostic contains:", - " @BeforeAll public void setUp2() {}", + " public void setUp2() {}", + "", + " @BeforeAll", " // BUG: Diagnostic contains:", - " @BeforeAll protected void setUp3() {}", + " protected void setUp3() {}", + "", + " @BeforeAll", " // BUG: Diagnostic contains:", - " @BeforeAll private void setUp4() {}", + " private void setUp4() {}", + "", + " @BeforeEach", + " void setup5() {}", "", - " @BeforeEach void setup5() {}", + " @BeforeEach", " // BUG: Diagnostic contains:", - " @BeforeEach public void setUp6() {}", + " public void setUp6() {}", + "", + " @BeforeEach", " // BUG: Diagnostic contains:", - " @BeforeEach protected void setUp7() {}", + " protected void setUp7() {}", + "", + " @BeforeEach", " // BUG: Diagnostic contains:", - " @BeforeEach private void setUp8() {}", + " private void setUp8() {}", + "", + " @AfterEach", + " void tearDown1() {}", "", - " @AfterEach void tearDown1() {}", + " @AfterEach", " // BUG: Diagnostic contains:", - " @AfterEach public void tearDown2() {}", + " public void tearDown2() {}", + "", + " @AfterEach", " // BUG: Diagnostic contains:", - " @AfterEach protected void tearDown3() {}", + " protected void tearDown3() {}", + "", + " @AfterEach", " // BUG: Diagnostic contains:", - " @AfterEach private void tearDown4() {}", + " private void tearDown4() {}", "", - " @AfterAll void tearDown5() {}", + " @AfterAll", + " void tearDown5() {}", + "", + " @AfterAll", " // BUG: Diagnostic contains:", - " @AfterAll public void tearDown6() {}", + " public void tearDown6() {}", + "", + " @AfterAll", " // BUG: Diagnostic contains:", - " @AfterAll protected void tearDown7() {}", + " protected void tearDown7() {}", + "", + " @AfterAll", " // BUG: Diagnostic contains:", - " @AfterAll private void tearDown8() {}", + " private void tearDown8() {}", "", - " @Test void method1() {}", + " @Test", + " void method1() {}", + "", + " @Test", " // BUG: Diagnostic contains:", - " @Test void testMethod2() {}", + " void testMethod2() {}", + "", + " @Test", " // BUG: Diagnostic contains:", - " @Test public void method3() {}", + " public void method3() {}", + "", + " @Test", " // BUG: Diagnostic contains:", - " @Test protected void method4() {}", + " protected void method4() {}", + "", + " @Test", " // BUG: Diagnostic contains:", - " @Test private void method5() {}", + " private void method5() {}", + "", + " @ParameterizedTest", + " void method6() {}", "", - " @ParameterizedTest void method6() {}", + " @ParameterizedTest", " // BUG: Diagnostic contains:", - " @ParameterizedTest void testMethod7() {}", + " void testMethod7() {}", + "", + " @ParameterizedTest", " // BUG: Diagnostic contains:", - " @ParameterizedTest public void method8() {}", + " public void method8() {}", + "", + " @ParameterizedTest", " // BUG: Diagnostic contains:", - " @ParameterizedTest protected void method9() {}", + " protected void method9() {}", + "", + " @ParameterizedTest", " // BUG: Diagnostic contains:", - " @ParameterizedTest private void method10() {}", + " private void method10() {}", + "", + " @BeforeEach", + " @BeforeAll", + " @AfterEach", + " @AfterAll", + " void testNonTestMethod1() {}", "", - " @BeforeEach @BeforeAll @AfterEach @AfterAll void testNonTestMethod1() {}", " public void testNonTestMethod2() {}", + "", " protected void testNonTestMethod3() {}", + "", " private void testNonTestMethod4() {}", - " @Test void test5() {}", "", - " // BUG: Diagnostic contains: (but note that a method named `overload` already exists in this class)", - " @Test void testOverload() {}", + " @Test", + " void test5() {}", + "", + " @Test", + " // BUG: Diagnostic contains: (but note that a method named `overload` already exists in this", + " // class)", + " void testOverload() {}", + "", " void overload() {}", + "", + " @Test", " // BUG: Diagnostic contains: (but note that `arguments` is already statically imported)", - " @Test void testArguments() {}", + " void testArguments() {}", + "", + " @Test", " // BUG: Diagnostic contains: (but note that `public` is a reserved keyword)", - " @Test void testPublic() {}", + " void testPublic() {}", "}") .addSourceLines( "B.java", @@ -102,41 +168,117 @@ void identification() { "import org.junit.jupiter.params.ParameterizedTest;", "", "class B extends A {", - " @Override @BeforeAll void setUp1() {}", - " @Override @BeforeAll public void setUp2() {}", - " @Override @BeforeAll protected void setUp3() {}", - "", - " @Override @BeforeEach void setup5() {}", - " @Override @BeforeEach public void setUp6() {}", - " @Override @BeforeEach protected void setUp7() {}", - "", - " @Override @AfterEach void tearDown1() {}", - " @Override @AfterEach public void tearDown2() {}", - " @Override @AfterEach protected void tearDown3() {}", - "", - " @Override @AfterAll void tearDown5() {}", - " @Override @AfterAll public void tearDown6() {}", - " @Override @AfterAll protected void tearDown7() {}", - "", - " @Override @Test void method1() {}", - " @Override @Test void testMethod2() {}", - " @Override @Test public void method3() {}", - " @Override @Test protected void method4() {}", - "", - " @Override @ParameterizedTest void method6() {}", - " @Override @ParameterizedTest void testMethod7() {}", - " @Override @ParameterizedTest public void method8() {}", - " @Override @ParameterizedTest protected void method9() {}", - "", - " @Override @BeforeEach @BeforeAll @AfterEach @AfterAll void testNonTestMethod1() {}", - " @Override public void testNonTestMethod2() {}", - " @Override protected void testNonTestMethod3() {}", - " @Override @Test void test5() {}", - "", - " @Override @Test void testOverload() {}", - " @Override void overload() {}", - " @Override @Test void testArguments() {}", - " @Override @Test void testPublic() {}", + " @Override", + " @BeforeAll", + " void setUp1() {}", + "", + " @Override", + " @BeforeAll", + " public void setUp2() {}", + "", + " @Override", + " @BeforeAll", + " protected void setUp3() {}", + "", + " @Override", + " @BeforeEach", + " void setup5() {}", + "", + " @Override", + " @BeforeEach", + " public void setUp6() {}", + "", + " @Override", + " @BeforeEach", + " protected void setUp7() {}", + "", + " @Override", + " @AfterEach", + " void tearDown1() {}", + "", + " @Override", + " @AfterEach", + " public void tearDown2() {}", + "", + " @Override", + " @AfterEach", + " protected void tearDown3() {}", + "", + " @Override", + " @AfterAll", + " void tearDown5() {}", + "", + " @Override", + " @AfterAll", + " public void tearDown6() {}", + "", + " @Override", + " @AfterAll", + " protected void tearDown7() {}", + "", + " @Override", + " @Test", + " void method1() {}", + "", + " @Override", + " @Test", + " void testMethod2() {}", + "", + " @Override", + " @Test", + " public void method3() {}", + "", + " @Override", + " @Test", + " protected void method4() {}", + "", + " @Override", + " @ParameterizedTest", + " void method6() {}", + "", + " @Override", + " @ParameterizedTest", + " void testMethod7() {}", + "", + " @Override", + " @ParameterizedTest", + " public void method8() {}", + "", + " @Override", + " @ParameterizedTest", + " protected void method9() {}", + "", + " @Override", + " @BeforeEach", + " @BeforeAll", + " @AfterEach", + " @AfterAll", + " void testNonTestMethod1() {}", + "", + " @Override", + " public void testNonTestMethod2() {}", + "", + " @Override", + " protected void testNonTestMethod3() {}", + "", + " @Override", + " @Test", + " void test5() {}", + "", + " @Override", + " @Test", + " void testOverload() {}", + "", + " @Override", + " void overload() {}", + "", + " @Override", + " @Test", + " void testArguments() {}", + "", + " @Override", + " @Test", + " void testPublic() {}", "}") .addSourceLines( "C.java", @@ -145,13 +287,19 @@ void identification() { "import org.junit.jupiter.api.Test;", "", "abstract class C {", - " @BeforeAll public void setUp() {}", - " @Test void testMethod1() {}", + " @BeforeAll", + " public void setUp() {}", "", + " @Test", + " void testMethod1() {}", + "", + " @AfterAll", " // BUG: Diagnostic contains:", - " @AfterAll private void tearDown() {}", + " private void tearDown() {}", + "", + " @Test", " // BUG: Diagnostic contains:", - " @Test final void testMethod2() {}", + " final void testMethod2() {}", "}") .doTest(); } @@ -172,22 +320,47 @@ void replacement() { "import org.junit.jupiter.params.ParameterizedTest;", "", "class A {", - " @BeforeAll public void setUp1() {}", - " @BeforeEach protected void setUp2() {}", - " @AfterEach private void setUp3() {}", - " @AfterAll private void setUp4() {}", + " {", + " arguments();", + " }", "", - " @Test void testFoo() {}", - " @ParameterizedTest void testBar() {}", + " @BeforeAll", + " public void setUp1() {}", "", - " @Test public void baz() {}", - " @RepeatedTest(2) private void qux() {}", - " @ParameterizedTest protected void quux() {}", + " @BeforeEach", + " protected void setUp2() {}", + "", + " @AfterEach", + " private void setUp3() {}", + "", + " @AfterAll", + " private void setUp4() {}", + "", + " @Test", + " void testFoo() {}", + "", + " @ParameterizedTest", + " void testBar() {}", + "", + " @Test", + " public void baz() {}", + "", + " @RepeatedTest(2)", + " private void qux() {}", + "", + " @ParameterizedTest", + " protected void quux() {}", + "", + " @Test", + " public void testOverload() {}", "", - " @Test public void testOverload() {}", " void overload() {}", - " @Test protected void testArguments() {}", - " @Test private void testClass() {}", + "", + " @Test", + " protected void testArguments() {}", + "", + " @Test", + " private void testClass() {}", "}") .addOutputLines( "out/A.java", @@ -202,22 +375,47 @@ void replacement() { "import org.junit.jupiter.params.ParameterizedTest;", "", "class A {", - " @BeforeAll void setUp1() {}", - " @BeforeEach void setUp2() {}", - " @AfterEach void setUp3() {}", - " @AfterAll void setUp4() {}", + " {", + " arguments();", + " }", "", - " @Test void foo() {}", - " @ParameterizedTest void bar() {}", + " @BeforeAll", + " void setUp1() {}", "", - " @Test void baz() {}", - " @RepeatedTest(2) void qux() {}", - " @ParameterizedTest void quux() {}", + " @BeforeEach", + " void setUp2() {}", + "", + " @AfterEach", + " void setUp3() {}", + "", + " @AfterAll", + " void setUp4() {}", + "", + " @Test", + " void foo() {}", + "", + " @ParameterizedTest", + " void bar() {}", + "", + " @Test", + " void baz() {}", + "", + " @RepeatedTest(2)", + " void qux() {}", + "", + " @ParameterizedTest", + " void quux() {}", + "", + " @Test", + " void testOverload() {}", "", - " @Test void testOverload() {}", " void overload() {}", - " @Test void testArguments() {}", - " @Test void testClass() {}", + "", + " @Test", + " void testArguments() {}", + "", + " @Test", + " void testClass() {}", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingCheckTest.java index 4b3bc78381..b7c4c2ae5f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationAttributeListingCheckTest.java @@ -26,8 +26,8 @@ void identification() { compilationTestHelper .addSourceLines( "A.java", - "import static java.math.RoundingMode.UP;", "import static java.math.RoundingMode.DOWN;", + "import static java.math.RoundingMode.UP;", "", "import com.fasterxml.jackson.annotation.JsonPropertyOrder;", "import io.swagger.annotations.ApiImplicitParam;", @@ -40,9 +40,13 @@ void identification() { "interface A {", " @interface Foo {", " String[] value() default {};", + "", " int[] ints() default {};", + "", " Class[] cls() default {};", + "", " RoundingMode[] enums() default {};", + "", " Bar[] anns() default {};", " }", "", @@ -50,50 +54,95 @@ void identification() { " String[] value() default {};", " }", "", - " @Foo({}) A noString();", - " @Foo({\"a\"}) A oneString();", - " @Foo({\"a\", \"b\"}) A sortedStrings();", + " @Foo({})", + " A noString();", + "", + " @Foo({\"a\"})", + " A oneString();", + "", + " @Foo({\"a\", \"b\"})", + " A sortedStrings();", " // BUG: Diagnostic contains:", - " @Foo({\"b\", \"a\"}) A unsortedString();", - " @Foo({\"ab\", \"Ac\"}) A sortedStringCaseInsensitive();", + " @Foo({\"b\", \"a\"})", + " A unsortedString();", + "", + " @Foo({\"ab\", \"Ac\"})", + " A sortedStringCaseInsensitive();", " // BUG: Diagnostic contains:", - " @Foo({\"ac\", \"Ab\"}) A unsortedStringCaseInsensitive();", - " @Foo({\"A\", \"a\"}) A sortedStringCaseInsensitiveWithTotalOrderFallback();", + " @Foo({\"ac\", \"Ab\"})", + " A unsortedStringCaseInsensitive();", + "", + " @Foo({\"A\", \"a\"})", + " A sortedStringCaseInsensitiveWithTotalOrderFallback();", " // BUG: Diagnostic contains:", - " @Foo({\"a\", \"A\"}) A unsortedStringCaseInsensitiveWithTotalOrderFallback();", + " @Foo({\"a\", \"A\"})", + " A unsortedStringCaseInsensitiveWithTotalOrderFallback();", + "", + " @Foo(ints = {})", + " A noInts();", + "", + " @Foo(ints = {0})", + " A oneInt();", "", - " @Foo(ints = {}) A noInts();", - " @Foo(ints = {0}) A oneInt();", - " @Foo(ints = {0, 1}) A sortedInts();", - " @Foo(ints = {1, 0}) A unsortedInts();", + " @Foo(ints = {0, 1})", + " A sortedInts();", "", - " @Foo(cls = {}) A noClasses();", - " @Foo(cls = {int.class}) A oneClass();", - " @Foo(cls = {int.class, long.class}) A sortedClasses();", + " @Foo(ints = {1, 0})", + " A unsortedInts();", + "", + " @Foo(cls = {})", + " A noClasses();", + "", + " @Foo(cls = {int.class})", + " A oneClass();", + "", + " @Foo(cls = {int.class, long.class})", + " A sortedClasses();", " // BUG: Diagnostic contains:", - " @Foo(cls = {long.class, int.class}) A unsortedClasses();", + " @Foo(cls = {long.class, int.class})", + " A unsortedClasses();", "", - " @Foo(enums = {}) A noEnums();", - " @Foo(enums = {DOWN}) A oneEnum();", - " @Foo(enums = {DOWN, UP}) A sortedEnums();", + " @Foo(enums = {})", + " A noEnums();", + "", + " @Foo(enums = {DOWN})", + " A oneEnum();", + "", + " @Foo(enums = {DOWN, UP})", + " A sortedEnums();", " // BUG: Diagnostic contains:", - " @Foo(enums = {UP, DOWN}) A unsortedEnums();", + " @Foo(enums = {UP, DOWN})", + " A unsortedEnums();", + "", + " @Foo(anns = {})", + " A noAnns();", "", - " @Foo(anns = {}) A noAnns();", - " @Foo(anns = {@Bar(\"a\")}) A oneAnn();", - " @Foo(anns = {@Bar(\"a\"), @Bar(\"b\")}) A sortedAnns();", + " @Foo(anns = {@Bar(\"a\")})", + " A oneAnn();", + "", + " @Foo(anns = {@Bar(\"a\"), @Bar(\"b\")})", + " A sortedAnns();", " // BUG: Diagnostic contains:", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) A unsortedAnns();", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " A unsortedAnns();", " // BUG: Diagnostic contains:", - " @Foo(anns = {@Bar(\"a\"), @Bar({\"b\", \"a\"})}) A unsortedInnderAnns();", + " @Foo(anns = {@Bar(\"a\"), @Bar({\"b\", \"a\"})})", + " A unsortedInnderAnns();", "", - " @Foo({\"a=foo\", \"a.b=bar\", \"a.c=baz\"}) A hierarchicallySorted();", + " @Foo({\"a=foo\", \"a.b=bar\", \"a.c=baz\"})", + " A hierarchicallySorted();", " // BUG: Diagnostic contains:", - " @Foo({\"a.b=bar\", \"a.c=baz\", \"a=foo\"}) A hierarchicallyUnsorted();", + " @Foo({\"a.b=bar\", \"a.c=baz\", \"a=foo\"})", + " A hierarchicallyUnsorted();", + "", + " @JsonPropertyOrder({\"field2\", \"field1\"})", + " A dto();", + "", + " @ApiImplicitParams({@ApiImplicitParam(\"p2\"), @ApiImplicitParam(\"p1\")})", + " A firstEndpoint();", "", - " @JsonPropertyOrder({\"field2\", \"field1\"}) A dto();", - " @ApiImplicitParams({@ApiImplicitParam(\"p2\"), @ApiImplicitParam(\"p1\")}) A firstEndpoint();", - " @Parameters({@Parameter(name = \"p2\"), @Parameter(name = \"p1\")}) A secondEndpoint();", + " @Parameters({@Parameter(name = \"p2\"), @Parameter(name = \"p1\")})", + " A secondEndpoint();", "", " @XmlType(propOrder = {\"field2\", \"field1\"})", " class Dummy {}", @@ -109,16 +158,19 @@ void replacement() { refactoringTestHelper .addInputLines( "in/A.java", - "import static java.math.RoundingMode.UP;", "import static java.math.RoundingMode.DOWN;", + "import static java.math.RoundingMode.UP;", "", "import java.math.RoundingMode;", "", "interface A {", " @interface Foo {", " String[] value() default {};", + "", " Class[] cls() default {};", + "", " RoundingMode[] enums() default {};", + "", " Bar[] anns() default {};", " }", "", @@ -126,24 +178,36 @@ void replacement() { " String[] value() default {};", " }", "", - " @Foo({\"b\", \"a\"}) A unsortedString();", - " @Foo(cls = {long.class, int.class}) A unsortedClasses();", - " @Foo(enums = {UP, DOWN}) A unsortedEnums();", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) A unsortedAnns();", - " @Foo(anns = {@Bar(\"a\"), @Bar({\"b\", \"a\"})}) A unsortedInnderAnns();", + " @Foo({\"b\", \"a\"})", + " A unsortedString();", + "", + " @Foo(cls = {long.class, int.class})", + " A unsortedClasses();", + "", + " @Foo(enums = {UP, DOWN})", + " A unsortedEnums();", + "", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " A unsortedAnns();", + "", + " @Foo(anns = {@Bar(\"a\"), @Bar({\"b\", \"a\"})})", + " A unsortedInnderAnns();", "}") .addOutputLines( "out/A.java", - "import static java.math.RoundingMode.UP;", "import static java.math.RoundingMode.DOWN;", + "import static java.math.RoundingMode.UP;", "", "import java.math.RoundingMode;", "", "interface A {", " @interface Foo {", " String[] value() default {};", + "", " Class[] cls() default {};", + "", " RoundingMode[] enums() default {};", + "", " Bar[] anns() default {};", " }", "", @@ -151,11 +215,20 @@ void replacement() { " String[] value() default {};", " }", "", - " @Foo({\"a\", \"b\"}) A unsortedString();", - " @Foo(cls = {int.class, long.class}) A unsortedClasses();", - " @Foo(enums = {DOWN, UP}) A unsortedEnums();", - " @Foo(anns = {@Bar(\"a\"), @Bar(\"b\")}) A unsortedAnns();", - " @Foo(anns = {@Bar(\"a\"), @Bar({\"a\", \"b\"})}) A unsortedInnderAnns();", + " @Foo({\"a\", \"b\"})", + " A unsortedString();", + "", + " @Foo(cls = {int.class, long.class})", + " A unsortedClasses();", + "", + " @Foo(enums = {DOWN, UP})", + " A unsortedEnums();", + "", + " @Foo(anns = {@Bar(\"a\"), @Bar(\"b\")})", + " A unsortedAnns();", + "", + " @Foo(anns = {@Bar(\"a\"), @Bar({\"a\", \"b\"})})", + " A unsortedInnderAnns();", "}") .doTest(TestMode.TEXT_MATCH); } @@ -171,28 +244,40 @@ void filtering() { "interface A {", " @interface Foo {", " String[] value() default {};", + "", " String[] value2() default {};", " }", "", " @interface Bar {", " String[] value() default {};", + "", " String[] value2() default {};", " }", "", " @interface Baz {", " String[] value() default {};", + "", " String[] value2() default {};", " }", "", " // BUG: Diagnostic contains:", - " @Foo({\"b\", \"a\"}) A fooValue();", + " @Foo({\"b\", \"a\"})", + " A fooValue();", " // BUG: Diagnostic contains:", - " @Foo(value2 = {\"b\", \"a\"}) A fooValue2();", - " @Bar({\"b\", \"a\"}) A barValue();", + " @Foo(value2 = {\"b\", \"a\"})", + " A fooValue2();", + "", + " @Bar({\"b\", \"a\"})", + " A barValue();", " // BUG: Diagnostic contains:", - " @Bar(value2 = {\"b\", \"a\"}) A barValue2();", - " @Baz({\"b\", \"a\"}) A bazValue();", - " @Baz(value2 = {\"b\", \"a\"}) A bazValue2();", + " @Bar(value2 = {\"b\", \"a\"})", + " A barValue2();", + "", + " @Baz({\"b\", \"a\"})", + " A bazValue();", + "", + " @Baz(value2 = {\"b\", \"a\"})", + " A bazValue2();", "}") .doTest(); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheckTest.java index e251863eb5..ad644631d6 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/LexicographicalAnnotationListingCheckTest.java @@ -27,7 +27,9 @@ void identification() { " @Repeatable(Foos.class)", " @interface Foo {", " String[] value() default {};", + "", " int[] ints() default {};", + "", " Bar[] anns() default {};", " }", "", @@ -40,33 +42,70 @@ void identification() { " }", "", " @interface Foos {", - " Foo[] value();", + " Foo[] value();", " }", "", " // BUG: Diagnostic matches: X", - " @Foo @Bar A unsortedSimpleCase();", + " @Foo", + " @Bar", + " A unsortedSimpleCase();", " // BUG: Diagnostic matches: X", - " @Foo() @Bar() A unsortedWithParens();", - " @Foo() A onlyOneAnnotation();", - " @Bar @Foo() A sortedAnnotationsOneWithParens();", + " @Foo()", + " @Bar()", + " A unsortedWithParens();", + "", + " @Foo()", + " A onlyOneAnnotation();", + "", + " @Bar", + " @Foo()", + " A sortedAnnotationsOneWithParens();", "", " // BUG: Diagnostic matches: X", - " @Foo @Baz @Bar A threeUnsortedAnnotationsSameInitialLetter();", + " @Foo", + " @Baz", + " @Bar", + " A threeUnsortedAnnotationsSameInitialLetter();", " // BUG: Diagnostic matches: X", - " @Bar @Foo() @Baz A firstOrderedWithTwoUnsortedAnnotations();", - " @Bar @Baz @Foo() A threeSortedAnnotations();", + " @Bar", + " @Foo()", + " @Baz", + " A firstOrderedWithTwoUnsortedAnnotations();", + "", + " @Bar", + " @Baz", + " @Foo()", + " A threeSortedAnnotations();", "", " // BUG: Diagnostic matches: X", - " @Foo({\"b\"}) @Bar({\"a\"}) A unsortedWithStringAttributes();", + " @Foo({\"b\"})", + " @Bar({\"a\"})", + " A unsortedWithStringAttributes();", " // BUG: Diagnostic matches: X", - " @Baz(str = {\"a\", \"b\"}) @Foo(ints = {1, 0}) @Bar A unsortedWithAttributes();", + " @Baz(str = {\"a\", \"b\"})", + " @Foo(ints = {1, 0})", + " @Bar", + " A unsortedWithAttributes();", " // BUG: Diagnostic matches: X", - " @Bar @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Baz A unsortedWithNestedBar();", - " @Bar @Baz @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) A sortedWithNestedBar();", - "", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Foo(ints = {1, 2}) @Foo({\"b\"}) A sortedRepeatableAnnotation();", + " @Bar", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Baz", + " A unsortedWithNestedBar();", + "", + " @Bar", + " @Baz", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " A sortedWithNestedBar();", + "", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Foo(ints = {1, 2})", + " @Foo({\"b\"})", + " A sortedRepeatableAnnotation();", " // BUG: Diagnostic matches: X", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Bar @Foo(ints = {1, 2}) A unsortedRepeatableAnnotation();", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Bar", + " @Foo(ints = {1, 2})", + " A unsortedRepeatableAnnotation();", "}") .doTest(); } @@ -77,11 +116,14 @@ void replacement() { .addInputLines( "in/A.java", "import java.lang.annotation.Repeatable;", + "", "interface A {", " @Repeatable(Foos.class)", " @interface Foo {", " String[] value() default {};", + "", " int[] ints() default {};", + "", " Bar[] anns() default {};", " }", "", @@ -94,29 +136,56 @@ void replacement() { " }", "", " @interface Foos {", - " Foo[] value();", + " Foo[] value();", " }", "", - " @Bar A singleAnnotation();", - " @Bar @Foo A sortedAnnotations();", - " @Foo @Bar A unsortedAnnotations();", - " @Foo() @Baz() @Bar A unsortedAnnotationsWithSomeParens();", - "", - " @Bar @Baz(str = {\"a\", \"b\"}) @Foo() A unsortedAnnotationsOneContainingAttributes();", - " @Baz(str = {\"a\", \"b\"}) @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Bar({\"b\"}) A unsortedAnnotationsWithAttributes();", - "", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Foo(ints = {1, 2}) @Foo({\"b\"}) A sortedRepeatableAnnotation();", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Bar @Foo(ints = {1, 2}) A unsortedRepeatableAnnotation();", - "", + " @Bar", + " A singleAnnotation();", + "", + " @Bar", + " @Foo", + " A sortedAnnotations();", + "", + " @Foo", + " @Bar", + " A unsortedAnnotations();", + "", + " @Foo()", + " @Baz()", + " @Bar", + " A unsortedAnnotationsWithSomeParens();", + "", + " @Bar", + " @Baz(str = {\"a\", \"b\"})", + " @Foo()", + " A unsortedAnnotationsOneContainingAttributes();", + "", + " @Baz(str = {\"a\", \"b\"})", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Bar({\"b\"})", + " A unsortedAnnotationsWithAttributes();", + "", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Foo(ints = {1, 2})", + " @Foo({\"b\"})", + " A sortedRepeatableAnnotation();", + "", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Bar", + " @Foo(ints = {1, 2})", + " A unsortedRepeatableAnnotation();", "}") .addOutputLines( "out/A.java", "import java.lang.annotation.Repeatable;", + "", "interface A {", " @Repeatable(Foos.class)", " @interface Foo {", " String[] value() default {};", + "", " int[] ints() default {};", + "", " Bar[] anns() default {};", " }", "", @@ -129,19 +198,44 @@ void replacement() { " }", "", " @interface Foos {", - " Foo[] value();", + " Foo[] value();", " }", - " @Bar A singleAnnotation();", - " @Bar @Foo A sortedAnnotations();", - " @Bar @Foo A unsortedAnnotations();", - " @Bar @Baz() @Foo() A unsortedAnnotationsWithSomeParens();", - "", - " @Bar @Baz(str = {\"a\", \"b\"}) @Foo() A unsortedAnnotationsOneContainingAttributes();", - " @Bar({\"b\"}) @Baz(str = {\"a\", \"b\"}) @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) A unsortedAnnotationsWithAttributes();", - "", - " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Foo(ints = {1, 2}) @Foo({\"b\"}) A sortedRepeatableAnnotation();", - " @Bar @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")}) @Foo(ints = {1, 2}) A unsortedRepeatableAnnotation();", "", + " @Bar", + " A singleAnnotation();", + "", + " @Bar", + " @Foo", + " A sortedAnnotations();", + "", + " @Bar", + " @Foo", + " A unsortedAnnotations();", + "", + " @Bar", + " @Baz()", + " @Foo()", + " A unsortedAnnotationsWithSomeParens();", + "", + " @Bar", + " @Baz(str = {\"a\", \"b\"})", + " @Foo()", + " A unsortedAnnotationsOneContainingAttributes();", + "", + " @Bar({\"b\"})", + " @Baz(str = {\"a\", \"b\"})", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " A unsortedAnnotationsWithAttributes();", + "", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Foo(ints = {1, 2})", + " @Foo({\"b\"})", + " A sortedRepeatableAnnotation();", + "", + " @Bar", + " @Foo(anns = {@Bar(\"b\"), @Bar(\"a\")})", + " @Foo(ints = {1, 2})", + " A unsortedRepeatableAnnotation();", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodMatcherFactoryTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodMatcherFactoryTest.java index 2c16a5db21..1713861c5d 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodMatcherFactoryTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodMatcherFactoryTest.java @@ -68,13 +68,21 @@ void matcher() { "", "public class A {", " public void m1() {}", + "", " public void m1(String s) {}", + "", " public void m1(int i, int j) {}", + "", " public void m2() {}", + "", " public void m2(String s) {}", + "", " public void m2(int i, int j) {}", + "", " public void m3() {}", + "", " public void m3(String s) {}", + "", " public void m3(int i, int j) {}", "}") .addSourceLines( @@ -83,13 +91,21 @@ void matcher() { "", "public class B {", " public void m1() {}", + "", " public void m1(String s) {}", + "", " public void m1(int i, int j) {}", + "", " public void m2() {}", + "", " public void m2(String s) {}", + "", " public void m2(int i, int j) {}", + "", " public void m3() {}", + "", " public void m3(String s) {}", + "", " public void m3(int i, int j) {}", "}") .addSourceLines( @@ -98,13 +114,21 @@ void matcher() { "", "public class A {", " public static void m1() {}", + "", " public static void m1(String s) {}", + "", " public static void m1(int i, int j) {}", + "", " public static void m2() {}", + "", " public static void m2(String s) {}", + "", " public static void m2(int i, int j) {}", + "", " public static void m3() {}", + "", " public static void m3(String s) {}", + "", " public static void m3(int i, int j) {}", "}") .addSourceLines( @@ -113,13 +137,21 @@ void matcher() { "", "public class B {", " public static void m1() {}", + "", " public static void m1(String s) {}", + "", " public static void m1(int i, int j) {}", + "", " public static void m2() {}", + "", " public static void m2(String s) {}", + "", " public static void m2(int i, int j) {}", + "", " public static void m3() {}", + "", " public static void m3(String s) {}", + "", " public static void m3(int i, int j) {}", "}") .addSourceLines( diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsageCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsageCheckTest.java index ede6dbe706..762cfda83b 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsageCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsageCheckTest.java @@ -17,37 +17,62 @@ void identification() { .addSourceLines( "A.java", "import com.google.common.collect.Streams;", - "import java.util.Map;", "import java.util.HashMap;", - "import java.util.stream.Stream;", + "import java.util.Map;", "import java.util.function.IntConsumer;", "import java.util.function.IntFunction;", + "import java.util.stream.Stream;", "", "class A {", " private final Stream s = Stream.of(1);", " private final Map m = new HashMap<>();", - " private final Runnable thrower = () -> { throw new RuntimeException(); };", + " private final Runnable thrower =", + " () -> {", + " throw new RuntimeException();", + " };", "", " void unaryExternalStaticFunctionCalls() {", " s.forEach(String::valueOf);", " // BUG: Diagnostic contains:", " s.forEach(v -> String.valueOf(v));", - " // BUG: Diagnostic contains:", - " s.forEach((v) -> { String.valueOf(v); });", - " // BUG: Diagnostic contains:", - " s.forEach((Integer v) -> { { String.valueOf(v); } });", - " s.forEach(v -> { String.valueOf(v); String.valueOf(v); });", + " s.forEach(", + " // BUG: Diagnostic contains:", + " (v) -> {", + " String.valueOf(v);", + " });", + " s.forEach(", + " // BUG: Diagnostic contains:", + " (Integer v) -> {", + " {", + " String.valueOf(v);", + " }", + " });", + " s.forEach(", + " v -> {", + " String.valueOf(v);", + " String.valueOf(v);", + " });", "", " s.map(String::valueOf);", " // BUG: Diagnostic contains:", " s.map(v -> String.valueOf(v));", " // BUG: Diagnostic contains:", " s.map((v) -> (String.valueOf(v)));", - " // BUG: Diagnostic contains:", - " s.map((Integer v) -> { return String.valueOf(v); });", - " // BUG: Diagnostic contains:", - " s.map((final Integer v) -> { return (String.valueOf(v)); });", - " s.map(v -> { String.valueOf(v); return String.valueOf(v); });", + " s.map(", + " // BUG: Diagnostic contains:", + " (Integer v) -> {", + " return String.valueOf(v);", + " });", + " s.map(", + " // BUG: Diagnostic contains:", + " (final Integer v) -> {", + " return (String.valueOf(v));", + " });", + " s.map(", + " v -> {", + " String.valueOf(v);", + " return String.valueOf(v);", + " });", "", " s.findFirst().orElseGet(() -> Integer.valueOf(\"0\"));", " m.forEach((k, v) -> String.valueOf(v));", @@ -59,14 +84,34 @@ void identification() { " // BUG: Diagnostic contains:", " m.forEach((k, v) -> m.put(k, v));", " m.forEach((k, v) -> m.put(v, k));", - " // BUG: Diagnostic contains:", - " m.forEach((Integer k, Integer v) -> { m.put(k, v); });", - " m.forEach((k, v) -> { m.put(k, k); });", - " // BUG: Diagnostic contains:", - " m.forEach((final Integer k, final Integer v) -> { { m.put(k, v); } });", - " m.forEach((k, v) -> { { m.put(v, v); } });", + " m.forEach(", + " // BUG: Diagnostic contains:", + " (Integer k, Integer v) -> {", + " m.put(k, v);", + " });", + " m.forEach(", + " (k, v) -> {", + " m.put(k, k);", + " });", + " m.forEach(", + " // BUG: Diagnostic contains:", + " (final Integer k, final Integer v) -> {", + " {", + " m.put(k, v);", + " }", + " });", + " m.forEach(", + " (k, v) -> {", + " {", + " m.put(v, v);", + " }", + " });", " m.forEach((k, v) -> new HashMap().put(k, v));", - " m.forEach((k, v) -> { m.put(k, v); m.put(k, v); });", + " m.forEach(", + " (k, v) -> {", + " m.put(k, v);", + " m.put(k, v);", + " });", "", " Streams.zip(s, s, m::put);", " // BUG: Diagnostic contains:", @@ -75,20 +120,45 @@ void identification() { " // BUG: Diagnostic contains:", " Streams.zip(s, s, (Integer a, Integer b) -> (m.put(a, b)));", " Streams.zip(s, s, (a, b) -> (m.put(a, a)));", - " // BUG: Diagnostic contains:", - " Streams.zip(s, s, (final Integer a, final Integer b) -> { return m.put(a, b); });", - " Streams.zip(s, s, (a, b) -> { return m.put(b, b); });", - " // BUG: Diagnostic contains:", - " Streams.zip(s, s, (a, b) -> { return (m.put(a, b)); });", - " Streams.zip(s, s, (a, b) -> { return (m.put(b, a)); });", - " Streams.zip(s, s, (a, b) -> { m.put(a, b); return m.put(a, b); });", + " Streams.zip(", + " s,", + " s,", + " // BUG: Diagnostic contains:", + " (final Integer a, final Integer b) -> {", + " return m.put(a, b);", + " });", + " Streams.zip(", + " s,", + " s,", + " (a, b) -> {", + " return m.put(b, b);", + " });", + " Streams.zip(", + " s,", + " s,", + " // BUG: Diagnostic contains:", + " (a, b) -> {", + " return (m.put(a, b));", + " });", + " Streams.zip(", + " s,", + " s,", + " (a, b) -> {", + " return (m.put(b, a));", + " });", + " Streams.zip(", + " s,", + " s,", + " (a, b) -> {", + " m.put(a, b);", + " return m.put(a, b);", + " });", " }", "", " void nullaryExternalInstanceFunctionCalls() {", " s.map(Integer::doubleValue);", " // BUG: Diagnostic contains:", " s.map(i -> i.doubleValue());", - // `s.map(Integer::toString)` is ambiguous " s.map(i -> i.toString());", " s.map(i -> s.toString());", "", @@ -108,24 +178,36 @@ void identification() { " s.forEach(this::ivoid1);", " // BUG: Diagnostic contains:", " s.forEach(v -> ivoid1(v));", - " // BUG: Diagnostic contains:", - " s.forEach(v -> { ivoid1(v); });", + " s.forEach(", + " // BUG: Diagnostic contains:", + " v -> {", + " ivoid1(v);", + " });", " s.forEach(this::iint1);", " // BUG: Diagnostic contains:", " s.forEach(v -> iint1(v));", - " // BUG: Diagnostic contains:", - " s.forEach(v -> { iint1(v); });", + " s.forEach(", + " // BUG: Diagnostic contains:", + " v -> {", + " iint1(v);", + " });", "", " s.forEach(A::svoid1);", " // BUG: Diagnostic contains:", " s.forEach(v -> svoid1(v));", - " // BUG: Diagnostic contains:", - " s.forEach(v -> { svoid1(v); });", + " s.forEach(", + " // BUG: Diagnostic contains:", + " v -> {", + " svoid1(v);", + " });", " s.forEach(A::sint1);", " // BUG: Diagnostic contains:", " s.forEach(v -> sint1(v));", - " // BUG: Diagnostic contains:", - " s.forEach(v -> { sint1(v); });", + " s.forEach(", + " // BUG: Diagnostic contains:", + " v -> {", + " sint1(v);", + " });", "", " s.forEach(v -> ivoid2(v, v));", " s.forEach(v -> iint2(v, v));", @@ -140,28 +222,43 @@ void identification() { " m.forEach(this::ivoid2);", " // BUG: Diagnostic contains:", " m.forEach((k, v) -> ivoid2(k, v));", - " // BUG: Diagnostic contains:", - " m.forEach((k, v) -> { ivoid2(k, v); });", + " m.forEach(", + " // BUG: Diagnostic contains:", + " (k, v) -> {", + " ivoid2(k, v);", + " });", " m.forEach(this::iint2);", " // BUG: Diagnostic contains:", " m.forEach((k, v) -> iint2(k, v));", - " // BUG: Diagnostic contains:", - " m.forEach((k, v) -> { iint2(k, v); });", + " m.forEach(", + " // BUG: Diagnostic contains:", + " (k, v) -> {", + " iint2(k, v);", + " });", "", " m.forEach(A::svoid2);", " // BUG: Diagnostic contains:", " m.forEach((k, v) -> svoid2(k, v));", - " // BUG: Diagnostic contains:", - " m.forEach((k, v) -> { svoid2(k, v); });", + " m.forEach(", + " // BUG: Diagnostic contains:", + " (k, v) -> {", + " svoid2(k, v);", + " });", " m.forEach(A::sint2);", " // BUG: Diagnostic contains:", " m.forEach((k, v) -> sint2(k, v));", - " // BUG: Diagnostic contains:", - " m.forEach((k, v) -> { sint2(k, v); });", + " m.forEach(", + " // BUG: Diagnostic contains:", + " (k, v) -> {", + " sint2(k, v);", + " });", " }", "", " void functionCallsWhoseReplacementWouldBeAmbiguous() {", - " receiver(i -> { Integer.toString(i); });", + " receiver(", + " i -> {", + " Integer.toString(i);", + " });", " }", "", " void assortedOtherEdgeCases() {", @@ -175,23 +272,47 @@ void identification() { " TernaryOp o7 = (a, b, c) -> b.concat(c);", " }", "", - " void receiver(IntFunction op) { }", - " void receiver(IntConsumer op) { }", + " void receiver(IntFunction op) {}", + "", + " void receiver(IntConsumer op) {}", + "", + " void ivoid0() {}", + "", + " void ivoid1(int a) {}", + "", + " void ivoid2(int a, int b) {}", + "", + " int iint0() {", + " return 0;", + " }", + "", + " int iint1(int a) {", + " return 0;", + " }", + "", + " int iint2(int a, int b) {", + " return 0;", + " }", + "", + " static void svoid0() {}", + "", + " static void svoid1(int a) {}", + "", + " static void svoid2(int a, int b) {}", + "", + " static void svoid3(int a, int b, int c) {}", + "", + " static int sint0() {", + " return 0;", + " }", "", - " void ivoid0() { }", - " void ivoid1(int a) { }", - " void ivoid2(int a, int b) { }", - " int iint0() { return 0; }", - " int iint1(int a) { return 0; }", - " int iint2(int a, int b) { return 0; }", + " static int sint1(int a) {", + " return 0;", + " }", "", - " static void svoid0() { }", - " static void svoid1(int a) { }", - " static void svoid2(int a, int b) { }", - " static void svoid3(int a, int b, int c) { }", - " static int sint0() { return 0; }", - " static int sint1(int a) { return 0; }", - " static int sint2(int a, int b) { return 0; }", + " static int sint2(int a, int b) {", + " return 0;", + " }", "", " interface TernaryOp {", " String collect(String a, String b, String c);", @@ -210,9 +331,6 @@ void replacement() { "import java.util.Collections;", "import java.util.List;", "import java.util.Map;", - // Don't import `java.util.Set`; it should be added. - "import java.util.function.IntConsumer;", - "import java.util.function.IntFunction;", "import java.util.function.IntSupplier;", "import java.util.function.Supplier;", "import java.util.stream.Stream;", @@ -245,12 +363,19 @@ void replacement() { " Stream.of((Map) null).map(Map::keySet).map(s -> s.size());", " }", "", - " @Override int iint0() { return 0; }", + " @Override", + " int iint0() {", + " return 0;", + " }", " }", "", - " int iint0() { return 0; }", + " int iint0() {", + " return 0;", + " }", "", - " static int sint0() { return 0; }", + " static int sint0() {", + " return 0;", + " }", "}") .addOutputLines( "out/A.java", @@ -260,8 +385,6 @@ void replacement() { "import java.util.List;", "import java.util.Map;", "import java.util.Set;", - "import java.util.function.IntConsumer;", - "import java.util.function.IntFunction;", "import java.util.function.IntSupplier;", "import java.util.function.Supplier;", "import java.util.stream.Stream;", @@ -294,12 +417,19 @@ void replacement() { " Stream.of((Map) null).map(Map::keySet).map(Set::size);", " }", "", - " @Override int iint0() { return 0; }", + " @Override", + " int iint0() {", + " return 0;", + " }", " }", "", - " int iint0() { return 0; }", + " int iint0() {", + " return 0;", + " }", "", - " static int sint0() { return 0; }", + " static int sint0() {", + " return 0;", + " }", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoStubbingCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoStubbingCheckTest.java index 0525d30c7d..fd331b4ade 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoStubbingCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoStubbingCheckTest.java @@ -42,8 +42,10 @@ void identification() { " doAnswer(inv -> null).when(biConsumer).accept(0, \"foo\");", " doAnswer(inv -> null).when(biConsumer).accept(eq(0), notNull());", " doAnswer(inv -> null).when(biConsumer).accept(notNull(), eq(\"foo\"));", - " // BUG: Diagnostic contains:", - " doAnswer(inv -> null).when(biConsumer).accept(ArgumentMatchers.eq(0), ArgumentMatchers.eq(\"foo\"));", + " doAnswer(inv -> null)", + " .when(biConsumer)", + " // BUG: Diagnostic contains:", + " .accept(ArgumentMatchers.eq(0), ArgumentMatchers.eq(\"foo\"));", " // BUG: Diagnostic contains:", " doAnswer(inv -> null).when(biConsumer).accept(eq(hashCode()), eq(toString()));", " }", @@ -71,7 +73,9 @@ void replacement() { " doAnswer(inv -> null).when(consumer).accept(eq(toString()));", "", " BiConsumer biConsumer = mock(BiConsumer.class);", - " doAnswer(inv -> null).when(biConsumer).accept(ArgumentMatchers.eq(0), ArgumentMatchers.eq(\"foo\"));", + " doAnswer(inv -> null)", + " .when(biConsumer)", + " .accept(ArgumentMatchers.eq(0), ArgumentMatchers.eq(\"foo\"));", " doAnswer(inv -> null).when(biConsumer).accept(eq(hashCode()), eq(toString()));", " }", "}") diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparisonCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparisonCheckTest.java index d49772a741..a26d5490d1 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparisonCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/PrimitiveComparisonCheckTest.java @@ -99,10 +99,21 @@ void byteComparison() { " cmp().thenComparingDouble(o -> Byte.valueOf((byte) 0));", " }", "", - " private Comparator cmp() { return null; }", - " private byte toPrimitive(Object o) { return 0; }", - " private Byte toBoxed(Object o) { return 0; }", - " private Function toBoxed() { return o -> 0; }", + " private Comparator cmp() {", + " return null;", + " }", + "", + " private byte toPrimitive(Object o) {", + " return 0;", + " }", + "", + " private Byte toBoxed(Object o) {", + " return 0;", + " }", + "", + " private Function toBoxed() {", + " return o -> 0;", + " }", "}") .doTest(); } @@ -191,11 +202,25 @@ void intComparison() { " cmp().thenComparingDouble(o -> Integer.valueOf(0));", " }", "", - " private Comparator cmp() { return null; }", - " private int toPrimitive(Object o) { return 0; }", - " private Integer toBoxed(Object o) { return 0; }", - " private Function toBoxed() { return o -> 0; }", - " private ToIntFunction toPrimitive() { return o -> 0; }", + " private Comparator cmp() {", + " return null;", + " }", + "", + " private int toPrimitive(Object o) {", + " return 0;", + " }", + "", + " private Integer toBoxed(Object o) {", + " return 0;", + " }", + "", + " private Function toBoxed() {", + " return o -> 0;", + " }", + "", + " private ToIntFunction toPrimitive() {", + " return o -> 0;", + " }", "}") .doTest(); } @@ -268,11 +293,25 @@ void longComparison() { " cmp().thenComparingDouble(o -> Long.valueOf(0));", " }", "", - " private Comparator cmp() { return null; }", - " private long toPrimitive(Object o) { return 0L; }", - " private Long toBoxed(Object o) { return 0L; }", - " private Function toBoxed() { return o -> 0L; }", - " private ToLongFunction toPrimitive() { return o -> 0L; }", + " private Comparator cmp() {", + " return null;", + " }", + "", + " private long toPrimitive(Object o) {", + " return 0L;", + " }", + "", + " private Long toBoxed(Object o) {", + " return 0L;", + " }", + "", + " private Function toBoxed() {", + " return o -> 0L;", + " }", + "", + " private ToLongFunction toPrimitive() {", + " return o -> 0L;", + " }", "}") .doTest(); } @@ -326,10 +365,21 @@ void floatComparison() { " cmp().thenComparingDouble(o -> Float.valueOf(0));", " }", "", - " private Comparator cmp() { return null; }", - " private float toPrimitive(Object o) { return 0.0f; }", - " private Float toBoxed(Object o) { return 0.0f; }", - " private Function toBoxed() { return o -> 0.0f; }", + " private Comparator cmp() {", + " return null;", + " }", + "", + " private float toPrimitive(Object o) {", + " return 0.0f;", + " }", + "", + " private Float toBoxed(Object o) {", + " return 0.0f;", + " }", + "", + " private Function toBoxed() {", + " return o -> 0.0f;", + " }", "}") .doTest(); } @@ -386,11 +436,25 @@ void doubleComparison() { " cmp().thenComparingDouble(o -> Double.valueOf(0));", " }", "", - " private Comparator cmp() { return null; }", - " private double toPrimitive(Object o) { return 0.0; }", - " private Double toBoxed(Object o) { return 0.0; }", - " private Function toBoxed() { return o -> 0.0; }", - " private ToDoubleFunction toPrimitive() { return o -> 0.0; }", + " private Comparator cmp() {", + " return null;", + " }", + "", + " private double toPrimitive(Object o) {", + " return 0.0;", + " }", + "", + " private Double toBoxed(Object o) {", + " return 0.0;", + " }", + "", + " private Function toBoxed() {", + " return o -> 0.0;", + " }", + "", + " private ToDoubleFunction toPrimitive() {", + " return o -> 0.0;", + " }", "}") .doTest(); } @@ -420,8 +484,13 @@ void stringComparison() { " cmp().thenComparing(toStr(), cmp());", " }", "", - " private Comparator cmp() { return null; }", - " private Function toStr() { return String::valueOf; }", + " private Comparator cmp() {", + " return null;", + " }", + "", + " private Function toStr() {", + " return String::valueOf;", + " }", "}") .doTest(); } @@ -692,22 +761,30 @@ void replacementWithBoxedVariantsInComplexSyntacticalContext() { "import java.util.Comparator;", "", "interface A extends Comparable {", - " Comparator bCmp = Comparator.comparing(o -> o).thenComparingInt(o -> Byte.valueOf((byte) 0));", - " Comparator cCmp = Comparator.comparing(o -> o).thenComparingInt(o -> Character.valueOf((char) 0));", - " Comparator sCmp = Comparator.comparing(o -> o).thenComparingInt(o -> Short.valueOf((short) 0));", + " Comparator bCmp =", + " Comparator.comparing(o -> o).thenComparingInt(o -> Byte.valueOf((byte) 0));", + " Comparator cCmp =", + " Comparator.comparing(o -> o).thenComparingInt(o -> Character.valueOf((char) 0));", + " Comparator sCmp =", + " Comparator.comparing(o -> o).thenComparingInt(o -> Short.valueOf((short) 0));", " Comparator iCmp = Comparator.comparing(o -> o).thenComparingInt(o -> Integer.valueOf(0));", " Comparator lCmp = Comparator.comparing(o -> o).thenComparingLong(o -> Long.valueOf(0));", - " Comparator fCmp = Comparator.comparing(o -> o).thenComparingDouble(o -> Float.valueOf(0));", - " Comparator dCmp = Comparator.comparing(o -> o).thenComparingDouble(o -> Double.valueOf(0));", + " Comparator fCmp =", + " Comparator.comparing(o -> o).thenComparingDouble(o -> Float.valueOf(0));", + " Comparator dCmp =", + " Comparator.comparing(o -> o).thenComparingDouble(o -> Double.valueOf(0));", "}") .addOutputLines( "out/A.java", "import java.util.Comparator;", "", "interface A extends Comparable {", - " Comparator bCmp = Comparator.comparing(o -> o).thenComparing(o -> Byte.valueOf((byte) 0));", - " Comparator cCmp = Comparator.comparing(o -> o).thenComparing(o -> Character.valueOf((char) 0));", - " Comparator sCmp = Comparator.comparing(o -> o).thenComparing(o -> Short.valueOf((short) 0));", + " Comparator bCmp =", + " Comparator.comparing(o -> o).thenComparing(o -> Byte.valueOf((byte) 0));", + " Comparator cCmp =", + " Comparator.comparing(o -> o).thenComparing(o -> Character.valueOf((char) 0));", + " Comparator sCmp =", + " Comparator.comparing(o -> o).thenComparing(o -> Short.valueOf((short) 0));", " Comparator iCmp = Comparator.comparing(o -> o).thenComparing(o -> Integer.valueOf(0));", " Comparator lCmp = Comparator.comparing(o -> o).thenComparing(o -> Long.valueOf(0));", " Comparator fCmp = Comparator.comparing(o -> o).thenComparing(o -> Float.valueOf(0));", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionCheckTest.java index 3f4bf00db3..7f00f87e75 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RedundantStringConversionCheckTest.java @@ -124,7 +124,7 @@ void identificationWithinBinaryOperation() { " s + String.valueOf((String) null),", " s + String.valueOf(null),", " s + String.valueOf(new char[0]),", - "", + " //", " 42 + this.toString(),", " 42 + super.toString(),", " 42 + i.toString(),", @@ -147,7 +147,7 @@ void identificationWithinBinaryOperation() { " String.valueOf((String) null) + s,", " String.valueOf(null) + s,", " String.valueOf(new char[0]) + s,", - "", + " //", " this.toString() + 42,", " super.toString() + 42,", " i.toString() + 42,", @@ -175,10 +175,7 @@ void identificationWithinBinaryOperation() { "", " int[] m2() {", " return new int[] {", - " 1 + 1,", - " 1 - 1,", - " 1 * 1,", - " 1 / 1,", + " 1 + 1, 1 - 1, 1 * 1, 1 / 1,", " };", " }", "}") @@ -284,9 +281,9 @@ void identificationWithinGuavaGuardMethod() { compilationTestHelper .addSourceLines( "A.java", - "import static com.google.common.base.Preconditions.checkState;", "import static com.google.common.base.Preconditions.checkArgument;", "import static com.google.common.base.Preconditions.checkNotNull;", + "import static com.google.common.base.Preconditions.checkState;", "import static com.google.common.base.Verify.verify;", "import static com.google.common.base.Verify.verifyNotNull;", "", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationCheckTest.java index 60bebd5b72..bcb8ffb35f 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestMappingAnnotationCheckTest.java @@ -28,7 +28,6 @@ void identification() { "import org.springframework.web.bind.annotation.RequestBody;", "import org.springframework.web.bind.annotation.RequestHeader;", "import org.springframework.web.bind.annotation.RequestMapping;", - "import org.springframework.web.bind.annotation.RequestMethod;", "import org.springframework.web.bind.annotation.RequestParam;", "import org.springframework.web.context.request.NativeWebRequest;", "import org.springframework.web.context.request.WebRequest;", @@ -38,48 +37,91 @@ void identification() { "", "interface A {", " A noMapping();", + "", " A noMapping(String param);", - " @DeleteMapping A properNoParameters();", - " @GetMapping A properPathVariable(@PathVariable String param);", - " @PatchMapping A properRequestBody(@RequestBody String body);", - " @PostMapping A properRequestHeader(@RequestHeader String header);", - " @PutMapping A properRequestParam(@RequestParam String param);", - " @RequestMapping A properInputStream(InputStream input);", - " @RequestMapping A properZoneId(ZoneId zoneId);", - " @RequestMapping A properLocale(Locale locale);", - " @RequestMapping A properTimeZone(TimeZone timeZone);", - " @RequestMapping A properHttpServletRequest(HttpServletRequest request);", - " @RequestMapping A properHttpServletResponse(HttpServletResponse response);", - " @RequestMapping A properHttpMethod(HttpMethod method);", - " @RequestMapping A properNativeWebRequest(NativeWebRequest request);", - " @RequestMapping A properWebRequest(WebRequest request);", - " @RequestMapping A properServerWebExchange(ServerWebExchange exchange);", - " @RequestMapping A properServerUriBuilder(UriBuilder builder);", - " @RequestMapping A properServerUriComponentsBuilder(UriComponentsBuilder builder);", "", + " @DeleteMapping", + " A properNoParameters();", + "", + " @GetMapping", + " A properPathVariable(@PathVariable String param);", + "", + " @PatchMapping", + " A properRequestBody(@RequestBody String body);", + "", + " @PostMapping", + " A properRequestHeader(@RequestHeader String header);", + "", + " @PutMapping", + " A properRequestParam(@RequestParam String param);", + "", + " @RequestMapping", + " A properInputStream(InputStream input);", + "", + " @RequestMapping", + " A properZoneId(ZoneId zoneId);", + "", + " @RequestMapping", + " A properLocale(Locale locale);", + "", + " @RequestMapping", + " A properTimeZone(TimeZone timeZone);", + "", + " @RequestMapping", + " A properHttpServletRequest(HttpServletRequest request);", + "", + " @RequestMapping", + " A properHttpServletResponse(HttpServletResponse response);", + "", + " @RequestMapping", + " A properHttpMethod(HttpMethod method);", + "", + " @RequestMapping", + " A properNativeWebRequest(NativeWebRequest request);", + "", + " @RequestMapping", + " A properWebRequest(WebRequest request);", + "", + " @RequestMapping", + " A properServerWebExchange(ServerWebExchange exchange);", + "", + " @RequestMapping", + " A properServerUriBuilder(UriBuilder builder);", + "", + " @RequestMapping", + " A properServerUriComponentsBuilder(UriComponentsBuilder builder);", + "", + " @DeleteMapping", " // BUG: Diagnostic contains:", - " @DeleteMapping A delete(String param);", + " A delete(String param);", "", + " @GetMapping", " // BUG: Diagnostic contains:", - " @GetMapping A get(String param);", + " A get(String param);", "", + " @PatchMapping", " // BUG: Diagnostic contains:", - " @PatchMapping A patch(String param);", + " A patch(String param);", "", + " @PostMapping", " // BUG: Diagnostic contains:", - " @PostMapping A post(String param);", + " A post(String param);", "", + " @PutMapping", " // BUG: Diagnostic contains:", - " @PutMapping A put(String param);", + " A put(String param);", "", + " @RequestMapping", " // BUG: Diagnostic contains:", - " @RequestMapping A requestMultiple(String param, String param2);", + " A requestMultiple(String param, String param2);", "", + " @RequestMapping", " // BUG: Diagnostic contains:", - " @RequestMapping A requestFirstParamViolation(String param, @PathVariable String param2);", + " A requestFirstParamViolation(String param, @PathVariable String param2);", "", + " @RequestMapping", " // BUG: Diagnostic contains:", - " @RequestMapping A requestSecondParamViolation(@RequestBody String param, String param2);", + " A requestSecondParamViolation(@RequestBody String param, String param2);", "}") .doTest(); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeCheckTest.java index 0751c5b5e8..37275252ed 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RequestParamTypeCheckTest.java @@ -28,23 +28,36 @@ void identification() { "import org.springframework.web.bind.annotation.RequestParam;", "", "interface A {", - " @PostMapping A properRequestParam(@RequestBody String body);", - " @GetMapping A properRequestParam(@RequestParam int param);", - " @GetMapping A properRequestParam(@RequestParam List param);", - " @PostMapping A properRequestParam(@RequestBody String body, @RequestParam Set param);", - " @PutMapping A properRequestParam(@RequestBody String body, @RequestParam Map param);", + " @PostMapping", + " A properRequestParam(@RequestBody String body);", "", + " @GetMapping", + " A properRequestParam(@RequestParam int param);", + "", + " @GetMapping", + " A properRequestParam(@RequestParam List param);", + "", + " @PostMapping", + " A properRequestParam(@RequestBody String body, @RequestParam Set param);", + "", + " @PutMapping", + " A properRequestParam(@RequestBody String body, @RequestParam Map param);", + "", + " @GetMapping", " // BUG: Diagnostic contains:", - " @GetMapping A get(@RequestParam ImmutableBiMap param);", + " A get(@RequestParam ImmutableBiMap param);", "", + " @PostMapping", " // BUG: Diagnostic contains:", - " @PostMapping A post(@Nullable @RequestParam ImmutableList param);", + " A post(@Nullable @RequestParam ImmutableList param);", "", + " @PutMapping", " // BUG: Diagnostic contains:", - " @PutMapping A put(@RequestBody String body, @RequestParam ImmutableSet param);", + " A put(@RequestBody String body, @RequestParam ImmutableSet param);", "", + " @DeleteMapping", " // BUG: Diagnostic contains:", - " @DeleteMapping A delete(@RequestBody String body, @RequestParam ImmutableMap param);", + " A delete(@RequestBody String body, @RequestParam ImmutableMap param);", "", " void negative(ImmutableSet set, ImmutableMap map);", "}") diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatementCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatementCheckTest.java index 3a56d70ee9..a7c9c5193d 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatementCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/Slf4jLogStatementCheckTest.java @@ -44,7 +44,8 @@ void identification() { " LOG.info(marker, s, o, t);", "", " LOG.warn(FMT0);", - " // BUG: Diagnostic contains: Log statement contains 0 placeholders, but specifies 1 matching argument(s)", + " // BUG: Diagnostic contains: Log statement contains 0 placeholders, but specifies 1 matching", + " // argument(s)", " LOG.error(FMT0, o);", " LOG.trace(FMT0, t);", " // BUG: Diagnostic contains:", @@ -56,13 +57,15 @@ void identification() { " // BUG: Diagnostic contains:", " LOG.trace(marker, FMT0, o, t);", "", - " // BUG: Diagnostic contains: Log statement contains 1 placeholders, but specifies 0 matching argument(s)", + " // BUG: Diagnostic contains: Log statement contains 1 placeholders, but specifies 0 matching", + " // argument(s)", " LOG.debug(FMT1);", " LOG.info(FMT1, o);", " // BUG: Diagnostic contains:", " LOG.warn(FMT1, t);", " LOG.error(FMT1, o, t);", - " // BUG: Diagnostic contains: Log statement contains 1 placeholders, but specifies 2 matching argument(s)", + " // BUG: Diagnostic contains: Log statement contains 1 placeholders, but specifies 2 matching", + " // argument(s)", " LOG.trace(FMT1, o, o);", " // BUG: Diagnostic contains:", " LOG.debug(FMT1, o, o, t);", diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationCheckTest.java index 842e08e556..bb2e3dc07a 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/SpringMvcAnnotationCheckTest.java @@ -19,9 +19,9 @@ void identification() { "import static org.springframework.web.bind.annotation.RequestMethod.DELETE;", "import static org.springframework.web.bind.annotation.RequestMethod.GET;", "import static org.springframework.web.bind.annotation.RequestMethod.HEAD;", + "import static org.springframework.web.bind.annotation.RequestMethod.PATCH;", "import static org.springframework.web.bind.annotation.RequestMethod.POST;", "import static org.springframework.web.bind.annotation.RequestMethod.PUT;", - "import static org.springframework.web.bind.annotation.RequestMethod.PATCH;", "", "import org.springframework.web.bind.annotation.DeleteMapping;", "import org.springframework.web.bind.annotation.GetMapping;", @@ -32,28 +32,53 @@ void identification() { "import org.springframework.web.bind.annotation.RequestMethod;", "", "interface A {", - " @RequestMapping A simple();", - " @RequestMapping(method = {}) A explicitDefault();", + " @RequestMapping", + " A simple();", + "", + " @RequestMapping(method = {})", + " A explicitDefault();", " // BUG: Diagnostic contains:", - " @RequestMapping(method = RequestMethod.GET) A get();", + " @RequestMapping(method = RequestMethod.GET)", + " A get();", " // BUG: Diagnostic contains:", - " @RequestMapping(method = {RequestMethod.POST}) A post();", + " @RequestMapping(method = {RequestMethod.POST})", + " A post();", " // BUG: Diagnostic contains:", - " @RequestMapping(method = {PUT}) A put();", + " @RequestMapping(method = {PUT})", + " A put();", " // BUG: Diagnostic contains:", - " @RequestMapping(method = {DELETE}) A delete();", + " @RequestMapping(method = {DELETE})", + " A delete();", " // BUG: Diagnostic contains:", - " @RequestMapping(method = {PATCH}) A patch();", - " @RequestMapping(method = HEAD) A head();", - " @RequestMapping(method = RequestMethod.OPTIONS) A options();", - " @RequestMapping(method = {GET, POST}) A simpleMix();", - " @RequestMapping(method = {RequestMethod.GET, RequestMethod.POST}) A verboseMix();", - "", - " @DeleteMapping A properDelete();", - " @GetMapping A properGet();", - " @PatchMapping A properPatch();", - " @PostMapping A properPost();", - " @PutMapping A properPut();", + " @RequestMapping(method = {PATCH})", + " A patch();", + "", + " @RequestMapping(method = HEAD)", + " A head();", + "", + " @RequestMapping(method = RequestMethod.OPTIONS)", + " A options();", + "", + " @RequestMapping(method = {GET, POST})", + " A simpleMix();", + "", + " @RequestMapping(method = {RequestMethod.GET, RequestMethod.POST})", + " A verboseMix();", + "", + " @DeleteMapping", + " A properDelete();", + "", + " @GetMapping", + " A properGet();", + "", + " @PatchMapping", + " A properPatch();", + "", + " @PostMapping", + " A properPost();", + "", + " @PutMapping", + " A properPut();", "}") .doTest(); } @@ -71,11 +96,25 @@ void replacement() { "import org.springframework.web.bind.annotation.RequestMethod;", "", "interface A {", - " @RequestMapping(method = RequestMethod.GET) A simple();", - " @RequestMapping(path = \"/foo/bar\", method = POST) A prefixed();", - " @RequestMapping(method = {RequestMethod.DELETE}, path = \"/foo/bar\") A suffixed();", - " @RequestMapping(path = \"/foo/bar\", method = {PUT}, consumes = {\"a\", \"b\"}) A surrounded();", - " @RequestMapping(method = {PATCH}) A curly();", + " @RequestMapping(method = RequestMethod.GET)", + " A simple();", + "", + " @RequestMapping(path = \"/foo/bar\", method = POST)", + " A prefixed();", + "", + " @RequestMapping(", + " method = {RequestMethod.DELETE},", + " path = \"/foo/bar\")", + " A suffixed();", + "", + " @RequestMapping(", + " path = \"/foo/bar\",", + " method = {PUT},", + " consumes = {\"a\", \"b\"})", + " A surrounded();", + "", + " @RequestMapping(method = {PATCH})", + " A curly();", "}") .addOutputLines( "out/A.java", @@ -92,11 +131,22 @@ void replacement() { "import org.springframework.web.bind.annotation.RequestMethod;", "", "interface A {", - " @GetMapping() A simple();", - " @PostMapping(path = \"/foo/bar\") A prefixed();", - " @DeleteMapping(path = \"/foo/bar\") A suffixed();", - " @PutMapping(path = \"/foo/bar\", consumes = {\"a\", \"b\"}) A surrounded();", - " @PatchMapping() A curly();", + " @GetMapping()", + " A simple();", + "", + " @PostMapping(path = \"/foo/bar\")", + " A prefixed();", + "", + " @DeleteMapping(path = \"/foo/bar\")", + " A suffixed();", + "", + " @PutMapping(", + " path = \"/foo/bar\",", + " consumes = {\"a\", \"b\"})", + " A surrounded();", + "", + " @PatchMapping()", + " A curly();", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportCheckTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportCheckTest.java index 398242a2a1..328e1473b1 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportCheckTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/StaticImportCheckTest.java @@ -51,9 +51,8 @@ void identification() { "import java.nio.charset.StandardCharsets;", "import java.time.ZoneOffset;", "import java.util.Optional;", - "import java.util.function.Predicate;", "import java.util.UUID;", - "import org.springframework.boot.test.context.SpringBootTest;", + "import java.util.function.Predicate;", "import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;", "import org.springframework.http.MediaType;", "", @@ -135,10 +134,10 @@ void replacement() { "import java.util.Objects;", "import java.util.regex.Pattern;", "import org.junit.jupiter.params.provider.Arguments;", - "import org.springframework.format.annotation.DateTimeFormat;", - "import org.springframework.format.annotation.DateTimeFormat.ISO;", "import org.springframework.boot.test.context.SpringBootTest;", "import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;", + "import org.springframework.format.annotation.DateTimeFormat;", + "import org.springframework.format.annotation.DateTimeFormat.ISO;", "import org.springframework.http.MediaType;", "", "class A {", @@ -181,15 +180,15 @@ void replacement() { " @DateTimeFormat(iso = ISO.TIME) String time) {}", "", " @BugPattern(", - " name = \"TestBugPattern\",", - " summary = \"\",", - " linkType = BugPattern.LinkType.NONE,", - " severity = SeverityLevel.SUGGESTION,", - " tags = BugPattern.StandardTags.SIMPLIFICATION)", + " name = \"TestBugPattern\",", + " summary = \"\",", + " linkType = BugPattern.LinkType.NONE,", + " severity = SeverityLevel.SUGGESTION,", + " tags = BugPattern.StandardTags.SIMPLIFICATION)", " static final class TestBugPattern {}", "", - " @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)", - " final class Test {}", + " @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)", + " final class Test {}", "}") .addOutputLines( "out/A.java", @@ -250,10 +249,7 @@ void replacement() { " Object o = UTF_8;", "", " ImmutableSet.of(", - " MediaType.ALL,", - " APPLICATION_XHTML_XML,", - " TEXT_HTML,", - " MediaType.valueOf(\"image/webp\"));", + " MediaType.ALL, APPLICATION_XHTML_XML, TEXT_HTML, MediaType.valueOf(\"image/webp\"));", "", " Pattern.compile(\"\", CASE_INSENSITIVE);", " }", @@ -269,15 +265,15 @@ void replacement() { " @DateTimeFormat(iso = TIME) String time) {}", "", " @BugPattern(", - " name = \"TestBugPattern\",", - " summary = \"\",", - " linkType = NONE,", - " severity = SUGGESTION,", - " tags = SIMPLIFICATION)", + " name = \"TestBugPattern\",", + " summary = \"\",", + " linkType = NONE,", + " severity = SUGGESTION,", + " tags = SIMPLIFICATION)", " static final class TestBugPattern {}", "", - " @SpringBootTest(webEnvironment = RANDOM_PORT)", - " final class Test {}", + " @SpringBootTest(webEnvironment = RANDOM_PORT)", + " final class Test {}", "}") .doTest(TestMode.TEXT_MATCH); } diff --git a/pom.xml b/pom.xml index 1de56e3887..afb76898d3 100644 --- a/pom.xml +++ b/pom.xml @@ -237,6 +237,11 @@ javac 9+181-r4173-1 + + com.google.googlejavaformat + google-java-format + 1.15.0 + diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/util/IsArrayTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/util/IsArrayTest.java index a54af5b346..33d76c771a 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/util/IsArrayTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/util/IsArrayTest.java @@ -17,7 +17,6 @@ void matches() { CompilationTestHelper.newInstance(TestChecker.class, getClass()) .addSourceLines( "A.java", - "package test.foo;", "class A {", " Object negative1() {", " return alwaysNull();",