From 3a3eedcd181754d5c4f837afc034262cb99d7f42 Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Fri, 23 Oct 2020 13:04:11 -0400 Subject: [PATCH] Implement error-prone ObjectsHashCodeUnnecessaryVarargs migration (#1522) Implement error-prone ObjectsHashCodeUnnecessaryVarargs migration --- README.md | 1 + .../ObjectsHashCodeUnnecessaryVarargs.java | 61 +++++++++++++++++++ ...ObjectsHashCodeUnnecessaryVarargsTest.java | 61 +++++++++++++++++++ .../errorprone/RefactoringValidator.java | 2 + changelog/@unreleased/pr-1522.v2.yml | 5 ++ .../BaselineErrorProneExtension.java | 1 + 6 files changed, 131 insertions(+) create mode 100644 baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargs.java create mode 100644 baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargsTest.java create mode 100644 changelog/@unreleased/pr-1522.v2.yml diff --git a/README.md b/README.md index dd1b52af8..93a6207a9 100644 --- a/README.md +++ b/README.md @@ -190,6 +190,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c - `ExtendsErrorOrThrowable`: Avoid extending Error (or subclasses of it) or Throwable directly. - `ImmutablesStyleCollision`: Prevent unintentionally voiding immutables Style meta-annotations through the introduction of inline style annotations. - `TooManyArguments`: Prefer Interface that take few arguments rather than many. +- `ObjectsHashCodeUnnecessaryVarargs`: java.util.Objects.hash(non-varargs) should be replaced with java.util.Objects.hashCode(value) to avoid unnecessary varargs array allocations. - `PreferStaticLoggers`: Prefer static loggers over instance loggers. - `LogsafeArgName`: Prevent certain named arguments as being logged as safe. Specify unsafe argument names using `LogsafeArgName:UnsafeArgNames` errorProne flag. - `ImplicitPublicBuilderConstructor`: Prevent builders from unintentionally leaking public constructors. diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargs.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargs.java new file mode 100644 index 000000000..2fcff683c --- /dev/null +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargs.java @@ -0,0 +1,61 @@ +/* + * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.BugPattern.LinkType; +import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFixes; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.Matchers; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; + +@AutoService(BugChecker.class) +@BugPattern( + name = "ObjectsHashCodeUnnecessaryVarargs", + link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks", + linkType = LinkType.CUSTOM, + severity = SeverityLevel.WARNING, + summary = "java.util.Objects.hash(non-varargs) should be replaced with java.util.Objects.hashCode(value) " + + "to avoid unnecessary varargs array allocations.") +public final class ObjectsHashCodeUnnecessaryVarargs extends BugChecker + implements BugChecker.MethodInvocationTreeMatcher { + + private static final Matcher HASH_MATCHER = + MethodMatchers.staticMethod().onClass("java.util.Objects").named("hash"); + + private static final Matcher ARRAY_MATCHER = Matchers.isArrayType(); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (HASH_MATCHER.matches(tree, state) && tree.getArguments().size() == 1) { + ExpressionTree argument = tree.getArguments().get(0); + if (!ARRAY_MATCHER.matches(argument, state)) { + return buildDescription(tree) + .addFix(SuggestedFixes.renameMethodInvocation(tree, "hashCode", state)) + .build(); + } + } + return Description.NO_MATCH; + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargsTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargsTest.java new file mode 100644 index 000000000..f6346a225 --- /dev/null +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ObjectsHashCodeUnnecessaryVarargsTest.java @@ -0,0 +1,61 @@ +/* + * (c) Copyright 2020 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.errorprone; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +class ObjectsHashCodeUnnecessaryVarargsTest { + + @Test + public void test() { + fix().addInputLines( + "Test.java", + "import java.util.Objects;", + "public class Test {", + " void f(Object a, Object[] b, String[] c, String d) {", + " Objects.hash(a);", + " Objects.hash(a, a);", + " Objects.hash(b);", + " Objects.hash(c);", + " Objects.hash(d);", + " }", + "}") + .addOutputLines( + "Test.java", + "import java.util.Objects;", + "public class Test {", + " void f(Object a, Object[] b, String[] c, String d) {", + " Objects.hashCode(a);", + " Objects.hash(a, a);", + " Objects.hash(b);", + " // BUG: Diagnostic contains: non-varargs call of varargs method", + " Objects.hash(c);", + " Objects.hashCode(d);", + " }", + "}") + .doTest(); + } + + private RefactoringValidator fix() { + return RefactoringValidator.of(new ObjectsHashCodeUnnecessaryVarargs(), getClass()); + } + + public CompilationTestHelper helper() { + return CompilationTestHelper.newInstance(ObjectsHashCodeUnnecessaryVarargs.class, getClass()); + } +} diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/RefactoringValidator.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/RefactoringValidator.java index 8dc6d8b43..8c7d4037a 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/RefactoringValidator.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/RefactoringValidator.java @@ -88,6 +88,7 @@ void doTest() { delegate.doTest(); helper.compilationHelper .addSourceLines(helper.outputPath, helper.outputLines) + .matchAllDiagnostics() .doTest(); } @@ -95,6 +96,7 @@ void doTest(BugCheckerRefactoringTestHelper.TestMode testMode) { delegate.doTest(testMode); helper.compilationHelper .addSourceLines(helper.outputPath, helper.outputLines) + .matchAllDiagnostics() .doTest(); } diff --git a/changelog/@unreleased/pr-1522.v2.yml b/changelog/@unreleased/pr-1522.v2.yml new file mode 100644 index 000000000..d196ccdd6 --- /dev/null +++ b/changelog/@unreleased/pr-1522.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Implement error-prone ObjectsHashCodeUnnecessaryVarargs migration + links: + - https://github.com/palantir/gradle-baseline/pull/1522 diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java index 365d4efd6..ca5e24789 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java @@ -41,6 +41,7 @@ public class BaselineErrorProneExtension { "LambdaMethodReference", "LoggerEnclosingClass", "LogsafeArgName", + "ObjectsHashCodeUnnecessaryVarargs", "OptionalFlatMapOfNullable", "OptionalOrElseMethodInvocation", "PreferBuiltInConcurrentKeySet",