Skylint: add lint for missing docstrings

RELNOTES: None
PiperOrigin-RevId: 166846007

2 files changed, 138 insertions, 0 deletions

diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java
new file mode 100644
index 0000000000..9dd7f82e08
--- /dev/null
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java
@@ -0,0 +1,71 @@
+// Copyright 2017 The Bazel Authors. 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.google.devtools.skylark.skylint;
+
+import com.google.devtools.build.lib.syntax.BuildFileAST;
+import com.google.devtools.build.lib.syntax.Expression;
+import com.google.devtools.build.lib.syntax.ExpressionStatement;
+import com.google.devtools.build.lib.syntax.FunctionDefStatement;
+import com.google.devtools.build.lib.syntax.Statement;
+import com.google.devtools.build.lib.syntax.StringLiteral;
+import com.google.devtools.build.lib.syntax.SyntaxTreeVisitor;
+import java.util.ArrayList;
+import java.util.List;
+
+/** Checks the existence of docstrings. */
+public class DocstringChecker extends SyntaxTreeVisitor {
+
+  private final List<Issue> issues = new ArrayList<>();
+
+  public static List<Issue> check(BuildFileAST ast) {
+    DocstringChecker checker = new DocstringChecker();
+    ast.accept(checker);
+    return checker.issues;
+  }
+
+  @Override
+  public void visit(BuildFileAST node) {
+    String moduleDocstring = extractDocstring(node.getStatements());
+    if (moduleDocstring == null) {
+      issues.add(new Issue("file has no module docstring", node.getLocation()));
+    }
+    super.visit(node);
+  }
+
+  @Override
+  public void visit(FunctionDefStatement node) {
+    String functionDocstring = extractDocstring(node.getStatements());
+    if (functionDocstring == null && !node.getIdentifier().getName().startsWith("_")) {
+      issues.add(
+          new Issue(
+              "function '" + node.getIdentifier().getName() + "' has no docstring",
+              node.getLocation()));
+    }
+  }
+
+  private static String extractDocstring(List<Statement> statements) {
+    if (statements.isEmpty()) {
+      return null;
+    }
+    Statement statement = statements.get(0);
+    if (statement instanceof ExpressionStatement) {
+      Expression expr = ((ExpressionStatement) statement).getExpression();
+      if (expr instanceof StringLiteral) {
+        return ((StringLiteral) expr).getValue();
+      }
+    }
+    return null;
+  }
+}
diff --git a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java
new file mode 100644
index 0000000000..92742e29ae
--- /dev/null
+++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java
@@ -0,0 +1,67 @@
+// Copyright 2017 The Bazel Authors. 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.google.devtools.skylark.skylint;
+
+import com.google.common.truth.Truth;
+import com.google.devtools.build.lib.syntax.BuildFileAST;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests the lint done by {@link DocstringChecker}. */
+@RunWith(JUnit4.class)
+public class DocstringCheckerTests {
+  private static List<Issue> findIssues(String... lines) {
+    String content = String.join("\n", lines);
+    BuildFileAST ast =
+        BuildFileAST.parseSkylarkString(
+            event -> {
+              throw new IllegalArgumentException(event.getMessage());
+            },
+            content);
+    return DocstringChecker.check(ast);
+  }
+
+  @Test
+  public void reportMissingDocString() throws Exception {
+    String errorMessage =
+        findIssues("# no module docstring", "def function():", "  pass # no function docstring")
+            .toString();
+    Truth.assertThat(errorMessage).contains(":2:1: file has no module docstring");
+    Truth.assertThat(errorMessage).contains(":2:1: function 'function' has no docstring");
+  }
+
+  @Test
+  public void dontReportExistingDocstrings() throws Exception {
+    Truth.assertThat(
+            findIssues(
+                "\"\"\" This is a module docstring",
+                "\n\"\"\"",
+                "def function():",
+                "  \"\"\" This is a function docstring\n\"\"\""))
+        .isEmpty();
+  }
+
+  @Test
+  public void dontReportPrivateFunctionWithoutDocstring() throws Exception {
+    Truth.assertThat(
+            findIssues(
+                "\"\"\" Module docstring\n\"\"\"",
+                "def _private_function():",
+                "  pass # no docstring necessary for private functions"))
+        .isEmpty();
+  }
+}