From 16759a4afe9ef838d4689942f6542dc35b1f1637 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Thu, 23 Jun 2022 11:16:20 +0200 Subject: [PATCH] Have `InvalidParam` support records Fixes #2321. --- .../google/errorprone/util/ASTHelpers.java | 23 ++++ .../bugpatterns/javadoc/InvalidParam.java | 20 ++- .../bugpatterns/javadoc/InvalidParamTest.java | 116 ++++++++++++++++++ 3 files changed, 158 insertions(+), 1 deletion(-) diff --git a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java index 9a5415acaa6..4abf7a0a684 100644 --- a/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java +++ b/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java @@ -808,6 +808,29 @@ private static boolean isFinal(Symbol symbol) { return (symbol.flags() & Flags.FINAL) == Flags.FINAL; } + /** + * Flag for record types, canonical record constructors and type members that are part of a + * record's state vector. Can be replaced by {@code com.sun.tools.javac.code.Flags.RECORD} once + * the minimum JDK version is 14. + */ + private static final long RECORD_FLAG = 1L << 61; + + /** + * Returns true if the given {@link Tree} is a record, a record's canonical constructor or a + * member that is part of a record's state vector. + */ + public static boolean isRecord(Tree tree) { + return isRecord(getSymbol(tree)); + } + + /** + * Returns true if the given {@link Symbol} is a record, a record's canonical constructor or a + * member that is part of a record's state vector. + */ + public static boolean isRecord(Symbol symbol) { + return symbol != null && (symbol.flags() & RECORD_FLAG) == RECORD_FLAG; + } + /** * Determines whether a symbol has an annotation of the given type. This includes annotations * inherited from superclasses due to {@code @Inherited}. diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/InvalidParam.java b/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/InvalidParam.java index 4e5ef003e8b..fb57ec144fa 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/InvalidParam.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/javadoc/InvalidParam.java @@ -17,6 +17,7 @@ package com.google.errorprone.bugpatterns.javadoc; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.bugpatterns.javadoc.Utils.diagnosticPosition; import static com.google.errorprone.bugpatterns.javadoc.Utils.getBestMatch; @@ -26,6 +27,7 @@ import static com.google.errorprone.names.LevenshteinEditDistance.getEditDistance; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.MoreCollectors; import com.google.common.collect.Sets; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; @@ -33,6 +35,7 @@ import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; import com.google.errorprone.matchers.Description; +import com.google.errorprone.util.ASTHelpers; import com.sun.source.doctree.DocTree; import com.sun.source.doctree.DocTree.Kind; import com.sun.source.doctree.LiteralTree; @@ -74,11 +77,18 @@ public final class InvalidParam extends BugChecker implements ClassTreeMatcher, public Description matchClass(ClassTree classTree, VisitorState state) { DocTreePath path = getDocTreePath(state); if (path != null) { - ImmutableSet parameters = ImmutableSet.of(); + ImmutableSet parameters = + ASTHelpers.isRecord(classTree) + ? getCanonicalRecordConstructor(classTree).getParameters().stream() + .map(p -> p.getName().toString()) + .collect(toImmutableSet()) + : ImmutableSet.of(); + ImmutableSet typeParameters = classTree.getTypeParameters().stream() .map(t -> t.getName().toString()) .collect(toImmutableSet()); + new ParamsChecker(state, classTree, parameters, typeParameters).scan(path, null); } return Description.NO_MATCH; @@ -101,6 +111,14 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) { return Description.NO_MATCH; } + private static MethodTree getCanonicalRecordConstructor(ClassTree classTree) { + return classTree.getMembers().stream() + .filter(MethodTree.class::isInstance) + .map(MethodTree.class::cast) + .filter(ASTHelpers::isRecord) + .collect(onlyElement()); + } + /** Checks that documented parameters match the method's parameter list. */ private final class ParamsChecker extends DocTreePathScanner { private final VisitorState state; diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/javadoc/InvalidParamTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/javadoc/InvalidParamTest.java index d5feb492daa..f51c107ec11 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/javadoc/InvalidParamTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/javadoc/InvalidParamTest.java @@ -16,9 +16,12 @@ package com.google.errorprone.bugpatterns.javadoc; +import static org.junit.Assume.assumeTrue; + import com.google.errorprone.BugCheckerRefactoringTestHelper; import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode; import com.google.errorprone.CompilationTestHelper; +import com.google.errorprone.util.RuntimeVersion; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -164,4 +167,117 @@ public void excludedName_noMatchDespiteSimilarParam() { "}") .doTest(); } + + @Test + public void negative_record() { + assumeTrue(RuntimeVersion.isAtLeast16()); + helper + .addSourceLines( + "Test.java", // + "/**", + " * @param name Name.", + " */", + "public record Test(String name) {}") + .doTest(); + } + + @Test + public void badParameterName_record() { + assumeTrue(RuntimeVersion.isAtLeast16()); + helper + .addSourceLines( + "Test.java", + "/**", + " // BUG: Diagnostic contains: Parameter name `bar` is unknown", + " * @param bar Foo.", + " */", + "public record Test(String foo) {}") + .doTest(); + } + + @Test + public void multipleConstructors_record() { + assumeTrue(RuntimeVersion.isAtLeast16()); + helper + .addSourceLines( + "Test.java", + "/**", + " * @param foo Foo.", + " * @param bar Bar.", + " */", + "public record Test(String foo, Integer bar) {", + " public Test(Integer bar) {", + " this(null, bar);", + " }", + "", + " /**", + " // BUG: Diagnostic contains: Parameter name `bar` is unknown", + " * @param bar Foo.", + " */", + " public Test(String foo) {", + " this(foo, null);", + " }", + "}") + .doTest(); + } + + @Test + public void typeParameter_record() { + assumeTrue(RuntimeVersion.isAtLeast16()); + helper + .addSourceLines( + "Negative.java", + "/**", + " * @param The type parameter.", + " * @param contents Contents.", + " * @param bar Bar.", + " */", + "public record Negative(T contents, String bar) {}") + .addSourceLines( + "Positive.java", + "/**", + " // BUG: Diagnostic contains: Parameter name `E` is unknown", + " * @param The type parameter.", + " * @param contents Contents.", + " * @param bar Bar.", + " */", + "public record Positive(T contents, String bar) {}") + .doTest(); + } + + @Test + public void compactConstructor_record() { + assumeTrue(RuntimeVersion.isAtLeast16()); + helper + .addSourceLines( + "Test.java", + "/**", + " * @param name Name.", + " */", + "public record Test(String name) {", + " public Test {}", + "}") + .doTest(); + } + + @Test + public void normalConstructor_record() { + assumeTrue(RuntimeVersion.isAtLeast16()); + helper + .addSourceLines( + "Test.java", + "/**", + " * @param name Name.", + " */", + "public record Test(String name) {", + " /**", + " // BUG: Diagnostic contains: Parameter name `foo` is unknown", + " * @param foo Name.", + " */", + " public Test(String name) {", + " this.name = name;", + " }", + " }") + .doTest(); + } }