Preserve `pass` statements in the Skylark AST.

RELNOTES: none
PiperOrigin-RevId: 173125138

8 files changed, 54 insertions, 10 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
index 4842374765..9a1ed7a46c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
@@ -206,6 +206,8 @@ public class Eval {
       case LOAD:
         execLoad((LoadStatement) st);
         break;
+      case PASS:
+        break;
       case RETURN:
         execReturn((ReturnStatement) st);
         break;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index e59f3295af..ac0902a1e2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -1120,8 +1120,7 @@ public class Parser {
   // small_stmt | 'pass'
   private void parseSmallStatementOrPass(List<Statement> list) {
     if (token.kind == TokenKind.PASS) {
-      // Skip the token, don't add it to the list.
-      // It has no existence in the AST.
+      list.add(setLocation(new PassStatement(), token.left, token.right));
       expect(TokenKind.PASS);
     } else {
       list.add(parseSmallStatement());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
new file mode 100644
index 0000000000..e0351002ef
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/syntax/PassStatement.java
@@ -0,0 +1,37 @@
+// 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.build.lib.syntax;
+
+import java.io.IOException;
+
+/** Syntax node for a `pass` statement. */
+public class PassStatement extends Statement {
+
+  @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append("pass\n");
+  }
+
+  @Override
+  public void accept(SyntaxTreeVisitor visitor) {
+    visitor.visit(this);
+  }
+
+  @Override
+  public Kind kind() {
+    return Kind.PASS;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
index 1f08bbb030..43363fc310 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java
@@ -33,6 +33,7 @@ public abstract class Statement extends ASTNode {
     FUNCTION_DEF,
     IF,
     LOAD,
+    PASS,
     RETURN,
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
index c7c0eb0e53..22bc8b724b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
@@ -141,6 +141,8 @@ public class SyntaxTreeVisitor {
     visitBlock(node.getStatements());
   }
 
+  public void visit(PassStatement node) {}
+
   public void visit(ReturnStatement node) {
     if (node.getReturnExpression() != null) {
       visit(node.getReturnExpression());
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 858e0cafe4..a19fc83250 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -982,7 +982,8 @@ public class ParserTest extends EvaluationTestCase {
   @Test
   public void testPass() throws Exception {
     List<Statement> statements = parseFileForSkylark("pass\n");
-    assertThat(statements).isEmpty();
+    assertThat(statements).hasSize(1);
+    assertThat(statements.get(0)).isInstanceOf(PassStatement.class);
   }
 
   @Test
@@ -993,7 +994,7 @@ public class ParserTest extends EvaluationTestCase {
 
     assertThat(statements).hasSize(1);
     FunctionDefStatement stmt = (FunctionDefStatement) statements.get(0);
-    assertThat(stmt.getStatements()).isEmpty();
+    assertThat(stmt.getStatements().get(0)).isInstanceOf(PassStatement.class);
   }
 
   @Test
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