Preserve `pass` statements in the Skylark AST.

RELNOTES: none
PiperOrigin-RevId: 173125138

2 files changed, 8 insertions, 6 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 66b36a245f..3869bfa814 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
@@ -83,7 +83,7 @@ public class DocstringChecker extends SyntaxTreeVisitor {
       Location start = Location.from(node.getLocation().getStartLineAndColumn());
       Location end;
       if (node.getStatements().isEmpty()) {
-        // The function body can be empty because the parser discards `pass` statements:
+        // empty statement suites cannot come from the parser yet we should handle this gracefully:
         end = Location.from(node.getLocation().getEndLineAndColumn());
       } else {
         LineAndColumn lac = node.getStatements().get(0).getLocation().getStartLineAndColumn();
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 876ce24688..9abcfc1614 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
@@ -16,6 +16,7 @@ package com.google.devtools.skylark.skylint;
 
 import com.google.common.truth.Truth;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
+import com.google.devtools.build.lib.syntax.FunctionDefStatement;
 import java.util.List;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -44,11 +45,12 @@ public class DocstringCheckerTests {
         .contains("1:1-2:1: file has no module docstring [missing-docstring]");
     Truth.assertThat(errorMessage)
         .contains("2:1-3:2: function 'function' has no docstring [missing-docstring]");
-    // The following function has zero statements since the parser throws `pass` statements away.
-    // Hence we have to check this case to make sure the end location is set correctly.
-    errorMessage = findIssues("def function():", "  pass # no function docstring").toString();
-    Truth.assertThat(errorMessage)
-        .contains("1:1-2:30: function 'function' has no docstring [missing-docstring]");
+    // This function has zero statements. We want to make sure the end location is set correctly:
+    BuildFileAST ast = BuildFileAST.parseString(event -> {}, "def foo():");
+    FunctionDefStatement funDefWithEmptySuite = (FunctionDefStatement) ast.getStatements().get(0);
+    Truth.assertThat(funDefWithEmptySuite.getStatements()).isEmpty();
+    Truth.assertThat(DocstringChecker.check(ast).toString())
+        .contains("1:1-1:10: function 'foo' has no docstring [missing-docstring]");
   }
 
   @Test