Preserve `pass` statements in the Skylark AST.
RELNOTES: none
PiperOrigin-RevId: 173125138
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 4842374..9a1ed7a 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 @@
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 e59f329..ac0902a 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 @@
// 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 0000000..e035100
--- /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 1f08bbb..43363fc 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 @@
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 c7c0eb0..22bc8b7 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 @@
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 858e0ca..a19fc83 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 @@
@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 @@
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 66b36a2..3869bfa 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 @@
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 876ce24..9abcfc1 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 @@
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 @@
.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
