From 196edf9118e7a141e9667cff456ebf7afa114114 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sun, 10 May 2020 21:01:41 +0200 Subject: [PATCH] WIP: Introduce Checker Framework To be determined: move to a separate profile? --- .../BugPatternTestExtractor.java | 5 +- .../documentation/DocumentationGenerator.java | 3 +- .../DocumentationGeneratorTaskListener.java | 7 +- .../errorprone/documentation/Compilation.java | 35 +++--- error-prone-contrib/pom.xml | 4 + .../FormatStringConcatenation.java | 5 +- .../bugpatterns/JUnitValueSource.java | 3 + .../bugpatterns/MethodReferenceUsage.java | 8 +- .../util/MethodMatcherFactory.java | 3 + .../refasterrules/AssertJStringRules.java | 9 +- .../AssertJThrowingCallableRules.java | 5 + .../refasterrules/EqualityRules.java | 8 +- .../refasterrules/PatternRules.java | 5 +- .../errorprone/refasterrules/StreamRules.java | 24 ++++- pom.xml | 102 +++++++++++++++++- .../RefasterRuleCompilerTaskListener.java | 4 +- refaster-runner/pom.xml | 4 + .../errorprone/refaster/runner/Refaster.java | 3 +- .../refaster/runner/RefasterTest.java | 1 + .../errorprone/refaster/matchers/IsEmpty.java | 3 +- ...AnnotatedCompositeCodeTransformerTest.java | 5 +- .../matchers/AbstractMatcherTestChecker.java | 2 + .../refaster/matchers/IsEmptyTest.java | 6 ++ 23 files changed, 214 insertions(+), 40 deletions(-) diff --git a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java index e24a23c8..76e7cf48 100644 --- a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java +++ b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java @@ -2,6 +2,7 @@ package tech.picnic.errorprone.documentation; import static com.google.errorprone.matchers.Matchers.instanceMethod; import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod; +import static java.util.Objects.requireNonNull; import static java.util.function.Predicate.not; import com.google.auto.service.AutoService; @@ -17,7 +18,9 @@ import com.sun.source.tree.MethodInvocationTree; import com.sun.source.util.TreeScanner; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.Optional; +import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; import tech.picnic.errorprone.documentation.BugPatternTestExtractor.TestCases; @@ -157,7 +160,7 @@ public final class BugPatternTestExtractor implements Extractor { * is safe, because this code is guarded by an earlier call to `#getClassUnderTest(..)`, * which ensures that `tree` is part of a longer method invocation chain. */ - MethodInvocationTree inputTree = (MethodInvocationTree) ASTHelpers.getReceiver(tree); + MethodInvocationTree inputTree = (MethodInvocationTree) requireNonNull(ASTHelpers.getReceiver(tree)); String path = ASTHelpers.constValue(inputTree.getArguments().get(0), String.class); Optional inputCode = getSourceCode(inputTree); diff --git a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGenerator.java b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGenerator.java index 39169faa..c12abe73 100644 --- a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGenerator.java +++ b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGenerator.java @@ -1,6 +1,7 @@ package tech.picnic.errorprone.documentation; import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; import com.google.auto.service.AutoService; import com.google.common.annotations.VisibleForTesting; @@ -46,7 +47,7 @@ public final class DocumentationGenerator implements Plugin { checkArgument( matcher.matches(), "'%s' must be of the form '%s='", pathArg, OUTPUT_DIRECTORY_FLAG); - String path = matcher.group(1); + String path = requireNonNull(matcher.group(1), "Path must be present"); try { return Path.of(path); } catch (InvalidPathException e) { diff --git a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGeneratorTaskListener.java b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGeneratorTaskListener.java index 07261650..172fe851 100644 --- a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGeneratorTaskListener.java +++ b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/DocumentationGeneratorTaskListener.java @@ -1,6 +1,7 @@ package tech.picnic.errorprone.documentation; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Objects.requireNonNull; import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility; import com.fasterxml.jackson.annotation.PropertyAccessor; @@ -93,7 +94,7 @@ final class DocumentationGeneratorTaskListener implements TaskListener { } } - private void writeToFile(String identifier, String className, T data) { + private void writeToFile(String identifier, String className, T data) { File file = docsPath.resolve(String.format("%s-%s.json", identifier, className)).toFile(); try (FileWriter fileWriter = new FileWriter(file, UTF_8)) { @@ -104,6 +105,8 @@ final class DocumentationGeneratorTaskListener implements TaskListener { } private static String getSimpleClassName(URI path) { - return Paths.get(path).getFileName().toString().replace(".java", ""); + return requireNonNull(Paths.get(path).getFileName(), "Path lacks filename") + .toString() + .replace(".java", ""); } } diff --git a/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java b/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java index 452b5d2d..b7868ae8 100644 --- a/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java +++ b/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java @@ -8,6 +8,8 @@ import com.google.errorprone.FileObjects; import com.sun.tools.javac.api.JavacTaskImpl; import com.sun.tools.javac.api.JavacTool; import com.sun.tools.javac.file.JavacFileManager; +import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; @@ -53,22 +55,25 @@ public final class Compilation { } private static void compile(ImmutableList options, JavaFileObject javaFileObject) { - JavacFileManager javacFileManager = FileManagers.testFileManager(); - JavaCompiler compiler = JavacTool.create(); + try (JavacFileManager javacFileManager = FileManagers.testFileManager()) { + JavaCompiler compiler = JavacTool.create(); - List> diagnostics = new ArrayList<>(); - JavacTaskImpl task = - (JavacTaskImpl) - compiler.getTask( - null, - javacFileManager, - diagnostics::add, - options, - ImmutableList.of(), - ImmutableList.of(javaFileObject)); + List> diagnostics = new ArrayList<>(); + JavacTaskImpl task = + (JavacTaskImpl) + compiler.getTask( + null, + javacFileManager, + diagnostics::add, + options, + ImmutableList.of(), + ImmutableList.of(javaFileObject)); - Boolean result = task.call(); - assertThat(diagnostics).isEmpty(); - assertThat(result).isTrue(); + Boolean result = task.call(); + assertThat(diagnostics).isEmpty(); + assertThat(result).isTrue(); + } catch (IOException e) { + throw new UncheckedIOException("Failed to close `JavaCompiler`", e); + } } } diff --git a/error-prone-contrib/pom.xml b/error-prone-contrib/pom.xml index f268bbea..31329aba 100644 --- a/error-prone-contrib/pom.xml +++ b/error-prone-contrib/pom.xml @@ -146,6 +146,10 @@ assertj-core provided + + org.checkerframework + checker-util + org.immutables value-annotations diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java index 9f4c8f51..33c22942 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/FormatStringConcatenation.java @@ -31,6 +31,7 @@ import com.sun.source.tree.ParenthesizedTree; import com.sun.source.tree.Tree; import com.sun.source.tree.Tree.Kind; import com.sun.source.util.SimpleTreeVisitor; +import com.sun.tools.javac.code.Type; import java.util.ArrayList; import java.util.Formatter; import java.util.List; @@ -208,7 +209,9 @@ public final class FormatStringConcatenation extends BugChecker } private static boolean isStringTyped(ExpressionTree tree, VisitorState state) { - return ASTHelpers.isSameType(ASTHelpers.getType(tree), state.getSymtab().stringType, state); + // XXX: Open Error Prone PR to improve the `@Nullable` annotations on `ASTHelpers`. + Type type = ASTHelpers.getType(tree); + return type != null && ASTHelpers.isSameType(type, state.getSymtab().stringType, state); } private static class ReplacementArgumentsConstructor diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java index e6dc139f..eb612b1c 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/JUnitValueSource.java @@ -171,6 +171,9 @@ public final class JUnitValueSource extends BugChecker implements MethodTreeMatc return findMatchingSibling(tree, m -> m.getName().contentEquals(methodName), state); } + // XXX: File ticket for the `@SuppressWarnings`: the checker incorrectly claims that the predicate + // only accepts `@InternedDistinct @SignatureBottom MethodTree` values (IIUC). + @SuppressWarnings({"interning:argument", "signature:argument"}) private static Optional findMatchingSibling( MethodTree tree, Predicate predicate, VisitorState state) { return state.findEnclosing(ClassTree.class).getMembers().stream() diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java index 5c79baf8..7b217b18 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MethodReferenceUsage.java @@ -34,6 +34,7 @@ import com.sun.tools.javac.code.Type; import java.util.List; import java.util.Optional; import javax.lang.model.element.Name; +import org.checkerframework.checker.nullness.qual.NonNull; /** * A {@link BugChecker} that flags lambda expressions that can be replaced with method references. @@ -189,7 +190,12 @@ public final class MethodReferenceUsage extends BugChecker implements LambdaExpr } } - return Optional.of(diff == 0 ? Optional.empty() : Optional.of(expectedArguments.get(0))); + // XXX: These type hints shouldn't be necessary; see + // https://github.com/typetools/checker-framework/issues/4007. + return Optional.of( + diff == 0 + ? Optional.<@NonNull Name>empty() + : Optional.<@NonNull Name>of(expectedArguments.get(0))); } private static ImmutableList getVariables(LambdaExpressionTree tree) { diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactory.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactory.java index de03abd0..02ecbead 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactory.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/util/MethodMatcherFactory.java @@ -45,6 +45,9 @@ public final class MethodMatcherFactory { // XXX: It seems parse errors are silently swallowed. Double-check; if true, file a ticket. // XXX: This (probably) doesn't work for methods with array type arguments; if true, implement a // fix. + // XXX: The `nullness` warning suppression shouldn't be necessary. See + // https://github.com/typetools/checker-framework/issues/4006. + @SuppressWarnings("nullness:argument") private static Matcher createMethodMatcher(CharSequence signature) { java.util.regex.Matcher m = METHOD_SIGNATURE.matcher(signature); checkArgument(m.matches(), "Not a valid method signature: %s", signature); diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java index 4143884e..826091ec 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJStringRules.java @@ -13,6 +13,7 @@ import java.nio.file.Files; import java.nio.file.Path; import org.assertj.core.api.AbstractAssert; import org.assertj.core.api.AbstractStringAssert; +import org.checkerframework.checker.regex.qual.Regex; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; @OnlineDocumentation @@ -71,26 +72,26 @@ final class AssertJStringRules { static final class AssertThatMatches { @BeforeTemplate - AbstractAssert before(String string, String regex) { + AbstractAssert before(String string, @Regex String regex) { return assertThat(string.matches(regex)).isTrue(); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - AbstractAssert after(String string, String regex) { + AbstractAssert after(String string, @Regex String regex) { return assertThat(string).matches(regex); } } static final class AssertThatDoesNotMatch { @BeforeTemplate - AbstractAssert before(String string, String regex) { + AbstractAssert before(String string, @Regex String regex) { return assertThat(string.matches(regex)).isFalse(); } @AfterTemplate @UseImportPolicy(STATIC_IMPORT_ALWAYS) - AbstractAssert after(String string, String regex) { + AbstractAssert after(String string, @Regex String regex) { return assertThat(string).doesNotMatch(regex); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java index 74bd22df..e43deed1 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/AssertJThrowingCallableRules.java @@ -16,6 +16,7 @@ import java.io.IOException; import org.assertj.core.api.AbstractObjectAssert; import org.assertj.core.api.AbstractThrowableAssert; import org.assertj.core.api.ThrowableAssert.ThrowingCallable; +import org.checkerframework.checker.formatter.qual.FormatMethod; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; /** @@ -591,6 +592,7 @@ final class AssertJThrowingCallableRules { // arguments to a wide range of format methods. static final class AbstractThrowableAssertHasMessage { @BeforeTemplate + @FormatMethod AbstractThrowableAssert before( AbstractThrowableAssert abstractThrowableAssert, String message, @@ -599,6 +601,7 @@ final class AssertJThrowingCallableRules { } @AfterTemplate + @FormatMethod AbstractThrowableAssert after( AbstractThrowableAssert abstractThrowableAssert, String message, @@ -611,6 +614,7 @@ final class AssertJThrowingCallableRules { // arguments to a wide range of format methods. static final class AbstractThrowableAssertWithFailMessage { @BeforeTemplate + @FormatMethod AbstractThrowableAssert before( AbstractThrowableAssert abstractThrowableAssert, String message, @@ -619,6 +623,7 @@ final class AssertJThrowingCallableRules { } @AfterTemplate + @FormatMethod AbstractThrowableAssert after( AbstractThrowableAssert abstractThrowableAssert, String message, diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java index 119f1634..fd344ccd 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/EqualityRules.java @@ -12,6 +12,7 @@ import com.google.errorprone.refaster.annotation.Placeholder; import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; +import org.checkerframework.checker.interning.qual.Interned; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; /** Refaster rules related to expressions dealing with (in)equalities. */ @@ -21,7 +22,7 @@ final class EqualityRules { /** Prefer reference-based quality for enums. */ // Primitive value comparisons are not listed, because Error Prone flags those out of the box. - static final class PrimitiveOrReferenceEquality> { + static final class PrimitiveOrReferenceEquality> { /** * Enums can be compared by reference. It is safe to do so even in the face of refactorings, * because if the type is ever converted to a non-enum, then Error-Prone will complain about any @@ -30,6 +31,7 @@ final class EqualityRules { // XXX: This Refaster rule is the topic of https://github.com/google/error-prone/issues/559. We // work around the issue by selecting the "largest replacements". See the `Refaster` check. @BeforeTemplate + @SuppressWarnings("interning:unnecessary.equals" /* This violation will be rewritten. */) boolean before(T a, T b) { return Refaster.anyOf(a.equals(b), Objects.equals(a, b)); } @@ -96,7 +98,7 @@ final class EqualityRules { } @BeforeTemplate - boolean before(Object a, Object b) { + boolean before(@Interned Object a, @Interned Object b) { return !(a == b); } @@ -127,7 +129,7 @@ final class EqualityRules { } @BeforeTemplate - boolean before(Object a, Object b) { + boolean before(@Interned Object a, @Interned Object b) { return !(a != b); } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PatternRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PatternRules.java index 39ad8826..98171089 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PatternRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/PatternRules.java @@ -7,6 +7,7 @@ import com.google.errorprone.refaster.annotation.AfterTemplate; import com.google.errorprone.refaster.annotation.BeforeTemplate; import java.util.function.Predicate; import java.util.regex.Pattern; +import org.checkerframework.checker.regex.qual.Regex; import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation; /** Refaster rules related to code dealing with regular expressions. */ @@ -34,12 +35,12 @@ final class PatternRules { /** Prefer {@link Pattern#asPredicate()} over non-JDK alternatives. */ static final class PatternCompileAsPredicate { @BeforeTemplate - Predicate before(String pattern) { + Predicate before(@Regex String pattern) { return containsPattern(pattern); } @AfterTemplate - Predicate after(String pattern) { + Predicate after(@Regex String pattern) { return Pattern.compile(pattern).asPredicate(); } } diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java index be5a2b47..9bb9a573 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/StreamRules.java @@ -452,7 +452,11 @@ final class StreamRules { static final class StreamMapToIntSum { @BeforeTemplate - @SuppressWarnings("java:S4266" /* This violation will be rewritten. */) + @SuppressWarnings({ + "interning:return" /* Handle inference failure. */, + "java:S4266" /* This violation will be rewritten. */, + "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict" + }) long before(Stream stream, ToIntFunction mapper) { return stream.collect(summingInt(mapper)); } @@ -472,7 +476,11 @@ final class StreamRules { static final class StreamMapToDoubleSum { @BeforeTemplate - @SuppressWarnings("java:S4266" /* This violation will be rewritten. */) + @SuppressWarnings({ + "interning:return" /* Handle inference failure. */, + "java:S4266" /* This violation will be rewritten. */, + "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict" + }) double before(Stream stream, ToDoubleFunction mapper) { return stream.collect(summingDouble(mapper)); } @@ -492,7 +500,11 @@ final class StreamRules { static final class StreamMapToLongSum { @BeforeTemplate - @SuppressWarnings("java:S4266" /* This violation will be rewritten. */) + @SuppressWarnings({ + "interning:return" /* Handle inference failure. */, + "java:S4266" /* This violation will be rewritten. */, + "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict" + }) long before(Stream stream, ToLongFunction mapper) { return stream.collect(summingLong(mapper)); } @@ -548,7 +560,11 @@ final class StreamRules { static final class StreamCount { @BeforeTemplate - @SuppressWarnings("java:S4266" /* This violation will be rewritten. */) + @SuppressWarnings({ + "interning:return" /* Handle inference failure. */, + "java:S4266" /* This violation will be rewritten. */, + "key-to-resolve-AnnotationUseStyle-and-TrailingComment-check-conflict" + }) long before(Stream stream) { return stream.collect(counting()); } diff --git a/pom.xml b/pom.xml index c3141065..d2902cb0 100644 --- a/pom.xml +++ b/pom.xml @@ -203,6 +203,7 @@ that need to be referenced only once should *not* be listed here. --> 1.1.1 1.10.4 + 3.42.0 ${version.error-prone-orig} v${version.error-prone-orig}-picnic-1 2.24.0 @@ -411,7 +412,17 @@ org.checkerframework checker-qual - 3.42.0 + ${version.checker-framework} + + + org.checkerframework + checker-util + ${version.checker-framework} + + + org.checkerframework + dataflow-shaded + ${version.checker-framework} org.hamcrest @@ -481,6 +492,22 @@ + + + + + org.checkerframework + checker-qual + provided + + + org.checkerframework + dataflow-shaded + provided + + @@ -930,12 +957,54 @@ errorprone-slf4j ${version.error-prone-slf4j} + + org.checkerframework + checker + ${version.checker-framework} + + + + org.checkerframework + checker-qual + ${version.checker-framework} + org.mockito mockito-errorprone ${version.mockito} + + com.google.auto.service.processor.AutoServiceProcessor + com.google.auto.value.processor.AutoAnnotationProcessor + com.google.auto.value.processor.AutoValueProcessor + org.checkerframework.checker.calledmethods.CalledMethodsChecker + + org.checkerframework.checker.formatter.FormatterChecker + + org.checkerframework.checker.interning.InterningChecker + + + org.checkerframework.checker.regex.RegexChecker + + org.checkerframework.checker.signature.SignatureChecker + org.checkerframework.common.initializedfields.InitializedFieldsChecker + --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED @@ -944,10 +1013,41 @@ --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED + -AskipDefs=\.Auto(Annotation|Value)_ + + -AskipUses=^java\.util\.Objects$ + + -AcheckPurityAnnotations + -AconcurrentSemantics + + -AignoreRawTypeArguments=false + + -AresolveReflection + -AshowSuppressWarningsStrings + -AwarnRedundantAnnotations + -AwarnUnneededSuppressions + -ArequirePrefixInWarningSuppressions + -AshowPrefixInWarningMessages + + -Xmaxerrs 10000 -Xmaxwarns 10000 + true ${version.jdk} diff --git a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java index f62efde1..33dfd19d 100644 --- a/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java +++ b/refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java @@ -25,6 +25,7 @@ import com.sun.tools.javac.util.Name; import java.io.IOException; import java.io.ObjectOutput; import java.io.ObjectOutputStream; +import java.io.OutputStream; import java.io.UncheckedIOException; import java.lang.annotation.Annotation; import java.util.Map; @@ -145,7 +146,8 @@ final class RefasterRuleCompilerTaskListener implements TaskListener { private static void outputCodeTransformer(CodeTransformer codeTransformer, FileObject target) throws IOException { - try (ObjectOutput output = new ObjectOutputStream(target.openOutputStream())) { + try (OutputStream stream = target.openOutputStream(); + ObjectOutput output = new ObjectOutputStream(stream)) { output.writeObject(codeTransformer); } } diff --git a/refaster-runner/pom.xml b/refaster-runner/pom.xml index 78d7c95e..53c325ff 100644 --- a/refaster-runner/pom.xml +++ b/refaster-runner/pom.xml @@ -67,6 +67,10 @@ assertj-core test + + org.checkerframework + checker-util + org.jspecify jspecify diff --git a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java index f2a5d8c3..7431490c 100644 --- a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java +++ b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/Refaster.java @@ -42,6 +42,7 @@ import java.util.Optional; import java.util.regex.Pattern; import java.util.stream.Stream; import javax.inject.Inject; +import org.checkerframework.checker.regex.util.RegexUtil; /** * A {@link BugChecker} that flags code that can be simplified using Refaster rules located on the @@ -210,7 +211,7 @@ public final class Refaster extends BugChecker implements CompilationUnitTreeMat return CompositeCodeTransformer.compose( flags .get(INCLUDED_RULES_PATTERN_FLAG) - .map(Pattern::compile) + .map(pattern -> Pattern.compile(RegexUtil.asRegex(pattern))) .>map( nameFilter -> filterCodeTransformers(allTransformers, nameFilter)) .orElseGet(allTransformers::values)); diff --git a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java index 8dd3bc2a..29322177 100644 --- a/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java +++ b/refaster-runner/src/test/java/tech/picnic/errorprone/refaster/runner/RefasterTest.java @@ -178,6 +178,7 @@ final class RefasterTest { } } + @SuppressWarnings({"regex:argument", "regex:group.count"}) private static ImmutableList extractRefasterSeverities( String fileName, String message) { return Pattern.compile( diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsEmpty.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsEmpty.java index fb0c77b7..fd44397a 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsEmpty.java +++ b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/matchers/IsEmpty.java @@ -56,6 +56,7 @@ import java.util.stream.Stream; // XXX: Also recognize empty builders and `emptyBuilder.build()` invocations. public final class IsEmpty implements Matcher { private static final long serialVersionUID = 1L; + private static final Integer ZERO = 0; private static final Pattern EMPTY_INSTANCE_FACTORY_METHOD_PATTERN = Pattern.compile("empty.*"); private static final Matcher EMPTY_COLLECTION_CONSTRUCTOR_ARGUMENT = anyOf(isPrimitiveType(), isSubtypeOf(Comparator.class)); @@ -145,7 +146,7 @@ public final class IsEmpty implements Matcher { NewArrayTree newArray = (NewArrayTree) tree; return (!newArray.getDimensions().isEmpty() - && ASTHelpers.constValue(newArray.getDimensions().get(0), Integer.class) == 0) + && ZERO.equals(ASTHelpers.constValue(newArray.getDimensions().get(0), Integer.class))) || (newArray.getInitializers() != null && newArray.getInitializers().isEmpty()); } } diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/AnnotatedCompositeCodeTransformerTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/AnnotatedCompositeCodeTransformerTest.java index f83353f6..fb5e8366 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/AnnotatedCompositeCodeTransformerTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/AnnotatedCompositeCodeTransformerTest.java @@ -30,6 +30,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.stream.Stream; +import org.checkerframework.checker.mustcall.qual.MustCall; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -149,7 +150,7 @@ final class AnnotatedCompositeCodeTransformerTest { ImmutableSet annotations, Context expectedContext, Description returnedDescription) { - CodeTransformer codeTransformer = mock(); + @MustCall CodeTransformer codeTransformer = mock(); when(codeTransformer.annotations()).thenReturn(indexAnnotations(annotations)); doAnswer( @@ -183,7 +184,7 @@ final class AnnotatedCompositeCodeTransformerTest { private static Context context() { // XXX: Use `ErrorProneOptions#processArgs` to test the // `AnnotatedCompositeCodeTransformer#overrideSeverity` logic. - Context context = mock(); + @MustCall Context context = mock(); when(context.get(ErrorProneOptions.class)).thenReturn(ErrorProneOptions.empty()); return context; } diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java index 2f1f7e73..29e1a7fb 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/AbstractMatcherTestChecker.java @@ -35,7 +35,9 @@ abstract class AbstractMatcherTestChecker extends BugChecker implements Compilat @Override public Description matchCompilationUnit(CompilationUnitTree compilationUnit, VisitorState state) { new TreePathScanner<@Nullable Void, @Nullable Void>() { + // XXX: Update `TreePathScanner#scan` stub to declare its first parameter `@Nullable`. @Override + @SuppressWarnings("nullness:argument") public @Nullable Void scan(@Nullable Tree tree, @Nullable Void unused) { if (tree instanceof ExpressionTree) { TreePath path = new TreePath(getCurrentPath(), tree); diff --git a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsEmptyTest.java b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsEmptyTest.java index 318328cb..a6d6635a 100644 --- a/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsEmptyTest.java +++ b/refaster-support/src/test/java/tech/picnic/errorprone/refaster/matchers/IsEmptyTest.java @@ -55,6 +55,12 @@ final class IsEmptyTest { " return new int[][] {{0}};", " }", "", + " // XXX: Renumber the methods below.", + " int[] negative5X() {", + " int i = hashCode();", + " return new int[i];", + " }", + "", " Random negative5() {", " return new Random();", " }",