Skylint: check the documentation of parameters in function docstrings

RELNOTES: none
PiperOrigin-RevId: 167502103

7 files changed, 702 insertions, 25 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
index 9dd7f82e08..53e0e8d621 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringChecker.java
@@ -14,19 +14,24 @@
 
 package com.google.devtools.skylark.skylint;
 
+import static com.google.devtools.skylark.skylint.DocstringUtils.extractDocstring;
+import static com.google.devtools.skylark.skylint.DocstringUtils.parseDocString;
+
 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.Parameter;
 import com.google.devtools.build.lib.syntax.StringLiteral;
 import com.google.devtools.build.lib.syntax.SyntaxTreeVisitor;
+import com.google.devtools.skylark.skylint.DocstringUtils.DocstringInfo;
+import com.google.devtools.skylark.skylint.DocstringUtils.DocstringParseError;
+import com.google.devtools.skylark.skylint.DocstringUtils.ParameterDoc;
 import java.util.ArrayList;
+import java.util.LinkedHashSet;
 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) {
@@ -37,35 +42,99 @@ public class DocstringChecker extends SyntaxTreeVisitor {
 
   @Override
   public void visit(BuildFileAST node) {
-    String moduleDocstring = extractDocstring(node.getStatements());
+    StringLiteral moduleDocstring = extractDocstring(node.getStatements());
     if (moduleDocstring == null) {
       issues.add(new Issue("file has no module docstring", node.getLocation()));
+    } else {
+      List<DocstringParseError> errors = new ArrayList<>();
+      parseDocString(moduleDocstring.getValue(), errors);
+      for (DocstringParseError error : errors) {
+        LinterLocation loc =
+            new LinterLocation(
+                moduleDocstring.getLocation().getStartLine() + error.lineNumber - 1, 1);
+        issues.add(new Issue("invalid docstring format: " + error.message, loc));
+      }
     }
     super.visit(node);
   }
 
   @Override
   public void visit(FunctionDefStatement node) {
-    String functionDocstring = extractDocstring(node.getStatements());
+    StringLiteral 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;
+    if (functionDocstring == null) {
+      return;
+    }
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info = parseDocString(functionDocstring.getValue(), errors);
+    for (DocstringParseError error : errors) {
+      LinterLocation loc =
+          new LinterLocation(
+              functionDocstring.getLocation().getStartLine() + error.lineNumber - 1, 1);
+      issues.add(new Issue("invalid docstring format: " + error.message, loc));
     }
-    Statement statement = statements.get(0);
-    if (statement instanceof ExpressionStatement) {
-      Expression expr = ((ExpressionStatement) statement).getExpression();
-      if (expr instanceof StringLiteral) {
-        return ((StringLiteral) expr).getValue();
+    List<String> documentedParams = new ArrayList<>();
+    for (ParameterDoc param : info.parameters) {
+      documentedParams.add(param.parameterName);
+    }
+    List<String> declaredParams = new ArrayList<>();
+    for (Parameter<Expression, Expression> param : node.getParameters()) {
+      if (param.getName() != null) {
+        String name = param.getName();
+        if (param.isStar()) {
+          name = "*" + name;
+        }
+        if (param.isStarStar()) {
+          name = "**" + name;
+        }
+        declaredParams.add(name);
       }
     }
-    return null;
+    if (documentedParams.isEmpty() && !declaredParams.isEmpty()) {
+      // One-line docstrings don't have to specify parameters:
+      if (!info.longDescription.isEmpty()) {
+        issues.add(
+            new Issue(
+                "incomplete docstring: the function parameters are not documented",
+                functionDocstring.getLocation()));
+      }
+      return;
+    }
+    for (String param : declaredParams) {
+      if (!documentedParams.contains(param)) {
+        issues.add(
+            new Issue(
+                "incomplete docstring: parameter '" + param + "' not documented",
+                functionDocstring.getLocation()));
+      }
+    }
+    for (String param : documentedParams) {
+      if (!declaredParams.contains(param)) {
+        issues.add(
+            new Issue(
+                "inconsistent docstring: parameter '"
+                    + param
+                    + "' appears in docstring but not in function signature",
+                functionDocstring.getLocation()));
+      }
+    }
+    if (new LinkedHashSet<>(declaredParams).equals(new LinkedHashSet<>(documentedParams))
+        && !declaredParams.equals(documentedParams)) {
+      String message =
+          "inconsistent docstring: order of parameters differs from function signature\n"
+              + "Declaration order:   "
+              + String.join(", ", declaredParams)
+              + "\n"
+              + "Documentation order: "
+              + String.join(", ", documentedParams)
+              + "\n";
+      issues.add(new Issue(message, functionDocstring.getLocation()));
+    }
   }
+
 }
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringUtils.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringUtils.java
new file mode 100644
index 0000000000..747aec2ca8
--- /dev/null
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringUtils.java
@@ -0,0 +1,258 @@
+// 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.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.syntax.Expression;
+import com.google.devtools.build.lib.syntax.ExpressionStatement;
+import com.google.devtools.build.lib.syntax.Statement;
+import com.google.devtools.build.lib.syntax.StringLiteral;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+
+/** Utilities to extract and parse docstrings. */
+public final class DocstringUtils {
+  private DocstringUtils() {}
+
+  @Nullable
+  static StringLiteral 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;
+      }
+    }
+    return null;
+  }
+
+  /**
+   * Parses a docstring.
+   *
+   * <p>The format of the docstring is as follows
+   *
+   * <pre>{@code
+   * """One-line summary: must be followed and may be preceded by a blank line.
+   *
+   * Optional additional description like this.
+   *
+   * If it's a function docstring and the function has more than one argument, the docstring has
+   * to document these parameters as follows:
+   *
+   * Args:
+   *   parameter1: description of the first parameter
+   *   parameter2: description of the second
+   *     parameter that spans two lines. Each additional line
+   *     must be indented by (at least) two spaces
+   *   another_parameter (unused, mutable): a parameter may be followed
+   *     by additional attributes in parentheses
+   * """
+   * }</pre>
+   *
+   * @param docstring a docstring of the format described above
+   * @param parseErrors a list to which parsing error messages are written
+   * @return the parsed docstring information
+   */
+  static DocstringInfo parseDocString(String docstring, List<DocstringParseError> parseErrors) {
+    DocstringParser parser = new DocstringParser(docstring);
+    DocstringInfo result = parser.parse();
+    parseErrors.addAll(parser.errors);
+    return result;
+  }
+
+  static class DocstringInfo {
+    final String summary;
+    final List<ParameterDoc> parameters;
+    final String longDescription;
+
+    public DocstringInfo(String summary, List<ParameterDoc> parameters, String longDescription) {
+      this.summary = summary;
+      this.parameters = ImmutableList.copyOf(parameters);
+      this.longDescription = longDescription;
+    }
+  }
+
+  static class ParameterDoc {
+    final String parameterName;
+    final List<String> attributes; // e.g. a type annotation, "unused", "mutable"
+    final String description;
+
+    public ParameterDoc(String parameterName, List<String> attributes, String description) {
+      this.parameterName = parameterName;
+      this.attributes = ImmutableList.copyOf(attributes);
+      this.description = description;
+    }
+  }
+
+  private static class DocstringParser {
+    private final String docstring;
+    private int startOfLineOffset = 0;
+    private int endOfLineOffset = -1;
+    private int lineNumber = 0;
+    private int expectedIndentation = 0;
+    private String line = "";
+    private final List<DocstringParseError> errors = new ArrayList<>();
+
+    DocstringParser(String docstring) {
+      this.docstring = docstring;
+      nextLine();
+    }
+
+    boolean nextLine() {
+      if (startOfLineOffset >= docstring.length()) {
+        return false;
+      }
+      lineNumber++;
+      startOfLineOffset = endOfLineOffset + 1;
+      if (startOfLineOffset >= docstring.length()) {
+        line = "";
+        return false;
+      }
+      endOfLineOffset = docstring.indexOf('\n', startOfLineOffset);
+      if (endOfLineOffset < 0) {
+        endOfLineOffset = docstring.length();
+      }
+      line = docstring.substring(startOfLineOffset, endOfLineOffset);
+      int indentation = getIndentation(line);
+      if (!line.isEmpty()) {
+        if (indentation < expectedIndentation) {
+          error(
+              "line indented too little (here: "
+                  + indentation
+                  + " spaces; before: "
+                  + expectedIndentation
+                  + " spaces)");
+          expectedIndentation = indentation;
+        }
+        startOfLineOffset += expectedIndentation;
+      }
+      line = docstring.substring(startOfLineOffset, endOfLineOffset);
+      return true;
+    }
+
+    private static int getIndentation(String line) {
+      int index = 0;
+      while (index < line.length() && line.charAt(index) == ' ') {
+        index++;
+      }
+      return index;
+    }
+
+    void error(String message) {
+      errors.add(new DocstringParseError(message, lineNumber));
+    }
+
+    DocstringInfo parse() {
+      String summary = line;
+      if (!nextLine()) {
+        return new DocstringInfo(summary, Collections.emptyList(), "");
+      }
+      if (!line.isEmpty()) {
+        error("the one-line summary should be followed by a blank line");
+      } else {
+        nextLine();
+      }
+      expectedIndentation = getIndentation(line);
+      line = line.substring(expectedIndentation);
+      List<String> longDescriptionLines = new ArrayList<>();
+      List<ParameterDoc> params = new ArrayList<>();
+      boolean sectionStart = true;
+      do {
+        if (line.startsWith(" ")) {
+          error(
+              "line indented too much (here: "
+                  + (expectedIndentation + getIndentation(line))
+                  + " spaces; expected: "
+                  + expectedIndentation
+                  + " spaces)");
+        }
+        if (line.equals("Args:")) {
+          if (!sectionStart) {
+            error("section should be preceded by a blank line");
+          }
+          if (!params.isEmpty()) {
+            error("parameters were already documented before");
+          }
+          params.addAll(parseParameters());
+        } else {
+          longDescriptionLines.add(line);
+        }
+        sectionStart = line.isEmpty();
+      } while (nextLine());
+      return new DocstringInfo(summary, params, String.join("\n", longDescriptionLines));
+    }
+
+    private static final Pattern paramLineMatcher =
+        Pattern.compile(
+            "\\s*(?<name>[*\\w]+)( \\(\\s*(?<attributes>.*)\\s*\\))?: (?<description>.*)");
+
+    private static final Pattern attributesSeparator = Pattern.compile("\\s*,\\s*");
+
+    private List<ParameterDoc> parseParameters() {
+      nextLine();
+      List<ParameterDoc> params = new ArrayList<>();
+      while (!line.isEmpty()) {
+        if (getIndentation(line) != 2) {
+          error("parameter lines have to be indented by two spaces");
+        } else {
+          line = line.substring(2);
+        }
+        Matcher matcher = paramLineMatcher.matcher(line);
+        if (!matcher.matches()) {
+          error("invalid parameter documentation");
+          nextLine();
+          continue;
+        }
+        String parameterName = Preconditions.checkNotNull(matcher.group("name"));
+        String attributesString = matcher.group("attributes");
+        StringBuilder description = new StringBuilder(matcher.group("description"));
+        List<String> attributes =
+            attributesString == null
+                ? Collections.emptyList()
+                : Arrays.asList(attributesSeparator.split(attributesString));
+        while (nextLine() && getIndentation(line) > 2) {
+          description.append('\n');
+          description.append(line, getIndentation(line), line.length());
+        }
+        params.add(new ParameterDoc(parameterName, attributes, description.toString()));
+      }
+      return params;
+    }
+  }
+
+  static class DocstringParseError {
+    final String message;
+    final int lineNumber;
+
+    public DocstringParseError(String message, int lineNumber) {
+      this.message = message;
+      this.lineNumber = lineNumber;
+    }
+
+    @Override
+    public String toString() {
+      return ":" + lineNumber + ": " + message;
+    }
+  }
+}
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Issue.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Issue.java
index 975798a9cc..5fe186ad9c 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Issue.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Issue.java
@@ -20,25 +20,29 @@ import com.google.devtools.build.lib.events.Location;
 public class Issue {
   // TODO(skylark-team): Represent issues more efficiently than just by a string
   public final String message;
-  public final Location location;
+  public final LinterLocation location;
 
-  public Issue(String message, Location location) {
+  public Issue(String message, LinterLocation location) {
     this.message = message;
     this.location = location;
   }
 
+  public Issue(String message, Location location) {
+    this(message, LinterLocation.from(location));
+  }
+
   @Override
   public String toString() {
     return location + ": " + message;
   }
 
   public static int compare(Issue i1, Issue i2) {
-    Location l1 = i1.location;
-    Location l2 = i2.location;
-    int pathComparison = l1.getPath().compareTo(l2.getPath());
-    if (pathComparison != 0) {
-      return pathComparison;
+    LinterLocation l1 = i1.location;
+    LinterLocation l2 = i2.location;
+    int lineComparison = l1.line - l2.line;
+    if (lineComparison != 0) {
+      return lineComparison;
     }
-    return l1.getStartOffset() - l2.getStartOffset();
+    return l1.column - l2.column;
   }
 }
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/LinterLocation.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/LinterLocation.java
new file mode 100644
index 0000000000..6f97b59dd2
--- /dev/null
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/LinterLocation.java
@@ -0,0 +1,39 @@
+// 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.events.Location;
+import com.google.devtools.build.lib.events.Location.LineAndColumn;
+
+/** Location of a linter warning. */
+public class LinterLocation {
+  public final int line;
+  public final int column;
+
+  public LinterLocation(int line, int column) {
+    this.line = line;
+    this.column = column;
+  }
+
+  public static LinterLocation from(Location location) {
+    LineAndColumn lac = location.getStartLineAndColumn();
+    return new LinterLocation(lac.getLine(), lac.getColumn());
+  }
+
+  @Override
+  public String toString() {
+    return ":" + line + ":" + column;
+  }
+}
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java
index 428c6b4d4e..e6f3dba8e4 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Skylint.java
@@ -39,6 +39,7 @@ public class Skylint {
     issues.addAll(ControlFlowChecker.check(ast));
     issues.addAll(StatementWithoutEffectChecker.check(ast));
     issues.addAll(UsageChecker.check(ast));
+    issues.addAll(DocstringChecker.check(ast));
     issues.sort(Issue::compare);
     if (!issues.isEmpty()) {
       System.out.println(path);
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
index 84b4e6b9b2..5642f1c76a 100644
--- a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java
+++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringCheckerTests.java
@@ -45,10 +45,111 @@ public class DocstringCheckerTests {
   }
 
   @Test
+  public void reportMissingParameterDocumentation() throws Exception {
+    List<Issue> errors =
+        findIssues(
+            "\"\"\" module docstring \"\"\"",
+            "def f(param1, param2):",
+            "  \"\"\"summary",
+            "",
+            "more description",
+            "\"\"\"",
+            "  pass");
+    Truth.assertThat(errors).hasSize(1);
+    Truth.assertThat(errors.toString())
+        .contains(":3:3: incomplete docstring: the function parameters are not documented");
+  }
+
+  @Test
+  public void reportUndocumentedParameters() throws Exception {
+    String errorMessage =
+        findIssues(
+                "def function(foo, bar, baz):",
+                "  \"\"\"summary",
+                "",
+                "  Args:",
+                "    bar: blabla",
+                "  \"\"\"",
+                "  pass")
+            .toString();
+    Truth.assertThat(errorMessage)
+        .contains(":2:3: incomplete docstring: parameter 'foo' not documented");
+    Truth.assertThat(errorMessage)
+        .contains(":2:3: incomplete docstring: parameter 'baz' not documented");
+  }
+
+  @Test
+  public void reportObsoleteParameterDocumentation() throws Exception {
+    String errorMessage =
+        findIssues(
+                "def function(bar):",
+                "  \"\"\"summary",
+                "",
+                "  Args:",
+                "    foo: blabla",
+                "    bar: blabla",
+                "    baz: blabla",
+                "  \"\"\"",
+                "  pass")
+            .toString();
+    Truth.assertThat(errorMessage)
+        .contains(
+            ":2:3: inconsistent docstring: parameter 'foo' appears in docstring"
+                + " but not in function signature");
+    Truth.assertThat(errorMessage)
+        .contains(
+            ":2:3: inconsistent docstring: parameter 'baz' appears in docstring"
+                + " but not in function signature");
+  }
+
+  @Test
+  public void reportParametersDocumentedInDifferentOrder() throws Exception {
+    String errorMessage =
+        findIssues(
+                "def function(p1, p2):",
+                "  \"\"\"summary",
+                "",
+                "  Args:",
+                "    p2: blabla",
+                "    p1: blabla",
+                "  \"\"\"",
+                "  pass")
+            .toString();
+    Truth.assertThat(errorMessage)
+        .contains(
+            ":2:3: inconsistent docstring: order of parameters differs from function signature\n"
+                + "Declaration order:   p1, p2\n"
+                + "Documentation order: p2, p1");
+  }
+
+  @Test
+  public void reportInvalidDocstringFormat() throws Exception {
+    String errorMessage = findIssues("\"\"\"summary", "missing blank line\"\"\"").toString();
+    Truth.assertThat(errorMessage)
+        .contains(
+            ":2:1: invalid docstring format: "
+                + "the one-line summary should be followed by a blank line");
+
+    errorMessage =
+        findIssues(
+                "def f():",
+                "  \"\"\"summary",
+                "",
+                "  foo",
+                " bad indentation in this line",
+                "\"\"\"")
+            .toString();
+    Truth.assertThat(errorMessage)
+        .contains(
+            ":5:1: invalid docstring format: "
+                + "line indented too little (here: 1 spaces; before: 2 spaces)");
+  }
+
+  @Test
   public void dontReportExistingDocstrings() throws Exception {
     Truth.assertThat(
             findIssues(
-                "\"\"\" This is a module docstring",
+                "\"\"\"This is a module docstring",
                 "\n\"\"\"",
                 "def function():",
                 "  \"\"\" This is a function docstring\n\"\"\""))
@@ -56,6 +157,16 @@ public class DocstringCheckerTests {
   }
 
   @Test
+  public void dontReportSummaryDocstringWithoutParameters() throws Exception {
+    Truth.assertThat(
+            findIssues(
+                "\"\"\"module docstring\"\"\"",
+                "def function(param1, param2):",
+                "  \"\"\"summary without parameter docs is fine\"\"\""))
+        .isEmpty();
+  }
+
+  @Test
   public void dontReportPrivateFunctionWithoutDocstring() throws Exception {
     Truth.assertThat(
             findIssues(
@@ -64,4 +175,22 @@ public class DocstringCheckerTests {
                 "  pass # no docstring necessary for private functions"))
         .isEmpty();
   }
+
+  @Test
+  public void dontReportFunctionDocstringWithCorrectParameters() throws Exception {
+    Truth.assertThat(
+            findIssues(
+                "\"\"\" module docstring \"\"\"",
+                "def function(param1, param2, *args, **kwargs):",
+                "  \"\"\"summary",
+                "",
+                "  Args:",
+                "    param1: foo",
+                "    param2 (foo, bar): baz",
+                "    *args: foo",
+                "    **kwargs: bar",
+                "  \"\"\"",
+                "  pass"))
+        .isEmpty();
+  }
 }
diff --git a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringUtilsTest.java b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringUtilsTest.java
new file mode 100644
index 0000000000..d8fdb86cff
--- /dev/null
+++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringUtilsTest.java
@@ -0,0 +1,177 @@
+// 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.skylark.skylint.DocstringUtils.DocstringInfo;
+import com.google.devtools.skylark.skylint.DocstringUtils.DocstringParseError;
+import com.google.devtools.skylark.skylint.DocstringUtils.ParameterDoc;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests the {@link DocstringUtils} class. */
+@RunWith(JUnit4.class)
+public class DocstringUtilsTest {
+  @Test
+  public void oneLineDocstring() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info = DocstringUtils.parseDocString("summary", errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).isEmpty();
+    Truth.assertThat(info.longDescription).isEmpty();
+    Truth.assertThat(errors).isEmpty();
+  }
+
+  @Test
+  public void missingBlankLineAfterSummary() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info = DocstringUtils.parseDocString("summary\nfoo", errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).isEmpty();
+    Truth.assertThat(info.longDescription).isEqualTo("foo");
+    Truth.assertThat(errors.toString())
+        .contains(":2: the one-line summary should be followed by a blank line");
+  }
+
+  @Test
+  public void multiParagraphDocstring() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info = DocstringUtils.parseDocString("summary\n\nfoo\n\nbar", errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).isEmpty();
+    Truth.assertThat(info.longDescription).isEqualTo("foo\n\nbar");
+    Truth.assertThat(errors).isEmpty();
+  }
+
+  @Test
+  public void inconsistentIndentation() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info =
+        DocstringUtils.parseDocString("summary\n" + "\n" + "  foo\n" + "bar", errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).isEmpty();
+    Truth.assertThat(info.longDescription).isEqualTo("foo\nbar");
+    Truth.assertThat(errors.toString())
+        .contains(":4: line indented too little (here: 0 spaces; before: 2 spaces)");
+
+    errors = new ArrayList<>();
+    info = DocstringUtils.parseDocString("summary\n" + "\n" + "  foo\n" + "    bar\n", errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).isEmpty();
+    Truth.assertThat(info.longDescription).isEqualTo("foo\n  bar");
+    Truth.assertThat(errors.toString())
+        .contains(":4: line indented too much (here: 4 spaces; expected: 2 spaces)");
+  }
+
+  @Test
+  public void inconsistentIndentationInParameters() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info =
+        DocstringUtils.parseDocString("summary\n" + "\n" + "  Args:\n" + "  param1: foo\n", errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).hasSize(1);
+    Truth.assertThat(errors.toString())
+        .contains(":4: parameter lines have to be indented by two spaces");
+  }
+
+  @Test
+  public void docstringParameters() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info =
+        DocstringUtils.parseDocString(
+            "summary\n"
+                + "\n"
+                + "  Args:\n"
+                + "    param1: multi-\n"
+                + "      line\n"
+                + "    param2 (mutable, unused): bar\n"
+                + "    *args: args\n"
+                + "    **kwargs: kwargs\n"
+                + "\n"
+                + "  description",
+            errors);
+    Truth.assertThat(info.summary).isEqualTo("summary");
+    Truth.assertThat(info.parameters).hasSize(4);
+    Truth.assertThat(info.longDescription).isEqualTo("description");
+    ParameterDoc firstParam = info.parameters.get(0);
+    ParameterDoc secondParam = info.parameters.get(1);
+    ParameterDoc thirdParam = info.parameters.get(2);
+    ParameterDoc fourthParam = info.parameters.get(3);
+
+    Truth.assertThat(firstParam.parameterName).isEqualTo("param1");
+    Truth.assertThat(firstParam.attributes).isEmpty();
+    Truth.assertThat(firstParam.description).isEqualTo("multi-\nline");
+
+    Truth.assertThat(secondParam.parameterName).isEqualTo("param2");
+    Truth.assertThat(secondParam.attributes).isEqualTo(Arrays.asList("mutable", "unused"));
+    Truth.assertThat(secondParam.description).isEqualTo("bar");
+
+    Truth.assertThat(thirdParam.parameterName).isEqualTo("*args");
+    Truth.assertThat(thirdParam.attributes).isEmpty();
+    Truth.assertThat(thirdParam.description).isEqualTo("args");
+
+    Truth.assertThat(fourthParam.parameterName).isEqualTo("**kwargs");
+    Truth.assertThat(fourthParam.attributes).isEmpty();
+    Truth.assertThat(fourthParam.description).isEqualTo("kwargs");
+
+    Truth.assertThat(errors).isEmpty();
+  }
+
+  @Test
+  public void argsSectionNotPrecededByNewline() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringUtils.parseDocString("summary\n" + "\n" + "  foo\n" + "  Args:", errors);
+    Truth.assertThat(errors.toString()).contains(":4: section should be preceded by a blank line");
+  }
+
+  @Test
+  public void twoArgsSectionsError() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info =
+        DocstringUtils.parseDocString(
+            "summary\n"
+                + "\n"
+                + "  Args:\n"
+                + "    param1: foo\n"
+                + "\n"
+                + "  Args:\n"
+                + "    param2: bar\n"
+                + "\n"
+                + "  description",
+            errors);
+    Truth.assertThat(info.parameters).hasSize(2);
+    Truth.assertThat(errors.toString()).contains(":6: parameters were already documented before");
+  }
+
+  @Test
+  public void invalidParameterDoc() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringInfo info =
+        DocstringUtils.parseDocString(
+            "summary\n"
+                + "\n"
+                + "  Args:\n"
+                + "    invalid parameter doc\n"
+                + "\n"
+                + "  description",
+            errors);
+    Truth.assertThat(info.parameters).isEmpty();
+    Truth.assertThat(errors.toString()).contains(":4: invalid parameter documentation");
+  }
+}