Skylint: be less strict about indentation in docstrings

Instead of enforcing two spaces of indentation, we now
only enforce a positive number of spaces in indented lines.

Reason: Apparently, a lot of docstrings in Skylark files
are badly indented, so we should only warn about the
worst offenders until a formatting tool is available.

RELNOTES: none
PiperOrigin-RevId: 169429429

2 files changed, 35 insertions, 37 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 073bf44568..9cc0cacd9b 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
@@ -61,15 +61,17 @@ public final class DocstringUtils {
    * to document these parameters as follows:
    *
    * Args:
-   *   parameter1: description of the first parameter
+   *   parameter1: description of the first parameter. Each parameter line
+   *     should be indented by one, preferably two, spaces (as here).
    *   parameter2: description of the second
-   *     parameter that spans two lines. Each additional line
-   *     must be indented by (at least) two spaces
+   *     parameter that spans two lines. Each additional line should have a
+   *     hanging indentation of at least one, preferably two, additional spaces (as here).
    *   another_parameter (unused, mutable): a parameter may be followed
    *     by additional attributes in parentheses
    *
    * Returns:
    *   Description of the return value.
+   *   Should be indented by at least one, preferably two spaces (as here)
    *   Can span multiple lines.
    * """
    * }</pre>
@@ -238,26 +240,32 @@ public final class DocstringUtils {
     private List<ParameterDoc> parseParameters() {
       nextLine();
       List<ParameterDoc> params = new ArrayList<>();
+      int expectedParamLineIndentation = -1;
       while (!eof()) {
         if (line.isEmpty()) {
           nextLine();
           continue;
         }
-        if (getIndentation(line) == 0) {
+        int actualIndentation = getIndentation(line);
+        if (actualIndentation == 0) {
           if (!blankLineBefore) {
             error("end of 'Args' section without blank line");
           }
           break;
         }
         String trimmedLine;
-        if (getIndentation(line) < 2) {
+        if (expectedParamLineIndentation == -1) {
+          expectedParamLineIndentation = actualIndentation;
+        }
+        if (expectedParamLineIndentation != actualIndentation) {
           error(
-              "parameter lines have to be indented by two spaces"
-                  + " (relative to the left margin of the docstring)");
-          trimmedLine = line.substring(getIndentation(line));
-        } else {
-          trimmedLine = line.substring(2);
+              "inconsistent indentation of parameter lines (before: "
+                  + expectedParamLineIndentation
+                  + "; here: "
+                  + actualIndentation
+                  + " spaces)");
         }
+        trimmedLine = line.substring(actualIndentation);
         Matcher matcher = paramLineMatcher.matcher(trimmedLine);
         if (!matcher.matches()) {
           error("invalid parameter documentation");
@@ -271,31 +279,24 @@ public final class DocstringUtils {
             attributesString == null
                 ? Collections.emptyList()
                 : Arrays.asList(attributesSeparator.split(attributesString));
-        parseContinuedParamDescription(description);
+        parseContinuedParamDescription(actualIndentation, description);
         params.add(new ParameterDoc(parameterName, attributes, description.toString().trim()));
       }
       return params;
     }
 
     /** Parses additional lines that can come after "param: foo" in an 'Args' section. */
-    private void parseContinuedParamDescription(StringBuilder description) {
+    private void parseContinuedParamDescription(
+        int baselineIndentation, StringBuilder description) {
       while (nextLine()) {
         if (line.isEmpty()) {
           description.append('\n');
           continue;
         }
-        if (getIndentation(line) <= 2) {
+        if (getIndentation(line) <= baselineIndentation) {
           break;
         }
-        String trimmedLine;
-        if (getIndentation(line) < 4) {
-          error(
-              "continued parameter lines have to be indented by four spaces"
-                  + " (relative to the left margin of the docstring)");
-          trimmedLine = line.substring(getIndentation(line));
-        } else {
-          trimmedLine = line.substring(4);
-        }
+        String trimmedLine = line.substring(baselineIndentation);
         description.append('\n');
         description.append(trimmedLine);
       }
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 eeddd0f491..78b3e629cb 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
@@ -116,25 +116,22 @@ public class DocstringUtilsTest {
             "summary\n"
                 + "\n"
                 + "Args:\n"
+                + "  param0: two spaces indentation\n"
                 + " param1: only one space indentation\n"
-                + "   only three spaces indentation\n"
+                + "  only two spaces indentation in continued line\n"
                 + "unindented line after",
             0,
             errors);
     Truth.assertThat(info.summary).isEqualTo("summary");
-    Truth.assertThat(info.parameters).hasSize(1);
-    ParameterDoc param = info.parameters.get(0);
-    Truth.assertThat(param.description)
-        .isEqualTo("only one space indentation\nonly three spaces indentation");
-    Truth.assertThat(errors.toString())
-        .contains(
-            ":4: parameter lines have to be indented by two spaces"
-                + " (relative to the left margin of the docstring)");
+    Truth.assertThat(info.parameters).hasSize(2);
+    ParameterDoc param0 = info.parameters.get(0);
+    Truth.assertThat(param0.description).isEqualTo("two spaces indentation");
+    ParameterDoc param1 = info.parameters.get(1);
+    Truth.assertThat(param1.description)
+        .isEqualTo("only one space indentation\n only two spaces indentation in continued line");
     Truth.assertThat(errors.toString())
-        .contains(
-            ":5: continued parameter lines have to be indented by four spaces"
-                + " (relative to the left margin of the docstring)");
-    Truth.assertThat(errors.toString()).contains(":6: end of 'Args' section without blank line");
+        .contains(":5: inconsistent indentation of parameter lines (before: 2; here: 1 spaces)");
+    Truth.assertThat(errors.toString()).contains(":7: end of 'Args' section without blank line");
   }
 
   @Test
@@ -185,7 +182,7 @@ public class DocstringUtilsTest {
 
     Truth.assertThat(firstParam.parameterName).isEqualTo("param1");
     Truth.assertThat(firstParam.attributes).isEmpty();
-    Truth.assertThat(firstParam.description).isEqualTo("multi-\nline");
+    Truth.assertThat(firstParam.description).isEqualTo("multi-\n  line");
 
     Truth.assertThat(secondParam.parameterName).isEqualTo("param2");
     Truth.assertThat(secondParam.attributes).isEqualTo(Arrays.asList("mutable", "unused"));
@@ -227,7 +224,7 @@ public class DocstringUtilsTest {
 
     Truth.assertThat(firstParam.parameterName).isEqualTo("param1");
     Truth.assertThat(firstParam.attributes).isEmpty();
-    Truth.assertThat(firstParam.description).isEqualTo("multi-\n\nline");
+    Truth.assertThat(firstParam.description).isEqualTo("multi-\n\n  line");
 
     Truth.assertThat(secondParam.parameterName).isEqualTo("param2");
     Truth.assertThat(secondParam.attributes).isEmpty();