Skylint: docstring format: warn about empty sections and descriptions.

In the process, I refactored and extended the line handling and error reporting a little.

RELNOTES: none
PiperOrigin-RevId: 172760285

2 files changed, 57 insertions, 13 deletions

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
index 2c68157313..4d556543a6 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringUtils.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/DocstringUtils.java
@@ -232,12 +232,12 @@ public final class DocstringUtils {
     private boolean blankLineBefore = false;
     /** Whether we've seen a special section, e.g. 'Args:', already. */
     private boolean specialSectionsStarted = false;
-    /** The complete current line in the docstring, including all indentation. */
-    private String originalLine = "";
+    /** List of all parsed lines in the docstring so far, including all indentation. */
+    private ArrayList<String> originalLines = new ArrayList<>();
     /**
      * The current line in the docstring with the baseline indentation removed.
      *
-     * <p>If the indentation of {@link #originalLine} is less than the expected {@link
+     * <p>If the indentation of a docstring line is less than the expected {@link
      * #baselineIndentation}, only the existing indentation is stripped; none of the remaining
      * characters are cut off.
      */
@@ -257,22 +257,23 @@ public final class DocstringUtils {
      *
      * @return whether there are lines remaining to be parsed
      */
-    boolean nextLine() {
+    private boolean nextLine() {
       if (startOfLineOffset >= docstring.length()) {
         return false;
       }
       blankLineBefore = line.isEmpty();
-      lineNumber++;
       startOfLineOffset = endOfLineOffset + 1;
       if (startOfLineOffset >= docstring.length()) {
         line = "";
         return false;
       }
+      lineNumber++;
       endOfLineOffset = docstring.indexOf('\n', startOfLineOffset);
       if (endOfLineOffset < 0) {
         endOfLineOffset = docstring.length();
       }
-      originalLine = docstring.substring(startOfLineOffset, endOfLineOffset);
+      String originalLine = docstring.substring(startOfLineOffset, endOfLineOffset);
+      originalLines.add(originalLine);
       line = originalLine;
       int indentation = getIndentation(line);
       if (!line.isEmpty()) {
@@ -304,8 +305,12 @@ public final class DocstringUtils {
       return index;
     }
 
-    void error(String message) {
-      errors.add(new DocstringParseError(message, lineNumber, originalLine));
+    private void error(String message) {
+      error(this.lineNumber, message);
+    }
+
+    private void error(int lineNumber, String message) {
+      errors.add(new DocstringParseError(message, lineNumber, originalLines.get(lineNumber - 1)));
     }
 
     DocstringInfo parse() {
@@ -389,6 +394,7 @@ public final class DocstringUtils {
     private static final Pattern attributesSeparator = Pattern.compile("\\s*,\\s*");
 
     private List<ParameterDoc> parseParameters() {
+      int sectionLineNumber = lineNumber;
       nextLine();
       List<ParameterDoc> params = new ArrayList<>();
       int expectedParamLineIndentation = -1;
@@ -416,6 +422,7 @@ public final class DocstringUtils {
                   + actualIndentation
                   + " spaces)");
         }
+        int paramLineNumber = lineNumber;
         trimmedLine = line.substring(actualIndentation);
         Matcher matcher = paramLineMatcher.matcher(trimmedLine);
         if (!matcher.matches()) {
@@ -431,7 +438,14 @@ public final class DocstringUtils {
                 ? Collections.emptyList()
                 : Arrays.asList(attributesSeparator.split(attributesString));
         parseContinuedParamDescription(actualIndentation, description);
-        params.add(new ParameterDoc(parameterName, attributes, description.toString().trim()));
+        String parameterDescription = description.toString().trim();
+        if (parameterDescription.isEmpty()) {
+          error(paramLineNumber, "empty parameter description for '" + parameterName + "'");
+        }
+        params.add(new ParameterDoc(parameterName, attributes, parameterDescription));
+      }
+      if (params.isEmpty()) {
+        error(sectionLineNumber, "section is empty");
       }
       return params;
     }
@@ -454,8 +468,9 @@ public final class DocstringUtils {
     }
 
     private String parseSectionAfterHeading() {
+      int sectionLineNumber = lineNumber;
       nextLine();
-      StringBuilder returns = new StringBuilder();
+      StringBuilder contents = new StringBuilder();
       boolean firstLine = true;
       while (!eof()) {
         String trimmedLine;
@@ -477,13 +492,17 @@ public final class DocstringUtils {
           }
         }
         if (!firstLine) {
-          returns.append('\n');
+          contents.append('\n');
         }
-        returns.append(trimmedLine);
+        contents.append(trimmedLine);
         nextLine();
         firstLine = false;
       }
-      return returns.toString().trim();
+      String result = contents.toString().trim();
+      if (result.isEmpty()) {
+        error(sectionLineNumber, "section is empty");
+      }
+      return result;
     }
   }
 
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
index 27d2257226..56c5e2b073 100644
--- a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringUtilsTest.java
+++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/DocstringUtilsTest.java
@@ -135,6 +135,31 @@ public class DocstringUtilsTest {
   }
 
   @Test
+  public void emptySection() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringUtils.parseDocstring(
+        "summary\n" + "\n" + "Args:\n" + "More description.\n", 0, errors);
+    Truth.assertThat(errors.toString()).contains("3: section is empty");
+
+    errors = new ArrayList<>();
+    DocstringUtils.parseDocstring(
+        "summary\n" + "\n" + "Returns:\n" + "More description\n", 0, errors);
+    Truth.assertThat(errors.toString()).contains("3: section is empty");
+
+    errors = new ArrayList<>();
+    DocstringUtils.parseDocstring(
+        "summary\n" + "\n" + "Deprecated:\n" + "More description\n", 0, errors);
+    Truth.assertThat(errors.toString()).contains("3: section is empty");
+  }
+
+  @Test
+  public void emptyParamDescription() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringUtils.parseDocstring("summary\n" + "\n" + "Args:\n" + "" + "  foo: \n\n", 0, errors);
+    Truth.assertThat(errors.toString()).contains("4: empty parameter description for 'foo'");
+  }
+
+  @Test
   public void docstringReturn() throws Exception {
     List<DocstringParseError> errors = new ArrayList<>();
     DocstringInfo info =