Picnic-Bot
2023-05-14 17:01:46 +02:00
committed by GitHub
parent 137ec4c573
commit 08e55fdfb6
22 changed files with 77 additions and 228 deletions

View File

@@ -126,6 +126,11 @@
<artifactId>jakarta.servlet-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>

View File

@@ -40,6 +40,7 @@ import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;
import javax.inject.Inject;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.bugpatterns.util.AnnotationAttributeMatcher;
import tech.picnic.errorprone.bugpatterns.util.Flags;
@@ -94,7 +95,8 @@ public final class LexicographicalAnnotationAttributeListing extends BugChecker
*
* @param flags Any provided command line flags.
*/
public LexicographicalAnnotationAttributeListing(ErrorProneFlags flags) {
@Inject
LexicographicalAnnotationAttributeListing(ErrorProneFlags flags) {
matcher = createAnnotationAttributeMatcher(flags);
}

View File

@@ -50,6 +50,7 @@ import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;
import javax.inject.Inject;
import tech.picnic.errorprone.bugpatterns.util.Flags;
import tech.picnic.errorprone.bugpatterns.util.MethodMatcherFactory;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
@@ -155,7 +156,8 @@ public final class RedundantStringConversion extends BugChecker
*
* @param flags Any provided command line flags.
*/
public RedundantStringConversion(ErrorProneFlags flags) {
@Inject
RedundantStringConversion(ErrorProneFlags flags) {
conversionMethodMatcher = createConversionMethodMatcher(flags);
}

View File

@@ -27,6 +27,7 @@ import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.suppliers.Suppliers;
import com.sun.source.tree.Tree;
import com.sun.source.tree.VariableTree;
import javax.inject.Inject;
import tech.picnic.errorprone.bugpatterns.util.Flags;
/** A {@link BugChecker} that flags {@code @RequestParam} parameters with an unsupported type. */
@@ -55,7 +56,8 @@ public final class RequestParamType extends BugChecker implements VariableTreeMa
*
* @param flags Any provided command line flags.
*/
public RequestParamType(ErrorProneFlags flags) {
@Inject
RequestParamType(ErrorProneFlags flags) {
hasUnsupportedRequestParamType = hasUnsupportedRequestParamType(flags);
}

View File

@@ -1,89 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
import static com.sun.tools.javac.parser.Tokens.TokenKind.RPAREN;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.Streams;
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.Fix;
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.errorprone.util.ErrorProneTokens;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.util.Position;
import tech.picnic.errorprone.bugpatterns.util.SourceCode;
/**
* A {@link BugChecker} that flags calls to {@link String#toLowerCase()} and {@link
* String#toUpperCase()}, as these methods implicitly rely on the environment's default locale.
*/
// XXX: Also flag `String::toLowerCase` and `String::toUpperCase` method references. For these cases
// the suggested fix should introduce a lambda expression with a parameter of which the name does
// not coincide with the name of an existing variable name. Such functionality should likely be
// introduced in a utility class.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Specify a `Locale` when calling `String#to{Lower,Upper}Case`",
link = BUG_PATTERNS_BASE_URL + "StringCaseLocaleUsage",
linkType = CUSTOM,
severity = WARNING,
tags = FRAGILE_CODE)
public final class StringCaseLocaleUsage extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> DEFAULT_LOCALE_CASE_CONVERSION =
instanceMethod()
.onExactClass(String.class.getName())
.namedAnyOf("toLowerCase", "toUpperCase")
.withNoParameters();
/** Instantiates a new {@link StringCaseLocaleUsage} instance. */
public StringCaseLocaleUsage() {}
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (!DEFAULT_LOCALE_CASE_CONVERSION.matches(tree, state)) {
return Description.NO_MATCH;
}
int closingParenPosition = getClosingParenPosition(tree, state);
if (closingParenPosition == Position.NOPOS) {
return describeMatch(tree);
}
return buildDescription(tree)
.addFix(suggestLocale(closingParenPosition, "Locale.ROOT"))
.addFix(suggestLocale(closingParenPosition, "Locale.getDefault()"))
.build();
}
private static Fix suggestLocale(int insertPosition, String locale) {
return SuggestedFix.builder()
.addImport("java.util.Locale")
.replace(insertPosition, insertPosition, locale)
.build();
}
private static int getClosingParenPosition(MethodInvocationTree tree, VisitorState state) {
int startPosition = ASTHelpers.getStartPosition(tree);
if (startPosition == Position.NOPOS) {
return Position.NOPOS;
}
return Streams.findLast(
ErrorProneTokens.getTokens(SourceCode.treeToString(tree, state), state.context).stream()
.filter(t -> t.kind() == RPAREN))
.map(token -> startPosition + token.pos())
.orElse(Position.NOPOS);
}
}

