Lint for @hide tags in comments
Errorprone will warn for normal javadoc tags in comments, but doesn't
know about @hide and so ignores it. This can lead to accidental exposure
of APIs when folks mistakenly use a comment block instead of a javadoc
block.
Also, adds some static dependencies to enable actually running the tests
on the host.
Fixes: 181140148
Test: atest --host error_prone_android_framework_test:com.google.errorprone.bugpatterns.android.HideInCommentsCheckerTest
Change-Id: I46a59075f93468fc0e57264bca0d86994ebb3d86
diff --git a/errorprone/Android.bp b/errorprone/Android.bp
index a927f53..8f32f0e 100644
--- a/errorprone/Android.bp
+++ b/errorprone/Android.bp
@@ -1,4 +1,3 @@
-
package {
// See: http://go/android-license-faq
// A large-scale-change added 'default_applicable_licenses' to import
@@ -42,8 +41,10 @@
static_libs: [
"truth-prebuilt",
"kxml2-2.3.0",
+ "compile-testing-prebuilt",
"error_prone_android_framework_lib",
"error_prone_test_helpers",
+ "google_java_format",
"hamcrest-library",
"hamcrest",
"platform-test-annotations",
diff --git a/errorprone/java/com/google/errorprone/bugpatterns/android/HideInCommentsChecker.java b/errorprone/java/com/google/errorprone/bugpatterns/android/HideInCommentsChecker.java
new file mode 100644
index 0000000..07f1d4a
--- /dev/null
+++ b/errorprone/java/com/google/errorprone/bugpatterns/android/HideInCommentsChecker.java
@@ -0,0 +1,158 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * 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.google.errorprone.bugpatterns.android;
+
+import static com.google.errorprone.BugPattern.LinkType.NONE;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.ASTHelpers.getStartPosition;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.util.ASTHelpers;
+import com.google.errorprone.util.ErrorProneToken;
+import com.google.errorprone.util.ErrorProneTokens;
+import com.sun.source.tree.ClassTree;
+import com.sun.source.tree.CompilationUnitTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.NewClassTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.tree.VariableTree;
+import com.sun.tools.javac.parser.Tokens;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.lang.model.element.ElementKind;
+
+/**
+ * Bug checker to warn about {@code @hide} directives in comments.
+ *
+ * {@code @hide} tags are only meaningful inside of Javadoc comments. Errorprone has checks for
+ * standard Javadoc tags but doesn't know anything about {@code @hide} since it's an Android
+ * specific tag.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+ name = "AndroidHideInComments",
+ summary = "Warns when there are @hide declarations in comments rather than javadoc",
+ linkType = NONE,
+ severity = WARNING)
+public class HideInCommentsChecker extends BugChecker implements
+ BugChecker.CompilationUnitTreeMatcher {
+
+ @Override
+ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState state) {
+ final Map<Integer, Tree> javadocableTrees = findJavadocableTrees(tree);
+ final String sourceCode = state.getSourceCode().toString();
+ for (ErrorProneToken token : ErrorProneTokens.getTokens(sourceCode, state.context)) {
+ for (Tokens.Comment comment : token.comments()) {
+ if (!javadocableTrees.containsKey(token.pos())) {
+ continue;
+ }
+ generateFix(comment).ifPresent(fix -> {
+ final Tree javadocableTree = javadocableTrees.get(token.pos());
+ state.reportMatch(describeMatch(javadocableTree, fix));
+ });
+ }
+ }
+ // We might have multiple matches, so report them via VisitorState rather than the return
+ // value from the match function.
+ return NO_MATCH;
+ }
+
+ private static Optional<SuggestedFix> generateFix(Tokens.Comment comment) {
+ final String text = comment.getText();
+ if (text.startsWith("/**")) {
+ return Optional.empty();
+ }
+
+ if (!text.contains("@hide")) {
+ return Optional.empty();
+ }
+
+ if (text.startsWith("/*")) {
+ final int pos = comment.getSourcePos(1);
+ return Optional.of(SuggestedFix.replace(pos, pos, "*"));
+ } else if (text.startsWith("//")) {
+ final int endPos = comment.getSourcePos(text.length() - 1);
+ final char endChar = text.charAt(text.length() - 1);
+ String javadocClose = " */";
+ if (endChar != ' ') {
+ javadocClose = endChar + javadocClose;
+ }
+ final SuggestedFix fix = SuggestedFix.builder()
+ .replace(comment.getSourcePos(1), comment.getSourcePos(2), "**")
+ .replace(endPos, endPos + 1, javadocClose)
+ .build();
+ return Optional.of(fix);
+ }
+
+ return Optional.empty();
+ }
+
+
+ private Map<Integer, Tree> findJavadocableTrees(CompilationUnitTree tree) {
+ Map<Integer, Tree> javadoccableTrees = new HashMap<>();
+ new SuppressibleTreePathScanner<Void, Void>() {
+ @Override
+ public Void visitClass(ClassTree classTree, Void unused) {
+ javadoccableTrees.put(getStartPosition(classTree), classTree);
+ return super.visitClass(classTree, null);
+ }
+
+ @Override
+ public Void visitMethod(MethodTree methodTree, Void unused) {
+ // Generated constructors never have comments
+ if (!ASTHelpers.isGeneratedConstructor(methodTree)) {
+ javadoccableTrees.put(getStartPosition(methodTree), methodTree);
+ }
+ return super.visitMethod(methodTree, null);
+ }
+
+ @Override
+ public Void visitVariable(VariableTree variableTree, Void unused) {
+ ElementKind kind = getSymbol(variableTree).getKind();
+ if (kind == ElementKind.FIELD) {
+ javadoccableTrees.put(getStartPosition(variableTree), variableTree);
+ }
+ if (kind == ElementKind.ENUM_CONSTANT) {
+ javadoccableTrees.put(getStartPosition(variableTree), variableTree);
+ if (variableTree.getInitializer() instanceof NewClassTree) {
+ // Skip the generated class definition
+ ClassTree classBody =
+ ((NewClassTree) variableTree.getInitializer()).getClassBody();
+ if (classBody != null) {
+ scan(classBody.getMembers(), null);
+ }
+ return null;
+ }
+ }
+ return super.visitVariable(variableTree, null);
+ }
+
+ }.scan(tree, null);
+ return javadoccableTrees;
+ }
+
+}
diff --git a/errorprone/tests/java/com/google/errorprone/bugpatterns/android/HideInCommentsCheckerTest.java b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/HideInCommentsCheckerTest.java
new file mode 100644
index 0000000..f3e6c727
--- /dev/null
+++ b/errorprone/tests/java/com/google/errorprone/bugpatterns/android/HideInCommentsCheckerTest.java
@@ -0,0 +1,235 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * 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.google.errorprone.bugpatterns.android;
+
+import static com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode.TEXT_MATCH;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class HideInCommentsCheckerTest {
+ private static final String REFACTORING_FILE = "Test.java";
+
+ private BugCheckerRefactoringTestHelper mRefactoringHelper;
+ private CompilationTestHelper mCompilationHelper;
+
+ @Before
+ public void setUp() {
+ mRefactoringHelper = BugCheckerRefactoringTestHelper.newInstance(
+ HideInCommentsChecker.class, HideInCommentsCheckerTest.class);
+ mCompilationHelper = CompilationTestHelper.newInstance(
+ HideInCommentsChecker.class, HideInCommentsCheckerTest.class);
+ }
+
+
+ @Test
+ public void refactorSingleLineComment() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " // Foo @hide",
+ " void foo() {}",
+ "}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /** Foo @hide */",
+ " void foo() {}",
+ "}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void refactorSingleLineComment_doesntAddUnnecessarySpace() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " // Foo @hide ",
+ " void foo() {}",
+ "}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /** Foo @hide */",
+ " void foo() {}",
+ "}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void refactorSingleLineBlockComment() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /* Foo @hide */",
+ " void foo() {}",
+ "}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /** Foo @hide */",
+ " void foo() {}",
+ "}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void refactorMultiLineBlockComment() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /*",
+ " * Foo.",
+ " *",
+ " * @hide",
+ " */",
+ " void foo(int foo) {}",
+ "}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /**",
+ " * Foo.",
+ " *",
+ " * @hide",
+ " */",
+ " void foo(int foo) {}",
+ "}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void refactorFieldComment() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /* Foo @hide */",
+ " public int foo = 0;",
+ "}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /** Foo @hide */",
+ " public int foo = 0;",
+ "}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void refactorClassComment() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "/* Foo @hide */",
+ "public class Test {}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "/** Foo @hide */",
+ "public class Test {}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void refactorEnumComment() {
+ mRefactoringHelper
+ .addInputLines(
+ REFACTORING_FILE,
+ "public enum Test {",
+ " /* Foo @hide */",
+ " FOO",
+ "}")
+ .addOutputLines(
+ REFACTORING_FILE,
+ "public enum Test {",
+ " /** Foo @hide */",
+ " FOO",
+ "}")
+ .doTest(TEXT_MATCH);
+ }
+
+ @Test
+ public void canBeSuppressed() {
+ mCompilationHelper
+ .addSourceLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /* Foo @hide */",
+ " @SuppressWarnings(\"AndroidHideInComments\")",
+ " void foo() {}",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void isInJavadoc() {
+ mCompilationHelper
+ .addSourceLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /** Foo @hide */",
+ " void foo() {}",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void isInMultilineJavadoc() {
+ mCompilationHelper
+ .addSourceLines(
+ REFACTORING_FILE,
+ "public class Test {",
+ " /**",
+ " * Foo.",
+ " *",
+ " * @hide",
+ " */",
+ " void foo(int foo) {}",
+ "}")
+ .doTest();
+ }
+
+ @Test
+ public void noHidePresent() {
+ mCompilationHelper
+ .addSourceLines(
+ "test/" + REFACTORING_FILE,
+ "package test;",
+ "// Foo.",
+ "public class Test {",
+ " // Foo.",
+ " public int a;",
+ " /*",
+ " * Foo.",
+ " *",
+ " */",
+ " void foo(int foo) {}",
+ "}")
+ .doTest();
+ }
+
+}