From 2695c4d9e29a841ff799e5f4e77d25fe531c9e05 Mon Sep 17 00:00:00 2001 From: Stephan Schroevers Date: Sat, 16 Dec 2023 17:12:14 +0100 Subject: [PATCH] Address PR feedback --- .../bugpatterns/CanonicalClassNameUsage.java | 16 ++++++++++++++-- .../bugpatterns/CanonicalClassNameUsageTest.java | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsage.java b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsage.java index 6b356103e54..eb62e6b41b8 100644 --- a/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsage.java +++ b/error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsage.java @@ -3,7 +3,12 @@ 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.Matchers.allOf; +import static com.google.errorprone.matchers.Matchers.anything; +import static com.google.errorprone.matchers.Matchers.classLiteral; import static com.google.errorprone.matchers.Matchers.instanceMethod; +import static com.google.errorprone.matchers.Matchers.receiverOfInvocation; +import static com.google.errorprone.matchers.Matchers.toType; import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL; import com.google.auto.service.AutoService; @@ -29,8 +34,11 @@ * *

For top-level types these two methods generally return the same result, but for nested types * the former separates identifiers using a dollar sign ({@code $}) rather than a dot ({@code .}). + * + * @implNote This check currently only flags {@link Class#getName()} invocations on class literals, + * and doesn't flag method references. This avoids false positives, such as suggesting use of + * {@link Class#getCanonicalName()} in contexts where the canonical name is {@code null}. */ -// XXX: This check currently doesn't flag `Class::getName` method references. @AutoService(BugChecker.class) @BugPattern( summary = "This code should likely use the type's canonical name", @@ -42,7 +50,11 @@ public final class CanonicalClassNameUsage extends BugChecker implements MethodInvocationTreeMatcher { private static final long serialVersionUID = 1L; private static final Matcher GET_NAME_INVOCATION = - instanceMethod().onExactClass(Class.class.getCanonicalName()).named("getName"); + toType( + MethodInvocationTree.class, + allOf( + receiverOfInvocation(classLiteral(anything())), + instanceMethod().onExactClass(Class.class.getCanonicalName()).named("getName"))); private static final Pattern CANONICAL_NAME_USING_TYPES = Pattern.compile("(com\\.google\\.errorprone|tech\\.picnic\\.errorprone)\\..*"); diff --git a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsageTest.java b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsageTest.java index b471fd24b46..27a57e4b131 100644 --- a/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsageTest.java +++ b/error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/CanonicalClassNameUsageTest.java @@ -29,6 +29,7 @@ void identification() { " instanceMethod().onExactClass(A.class.getCanonicalName());", " MoreTypes.type(A.class.getCanonicalName());", " MoreTypes.type(A.class.getCanonicalName() + \".SubType\");", + " instanceMethod().onExactClass(new Object() {}.getClass().getName());", " instanceMethod().onExactClass(methodInUnnamedPackage(A.class.getName()));", " // BUG: Diagnostic contains:", " instanceMethod().onExactClass(A.class.getName());",