View File

@@ -3,6 +3,7 @@ package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChooser;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -25,6 +26,7 @@ final class BugCheckerRules {
}
@AfterTemplate
@CanIgnoreReturnValue
BugCheckerRefactoringTestHelper after(BugCheckerRefactoringTestHelper helper) {
return helper;
}

View File

@@ -12,6 +12,7 @@ import static java.util.function.Function.identity;
import com.google.common.collect.Comparators;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -78,6 +79,7 @@ final class ComparatorRules {
}
@AfterTemplate
@CanIgnoreReturnValue
@UseImportPolicy(STATIC_IMPORT_ALWAYS)
Comparator<T> after(Comparator<T> cmp) {
return cmp;

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -27,6 +28,7 @@ final class DoubleStreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
DoubleStream after(DoubleStream stream) {
return stream;
}

View File

@@ -1,5 +1,6 @@
package tech.picnic.errorprone.refasterrules;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.AlsoNegation;
@@ -63,6 +64,7 @@ final class EqualityRules {
}
@AfterTemplate
@CanIgnoreReturnValue
boolean after(boolean b) {
return b;
}

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -40,6 +41,7 @@ final class IntStreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
IntStream after(IntStream stream) {
return stream;
}

View File

@@ -1,6 +1,7 @@
package tech.picnic.errorprone.refasterrules;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -40,6 +41,7 @@ final class LongStreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
LongStream after(LongStream stream) {
return stream;
}

View File

@@ -3,6 +3,7 @@ package tech.picnic.errorprone.refasterrules;
import static com.google.errorprone.refaster.ImportPolicy.STATIC_IMPORT_ALWAYS;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -351,6 +352,7 @@ final class OptionalRules {
}
@AfterTemplate
@CanIgnoreReturnValue
Optional<T> after(Optional<T> optional) {
return optional;
}

View File

@@ -14,6 +14,7 @@ import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -411,6 +412,7 @@ final class ReactorRules {
}
@AfterTemplate
@CanIgnoreReturnValue
Mono<T> after(Mono<T> mono) {
return mono;
}
@@ -424,6 +426,7 @@ final class ReactorRules {
}
@AfterTemplate
@CanIgnoreReturnValue
Flux<T> after(Flux<T> flux) {
return flux;
}
@@ -1276,6 +1279,7 @@ final class ReactorRules {
}
@AfterTemplate
@CanIgnoreReturnValue
StepVerifier.Step<T> after(StepVerifier.Step<T> step) {
return step;
}

View File

@@ -21,6 +21,7 @@ import static java.util.stream.Collectors.summingInt;
import static java.util.stream.Collectors.summingLong;
import com.google.common.collect.Streams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
@@ -124,6 +125,7 @@ final class StreamRules {
}
@AfterTemplate
@CanIgnoreReturnValue
Stream<T> after(Stream<T> stream) {
return stream;
}

View File

@@ -1,124 +0,0 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.errorprone.BugCheckerRefactoringTestHelper;
import com.google.errorprone.BugCheckerRefactoringTestHelper.FixChoosers;
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class StringCaseLocaleUsageTest {
@Test
void identification() {
CompilationTestHelper.newInstance(StringCaseLocaleUsage.class, getClass())
.addSourceLines(
"A.java",
"import static java.util.Locale.ROOT;",
"",
"import java.util.Locale;",
"",
"class A {",
" void m() {",
" \"a\".toLowerCase(Locale.ROOT);",
" \"a\".toUpperCase(Locale.ROOT);",
" \"b\".toLowerCase(ROOT);",
" \"b\".toUpperCase(ROOT);",
" \"c\".toLowerCase(Locale.getDefault());",
" \"c\".toUpperCase(Locale.getDefault());",
" \"d\".toLowerCase(Locale.ENGLISH);",
" \"d\".toUpperCase(Locale.ENGLISH);",
" \"e\".toLowerCase(new Locale(\"foo\"));",
" \"e\".toUpperCase(new Locale(\"foo\"));",
"",
" // BUG: Diagnostic contains:",
" \"f\".toLowerCase();",
" // BUG: Diagnostic contains:",
" \"g\".toUpperCase();",
"",
" String h = \"h\";",
" // BUG: Diagnostic contains:",
" h.toLowerCase();",
" String i = \"i\";",
" // BUG: Diagnostic contains:",
" i.toUpperCase();",
" }",
"}")
.doTest();
}
@Test
void replacementFirstSuggestedFix() {
BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass())
.addInputLines(
"A.java",
"class A {",
" void m() {",
" \"a\".toLowerCase(/* Comment with parens: (). */ );",
" \"b\".toUpperCase();",
" \"c\".toLowerCase().toString();",
"",
" toString().toLowerCase();",
" toString().toUpperCase /* Comment with parens: (). */();",
"",
" this.toString().toLowerCase() /* Comment with parens: (). */;",
" this.toString().toUpperCase();",
" }",
"}")
.addOutputLines(
"A.java",
"import java.util.Locale;",
"",
"class A {",
" void m() {",
" \"a\".toLowerCase(/* Comment with parens: (). */ Locale.ROOT);",
" \"b\".toUpperCase(Locale.ROOT);",
" \"c\".toLowerCase(Locale.ROOT).toString();",
"",
" toString().toLowerCase(Locale.ROOT);",
" toString().toUpperCase /* Comment with parens: (). */(Locale.ROOT);",
"",
" this.toString().toLowerCase(Locale.ROOT) /* Comment with parens: (). */;",
" this.toString().toUpperCase(Locale.ROOT);",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
@Test
void replacementSecondSuggestedFix() {
BugCheckerRefactoringTestHelper.newInstance(StringCaseLocaleUsage.class, getClass())
.setFixChooser(FixChoosers.SECOND)
.addInputLines(
"A.java",
"class A {",
" void m() {",
" \"a\".toLowerCase();",
" \"b\".toUpperCase(/* Comment with parens: (). */ );",
" \"c\".toLowerCase().toString();",
"",
" toString().toLowerCase();",
" toString().toUpperCase /* Comment with parens: (). */();",
"",
" this.toString().toLowerCase() /* Comment with parens: (). */;",
" this.toString().toUpperCase();",
" }",
"}")
.addOutputLines(
"A.java",
"import java.util.Locale;",
"",
"class A {",
" void m() {",
" \"a\".toLowerCase(Locale.getDefault());",
" \"b\".toUpperCase(/* Comment with parens: (). */ Locale.getDefault());",
" \"c\".toLowerCase(Locale.getDefault()).toString();",
"",
" toString().toLowerCase(Locale.getDefault());",
" toString().toUpperCase /* Comment with parens: (). */(Locale.getDefault());",
"",
" this.toString().toLowerCase(Locale.getDefault()) /* Comment with parens: (). */;",
" this.toString().toUpperCase(Locale.getDefault());",
" }",
"}")
.doTest(TestMode.TEXT_MATCH);
}
}

24
pom.xml
View File

@@ -201,7 +201,7 @@
<version.auto-value>1.10.1</version.auto-value>
<version.error-prone>${version.error-prone-orig}</version.error-prone>
<version.error-prone-fork>v${version.error-prone-orig}-picnic-1</version.error-prone-fork>
<version.error-prone-orig>2.18.0</version.error-prone-orig>
<version.error-prone-orig>2.19.1</version.error-prone-orig>
<version.error-prone-slf4j>0.1.18</version.error-prone-slf4j>
<version.guava-beta-checker>1.0</version.guava-beta-checker>
<version.jdk>11</version.jdk>
@@ -362,6 +362,11 @@
<artifactId>jakarta.servlet-api</artifactId>
<version>6.0.0</version>
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<version>1</version>
</dependency>
<dependency>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
@@ -981,6 +986,17 @@
<ignoreClass>org.checkerframework.dataflow.qual.*</ignoreClass>
</ignoreClasses>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-aop</artifactId>
<!-- Spring AOP redistributes the AOP
Alliance interfaces of the
`aopalliance:aopalliance` artifact. See
https://jira.spring.io/browse/SPR-13984. -->
<ignoreClasses>
<ignoreClass>org.aopalliance.*</ignoreClass>
</ignoreClasses>
</dependency>
</dependencies>
<findAllDuplicates>true</findAllDuplicates>
</banDuplicateClasses>
@@ -1700,10 +1716,6 @@
<!-- XXX: Enable this once we open-source
this library. -->
-Xep:BetaApi:OFF
<!-- XXX: Enable once compatible with
`UnnecessarilyVisible`. See
https://github.com/google/error-prone/issues/3706. -->
-Xep:InjectOnBugCheckers:OFF
<!-- We don't target JDK 7. -->
-Xep:Java7ApiChecker:OFF
<!-- We don't target JDK 8. -->
@@ -1724,6 +1736,8 @@
-XepOpt:NullAway:AssertsEnabled=true
-XepOpt:NullAway:CheckOptionalEmptiness=true
-XepOpt:Nullness:Conservative=false
-XepOpt:StatementSwitchToExpressionSwitch:EnableAssignmentSwitchConversion=true
-XepOpt:StatementSwitchToExpressionSwitch:EnableReturnSwitchConversion=true
<!-- XXX: Enable once this check respects
the compilation source version. See
https://github.com/google/error-prone/pull/3646.

View File

@@ -56,6 +56,11 @@
<artifactId>guava</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>

View File

@@ -10,6 +10,7 @@ import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static java.util.function.Predicate.not;
import com.google.auto.service.AutoService;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableListMultimap;
@@ -40,6 +41,7 @@ import java.util.Map;
import java.util.Optional;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import javax.inject.Inject;
/**
* A {@link BugChecker} that flags code that can be simplified using Refaster rules located on the
@@ -75,6 +77,8 @@ public final class Refaster extends BugChecker implements CompilationUnitTreeMat
*
* @param flags Any provided command line flags.
*/
@Inject
@VisibleForTesting
public Refaster(ErrorProneFlags flags) {
codeTransformer = createCompositeCodeTransformer(flags);
}
@@ -165,15 +169,15 @@ public final class Refaster extends BugChecker implements CompilationUnitTreeMat
* that could cause {@link VisitorState#reportMatch(Description)}} to override the reported
* severity).
*/
@SuppressWarnings("RestrictedApiChecker" /* We create a heavily customized `Description` here. */)
@SuppressWarnings("RestrictedApi" /* We create a heavily customized `Description` here. */)
private static Description augmentDescription(
Description description, Optional<SeverityLevel> severityOverride) {
return Description.builder(
description.position,
"Refaster Rule",
description.getLink(),
severityOverride.orElse(description.severity),
String.join(": ", description.checkName, description.getRawMessage()))
.overrideSeverity(severityOverride.orElse(description.severity()))
.addAllFixes(description.fixes)
.build();
}

View File

@@ -78,7 +78,7 @@ public abstract class AnnotatedCompositeCodeTransformer implements CodeTransform
}
}
@SuppressWarnings("RestrictedApiChecker" /* We create a heavily customized `Description` here. */)
@SuppressWarnings("RestrictedApi" /* We create a heavily customized `Description` here. */)
private Description augmentDescription(
Description description, CodeTransformer delegate, Context context) {
String shortCheckName = getShortCheckName(description.checkName);
@@ -86,8 +86,8 @@ public abstract class AnnotatedCompositeCodeTransformer implements CodeTransform
description.position,
shortCheckName,
getLinkPattern(delegate, shortCheckName).orElse(null),
overrideSeverity(getSeverity(delegate), context),
getDescription(delegate))
.overrideSeverity(overrideSeverity(getSeverity(delegate), context))
.addAllFixes(description.fixes)
.build();
}

