From 2455ece333e4c98246a7b6f9343052149df27938 Mon Sep 17 00:00:00 2001 From: Rick Ossendrijver Date: Thu, 23 Jun 2022 11:16:20 +0200 Subject: [PATCH] Support records in `InvalidParam` Resolves #2321 --- .../google/errorprone/util/ASTHelpers.java | 7 ++ .../bugpatterns/javadoc/InvalidParam.java | 20 ++- .../bugpatterns/javadoc/InvalidParamTest.java | 116 ++++++++++++++++++ 3 files changed, 142 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 d56da359ac4..111bbe66a7a 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 @@ -798,6 +798,13 @@ private static boolean isFinal(Symbol symbol) { return (symbol.flags() & Flags.FINAL) == Flags.FINAL; } + private static final long RECORD_FLAG = 1L << 61; + + /** Returns true if the given {@link Symbol} is a record. */ + public static boolean isRecord(Symbol symbol) { + return (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..16d05a28946 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 @@ -33,6 +33,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 +75,19 @@ 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(); + // XXX: `isRecord`... should it also be able to handle `Tree`s? + ImmutableSet parameters = + ASTHelpers.isRecord(ASTHelpers.getSymbol(classTree)) + ? getGeneratedConstructor(classTree).getParameters().stream() + .map(v -> v.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 +110,15 @@ public Description matchMethod(MethodTree methodTree, VisitorState state) { return Description.NO_MATCH; } + private static MethodTree getGeneratedConstructor(ClassTree classTree) { + return classTree.getMembers().stream() + .filter(MethodTree.class::isInstance) + .map(MethodTree.class::cast) + .findFirst() + .filter(tree -> tree.getName().contentEquals("")) + .orElseThrow(() -> new IllegalStateException("Records must have a generated ctor")); + } + /** 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(); + } }