Linter: improve docstring formatting error messages

RELNOTES: none
PiperOrigin-RevId: 174479316

2 files changed, 33 insertions, 7 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 1863bccb1c..0c5486e602 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
@@ -386,6 +386,13 @@ public final class DocstringUtils {
             if (deprecated.isEmpty() && nonStandardDeprecation.isEmpty()) {
               nonStandardDeprecation = checkForNonStandardDeprecation(line);
             }
+            if (line.startsWith("Returns: ")) {
+              error(
+                  "the return value should be documented in a section, like this:\n\n"
+                      + "Returns:\n"
+                      + "  <documentation here>\n\n"
+                      + "For more details, please have a look at the documentation.");
+            }
             if (!(onLastLine() && line.trim().isEmpty())) {
               longDescriptionLines.add(line);
             }
@@ -411,7 +418,11 @@ public final class DocstringUtils {
 
     private String checkForNonStandardDeprecation(String line) {
       if (line.toLowerCase().startsWith("deprecated:") || line.contains("DEPRECATED")) {
-        error("use a 'Deprecated:' section for deprecations, similar to a 'Returns:' section");
+        error(
+            "use a 'Deprecated:' section for deprecations, similar to a 'Returns:' section:\n\n"
+                + "Deprecated:\n"
+                + "  <reason and alternative>\n\n"
+                + "For more details, please have a look at the documentation.");
         return line;
       }
       return "";
@@ -456,7 +467,10 @@ public final class DocstringUtils {
         trimmedLine = line.substring(actualIndentation);
         Matcher matcher = paramLineMatcher.matcher(trimmedLine);
         if (!matcher.matches()) {
-          error("invalid parameter documentation");
+          error(
+              "invalid parameter documentation"
+                  + " (expected format: \"parameter_name: documentation\")."
+                  + "For more details, please have a look at the documentation.");
           nextLine();
           continue;
         }
@@ -475,7 +489,7 @@ public final class DocstringUtils {
         params.add(new ParameterDoc(parameterName, attributes, parameterDescription));
       }
       if (params.isEmpty()) {
-        error(sectionLineNumber, "section is empty");
+        error(sectionLineNumber, "section is empty or badly formatted");
       }
       return params;
     }
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 ee07fd488f..df84e6e6a3 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
@@ -212,7 +212,7 @@ public class DocstringUtilsTest {
     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");
+    Truth.assertThat(errors.toString()).contains("3: section is empty or badly formatted");
 
     errors = new ArrayList<>();
     DocstringUtils.parseDocstring(
@@ -233,6 +233,15 @@ public class DocstringUtilsTest {
   }
 
   @Test
+  public void sectionOnOneLine() throws Exception {
+    List<DocstringParseError> errors = new ArrayList<>();
+    DocstringUtils.parseDocstring("summary\n" + "\n" + "Returns: foo\n", 0, errors);
+    Truth.assertThat(errors).hasSize(1);
+    Truth.assertThat(errors.get(0).toString())
+        .startsWith("3: the return value should be documented in a section");
+  }
+
+  @Test
   public void docstringReturn() throws Exception {
     List<DocstringParseError> errors = new ArrayList<>();
     DocstringInfo info =
@@ -282,7 +291,7 @@ public class DocstringUtilsTest {
     Truth.assertThat(info.longDescription).isEqualTo("Deprecated: foo");
     Truth.assertThat(errors).hasSize(1);
     Truth.assertThat(errors.get(0).toString())
-        .isEqualTo(
+        .startsWith(
             "3: use a 'Deprecated:' section for deprecations, similar to a 'Returns:' section");
 
     errors = new ArrayList<>();
@@ -295,7 +304,7 @@ public class DocstringUtilsTest {
     Truth.assertThat(info.longDescription).isEqualTo("This is DEPRECATED.");
     Truth.assertThat(errors).hasSize(1);
     Truth.assertThat(errors.get(0).toString())
-        .isEqualTo(
+        .startsWith(
             "3: use a 'Deprecated:' section for deprecations, similar to a 'Returns:' section");
   }
 
@@ -494,7 +503,10 @@ public class DocstringUtilsTest {
             2,
             errors);
     Truth.assertThat(info.parameters).isEmpty();
-    Truth.assertThat(errors.toString()).contains("4: invalid parameter documentation");
+    Truth.assertThat(errors.toString())
+        .contains(
+            "4: invalid parameter documentation"
+                + " (expected format: \"parameter_name: documentation\").");
   }
 
   @Test