View File

@@ -171,10 +171,11 @@ final class AnnotatedCompositeCodeTransformerTest {
return description(name, Optional.of(""), WARNING, "");
}
@SuppressWarnings("RestrictedApiChecker" /* We create a heavily customized `Description` here. */)
@SuppressWarnings("RestrictedApi" /* We create a heavily customized `Description` here. */)
private static Description description(
String name, Optional<String> link, SeverityLevel severityLevel, String message) {
return Description.builder(DUMMY_POSITION, name, link.orElse(null), severityLevel, message)
return Description.builder(DUMMY_POSITION, name, link.orElse(null), message)
.overrideSeverity(severityLevel)
.addFix(DUMMY_FIX)
.build();
}

View File

@@ -49,6 +49,11 @@
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>javax.inject</groupId>
<artifactId>javax.inject</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.jspecify</groupId>
<artifactId>jspecify</artifactId>

View File

@@ -44,6 +44,7 @@ import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
import javax.inject.Inject;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.refaster.runner.CodeTransformers;
import tech.picnic.errorprone.refaster.runner.Refaster;
@@ -77,7 +78,8 @@ public final class RefasterRuleCollection extends BugChecker implements Compilat
*
* @param flags Any provided command line flags.
*/
public RefasterRuleCollection(ErrorProneFlags flags) {
@Inject
RefasterRuleCollection(ErrorProneFlags flags) {
ruleCollectionUnderTest = getRuleCollectionUnderTest(flags);
delegate = createRefasterChecker(ruleCollectionUnderTest);
rulesUnderTest = getRulesUnderTest(ruleCollectionUnderTest);