bazel syntax: delete StarlarkFunction.getStatements
Clients cannot assume the representation of code is syntax trees;
soon, it will be bytecode.
Details:
- added StarlarkFunction.getDocumentation method.
- remove dead code from DocstringUtils
collectDocstringLiterals, getNameAndDocstring, getAssignedVariableName, extractDocstring
- DocstringUtils.parseDocstring now computes the indentation using a heuristic.
- removed assertion from StarlarkFunctionCodecTest
- rephrased assertion in PackageSerializationTest
- remove function syntax from some obscure log messages
PiperOrigin-RevId: 290084502
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkFunction.java
index bd53793..b9bf93b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkFunction.java
@@ -15,6 +15,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.events.Location;
+import javax.annotation.Nullable;
/** A StarlarkFunction is the function value created by a Starlark {@code def} statement. */
public final class StarlarkFunction extends BaseFunction {
@@ -69,11 +70,21 @@
return name;
}
- /** @deprecated Do not assume function values are represented as syntax trees. */
- // TODO(adonovan): the only non-test use is to obtain the function's doc string. Add API for that.
- @Deprecated
- public ImmutableList<Statement> getStatements() {
- return statements;
+ /** Returns the value denoted by the function's doc string literal, or null if absent. */
+ @Nullable
+ public String getDocumentation() {
+ if (statements.isEmpty()) {
+ return null;
+ }
+ Statement first = statements.get(0);
+ if (!(first instanceof ExpressionStatement)) {
+ return null;
+ }
+ Expression expr = ((ExpressionStatement) first).getExpression();
+ if (!(expr instanceof StringLiteral)) {
+ return null;
+ }
+ return ((StringLiteral) expr).getValue();
}
public Module getModule() {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
index bee563d..d38f85d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
@@ -23,16 +23,18 @@
/** Syntax node for a string literal. */
public final class StringLiteral extends Expression {
- String value;
- public String getValue() {
- return value;
- }
+ private final String value;
StringLiteral(String value) {
this.value = value;
}
+ /** Returns the value denoted by the string literal */
+ public String getValue() {
+ return value;
+ }
+
@Override
public void accept(NodeVisitor visitor) {
visitor.visit(this);
diff --git a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
index 06fe769..02ed1e0 100644
--- a/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
+++ b/src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
@@ -58,6 +58,8 @@
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
+import com.google.devtools.build.lib.syntax.Expression;
+import com.google.devtools.build.lib.syntax.ExpressionStatement;
import com.google.devtools.build.lib.syntax.LoadStatement;
import com.google.devtools.build.lib.syntax.Module;
import com.google.devtools.build.lib.syntax.Mutability;
@@ -122,7 +124,6 @@
import com.google.devtools.build.skydoc.rendering.proto.StardocOutputProtos.ProviderInfo;
import com.google.devtools.build.skydoc.rendering.proto.StardocOutputProtos.RuleInfo;
import com.google.devtools.common.options.OptionsParser;
-import com.google.devtools.skylark.common.DocstringUtils;
import java.io.BufferedOutputStream;
import java.io.FileOutputStream;
import java.io.IOException;
@@ -395,10 +396,12 @@
private static String getModuleDoc(StarlarkFile buildFileAST) {
ImmutableList<Statement> fileStatements = buildFileAST.getStatements();
if (!fileStatements.isEmpty()) {
- Statement moduleComment = fileStatements.get(0);
- StringLiteral moduleDocLiteral = DocstringUtils.getStringLiteral(moduleComment);
- if (moduleDocLiteral != null) {
- return moduleDocLiteral.getValue();
+ Statement stmt = fileStatements.get(0);
+ if (stmt instanceof ExpressionStatement) {
+ Expression expr = ((ExpressionStatement) stmt).getExpression();
+ if (expr instanceof StringLiteral) {
+ return ((StringLiteral) expr).getValue();
+ }
}
}
return "";
diff --git a/src/main/java/com/google/devtools/build/skydoc/rendering/FunctionUtil.java b/src/main/java/com/google/devtools/build/skydoc/rendering/FunctionUtil.java
index efe1a8e..712a6ca 100644
--- a/src/main/java/com/google/devtools/build/skydoc/rendering/FunctionUtil.java
+++ b/src/main/java/com/google/devtools/build/skydoc/rendering/FunctionUtil.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.Printer.BasePrinter;
import com.google.devtools.build.lib.syntax.StarlarkFunction;
-import com.google.devtools.build.lib.syntax.StringLiteral;
import com.google.devtools.build.skydoc.rendering.proto.StardocOutputProtos.FunctionParamInfo;
import com.google.devtools.build.skydoc.rendering.proto.StardocOutputProtos.StarlarkFunctionInfo;
import com.google.devtools.skylark.common.DocstringUtils;
@@ -41,24 +40,21 @@
* @param functionName the name of the function in the target scope. (Note this is not necessarily
* the original exported function name; the function may have been renamed in the target
* Starlark file's scope)
- * @param userDefinedFunction the raw function object
+ * @param fn the function object
* @throws com.google.devtools.build.skydoc.rendering.DocstringParseException if the function's
* docstring is malformed
*/
- public static StarlarkFunctionInfo fromNameAndFunction(
- String functionName, StarlarkFunction userDefinedFunction) throws DocstringParseException {
+ public static StarlarkFunctionInfo fromNameAndFunction(String functionName, StarlarkFunction fn)
+ throws DocstringParseException {
String functionDescription = "";
Map<String, String> paramNameToDocMap = Maps.newLinkedHashMap();
- StringLiteral docStringLiteral =
- DocstringUtils.extractDocstring(userDefinedFunction.getStatements());
-
- if (docStringLiteral != null) {
+ String doc = fn.getDocumentation();
+ if (doc != null) {
List<DocstringParseError> parseErrors = Lists.newArrayList();
- DocstringInfo docstringInfo = DocstringUtils.parseDocstring(docStringLiteral, parseErrors);
+ DocstringInfo docstringInfo = DocstringUtils.parseDocstring(doc, parseErrors);
if (!parseErrors.isEmpty()) {
- throw new DocstringParseException(
- functionName, userDefinedFunction.getLocation(), parseErrors);
+ throw new DocstringParseException(functionName, fn.getLocation(), parseErrors);
}
functionDescription += docstringInfo.getSummary();
if (!docstringInfo.getSummary().isEmpty() && !docstringInfo.getLongDescription().isEmpty()) {
@@ -69,7 +65,7 @@
paramNameToDocMap.put(paramDoc.getParameterName(), paramDoc.getDescription());
}
}
- List<FunctionParamInfo> paramsInfo = parameterInfos(userDefinedFunction, paramNameToDocMap);
+ List<FunctionParamInfo> paramsInfo = parameterInfos(fn, paramNameToDocMap);
return StarlarkFunctionInfo.newBuilder()
.setFunctionName(functionName)
.setDocString(functionDescription)
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
index b97ac8d..096247b 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
@@ -40,7 +40,6 @@
assertThat(stmt).isNotNull();
assertThat(stmt.getName()).isEqualTo("func");
assertThat(stmt.getSignature().numMandatoryPositionals()).isEqualTo(3);
- assertThat(stmt.getStatements()).hasSize(2);
}
@Test
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 6a6decf..37df98f 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
@@ -273,7 +273,7 @@
@Test
public void testStringLiteralOptimizationValue() throws Exception {
StringLiteral l = (StringLiteral) parseExpression("'abc' + 'def'");
- assertThat(l.value).isEqualTo("abcdef");
+ assertThat(l.getValue()).isEqualTo("abcdef");
}
@Test
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/common/DocstringUtils.java b/src/tools/skylark/java/com/google/devtools/skylark/common/DocstringUtils.java
index af7daea..f57dabb 100644
--- a/src/tools/skylark/java/com/google/devtools/skylark/common/DocstringUtils.java
+++ b/src/tools/skylark/java/com/google/devtools/skylark/common/DocstringUtils.java
@@ -15,123 +15,20 @@
package com.google.devtools.skylark.common;
import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.syntax.AssignmentStatement;
-import com.google.devtools.build.lib.syntax.DefStatement;
-import com.google.devtools.build.lib.syntax.Expression;
-import com.google.devtools.build.lib.syntax.ExpressionStatement;
-import com.google.devtools.build.lib.syntax.Identifier;
-import com.google.devtools.build.lib.syntax.StarlarkFile;
-import com.google.devtools.build.lib.syntax.Statement;
-import com.google.devtools.build.lib.syntax.StringLiteral;
import com.google.devtools.skylark.common.LocationRange.Location;
-import java.util.AbstractMap;
-import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
-import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import javax.annotation.Nullable;
/** Utilities to extract and parse docstrings. */
public final class DocstringUtils {
- private DocstringUtils() {}
- /**
- * Collect all docstrings in the AST and store them in a map: name -> docstring.
- *
- * <p>Note that local variables can't have docstrings.
- *
- * @param ast the AST to traverse
- * @return a map from identifier names to their docstring; if there is a file-level docstring, its
- * key is "".
- */
- public static ImmutableMap<String, StringLiteral> collectDocstringLiterals(StarlarkFile ast) {
- ImmutableMap.Builder<String, StringLiteral> nameToDocstringLiteral = ImmutableMap.builder();
- Statement previousStatement = null;
- for (Statement currentStatement : ast.getStatements()) {
- Map.Entry<String, StringLiteral> entry =
- getNameAndDocstring(previousStatement, currentStatement);
- if (entry != null) {
- nameToDocstringLiteral.put(entry);
- }
- previousStatement = currentStatement;
- }
- return nameToDocstringLiteral.build();
- }
-
- @Nullable
- private static Map.Entry<String, StringLiteral> getNameAndDocstring(
- @Nullable Statement previousStatement, Statement currentStatement) {
- // function docstring:
- if (currentStatement instanceof DefStatement) {
- StringLiteral docstring = extractDocstring(((DefStatement) currentStatement).getStatements());
- if (docstring != null) {
- return new AbstractMap.SimpleEntry<>(
- ((DefStatement) currentStatement).getIdentifier().getName(), docstring);
- }
- } else {
- StringLiteral docstring = getStringLiteral(currentStatement);
- if (docstring != null) {
- if (previousStatement == null) {
- // file docstring:
- return new SimpleEntry<>("", docstring);
- } else {
- // variable docstring:
- String variable = getAssignedVariableName(previousStatement);
- if (variable != null) {
- return new SimpleEntry<>(variable, docstring);
- }
- }
- }
- }
- return null;
- }
-
- /** If the statement is an assignment to one variable, returns its name, or otherwise null. */
- @Nullable
- public static String getAssignedVariableName(@Nullable Statement stmt) {
- if (stmt instanceof AssignmentStatement) {
- AssignmentStatement assign = (AssignmentStatement) stmt;
- Expression lhs = assign.getLHS();
- if (lhs instanceof Identifier && !assign.isAugmented()) {
- return ((Identifier) lhs).getName();
- }
- }
- return null;
- }
-
- /** If the statement is a string literal, returns it, or otherwise null. */
- @Nullable
- public static StringLiteral getStringLiteral(Statement stmt) {
- if (stmt instanceof ExpressionStatement) {
- Expression expr = ((ExpressionStatement) stmt).getExpression();
- if (expr instanceof StringLiteral) {
- return (StringLiteral) expr;
- }
- }
- return null;
- }
-
- /** Takes a function body and returns the docstring literal, if present. */
- @Nullable
- public static StringLiteral extractDocstring(List<Statement> statements) {
- if (statements.isEmpty()) {
- return null;
- }
- return getStringLiteral(statements.get(0));
- }
-
- /** Parses a docstring from a string literal and appends any new errors to the given list. */
- public static DocstringInfo parseDocstring(
- StringLiteral docstring, List<DocstringParseError> errors) {
- int indentation = docstring.getStartLocation().column() - 1;
- return parseDocstring(docstring.getValue(), indentation, errors);
- }
+ private DocstringUtils() {} // uninstantiable
/**
* Parses a docstring.
@@ -163,13 +60,11 @@
* }</pre>
*
* @param docstring a docstring of the format described above
- * @param indentation the indentation level (number of spaces) of the docstring
* @param parseErrors a list to which parsing error messages are written
* @return the parsed docstring information
*/
- public static DocstringInfo parseDocstring(
- String docstring, int indentation, List<DocstringParseError> parseErrors) {
- DocstringParser parser = new DocstringParser(docstring, indentation);
+ public static DocstringInfo parseDocstring(String doc, List<DocstringParseError> parseErrors) {
+ DocstringParser parser = new DocstringParser(doc);
DocstringInfo result = parser.parse();
parseErrors.addAll(parser.errors);
return result;
@@ -289,7 +184,7 @@
/** Current line number within the docstring. */
private int lineNumber = 0;
/**
- * The indentation of the doctring literal in the source file.
+ * The indentation of the docstring literal in the source file.
*
* <p>Every line except the first one must be indented by at least that many spaces.
*/
@@ -311,8 +206,31 @@
/** Errors that occurred so far. */
private final List<DocstringParseError> errors = new ArrayList<>();
- DocstringParser(String docstring, int indentation) {
+ private DocstringParser(String docstring) {
this.docstring = docstring;
+
+ // Infer the indentation level:
+ // the smallest amount of leading whitespace
+ // common to all non-blank lines except the first.
+ int indentation = Integer.MAX_VALUE;
+ boolean first = true;
+ for (String line : Splitter.on("\n").split(docstring)) {
+ // ignore first line
+ if (first) {
+ first = false;
+ continue;
+ }
+ // count leading spaces
+ int i;
+ for (i = 0; i < line.length() && line.charAt(i) == ' '; i++) {}
+ if (i != line.length()) {
+ indentation = Math.min(indentation, i);
+ }
+ }
+ if (indentation == Integer.MAX_VALUE) {
+ indentation = 0;
+ }
+
nextLine();
// the indentation is only relevant for the following lines, not the first one:
this.baselineIndentation = indentation;
