From edc799eb1d2ceee59c1695eff311537a33aabb38 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 31 May 2022 10:30:12 +0200 Subject: [PATCH 1/5] Fix NPE in Parameter.Star#getLocation() for unnamed vararg --- src/main/java/net/starlark/java/syntax/Parameter.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/net/starlark/java/syntax/Parameter.java b/src/main/java/net/starlark/java/syntax/Parameter.java index 8b7da9b1a42fa0..516670a4624fa3 100644 --- a/src/main/java/net/starlark/java/syntax/Parameter.java +++ b/src/main/java/net/starlark/java/syntax/Parameter.java @@ -116,6 +116,9 @@ public int getStartOffset() { @Override public int getEndOffset() { + if (getIdentifier() == null) { + return starOffset + 1; + } return getIdentifier().getEndOffset(); } } From 4d1fe9b2e1c14c283dc9d8d0436a1b13a742d738 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sun, 29 May 2022 19:55:19 +0200 Subject: [PATCH 2/5] Fix NodePrinter crash on Comprehension.Clause --- .../net/starlark/java/syntax/NodePrinter.java | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/main/java/net/starlark/java/syntax/NodePrinter.java b/src/main/java/net/starlark/java/syntax/NodePrinter.java index 7d6cebf8d3f501..4fcf4f26af5478 100644 --- a/src/main/java/net/starlark/java/syntax/NodePrinter.java +++ b/src/main/java/net/starlark/java/syntax/NodePrinter.java @@ -41,6 +41,9 @@ void printNode(Node n) { } else if (n instanceof Statement) { printStmt((Statement) n); + } else if (n instanceof Comprehension.Clause) { + printClause((Comprehension.Clause) n); + } else if (n instanceof StarlarkFile) { StarlarkFile file = (StarlarkFile) n; // Only statements are printed, not comments. @@ -277,17 +280,7 @@ private void printExpr(Expression expr) { printNode(comp.getBody()); // Expression or DictExpression.Entry for (Comprehension.Clause clause : comp.getClauses()) { buf.append(' '); - if (clause instanceof Comprehension.For) { - Comprehension.For forClause = (Comprehension.For) clause; - buf.append("for "); - printExpr(forClause.getVars()); - buf.append(" in "); - printExpr(forClause.getIterable()); - } else { - Comprehension.If ifClause = (Comprehension.If) clause; - buf.append("if "); - printExpr(ifClause.getCondition()); - } + printClause(clause); } buf.append(comp.isDict() ? '}' : ']'); break; @@ -482,4 +475,18 @@ private void printExpr(Expression expr) { } } } + + private void printClause(Comprehension.Clause clause) { + if (clause instanceof Comprehension.For) { + Comprehension.For forClause = (Comprehension.For) clause; + buf.append("for "); + printExpr(forClause.getVars()); + buf.append(" in "); + printExpr(forClause.getIterable()); + } else { + Comprehension.If ifClause = (Comprehension.If) clause; + buf.append("if "); + printExpr(ifClause.getCondition()); + } + } } From fc580d7be68688096e4edd95f8a0990822aca7c8 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 30 May 2022 15:20:44 +0200 Subject: [PATCH 3/5] Remove redundant null check in NodeVisitor A `ConditionalExpression`, differing from an `IfStatement`, always has an `else` case. --- src/main/java/net/starlark/java/syntax/NodeVisitor.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/net/starlark/java/syntax/NodeVisitor.java b/src/main/java/net/starlark/java/syntax/NodeVisitor.java index 096f9506a46f8a..e7ba3b81375b85 100644 --- a/src/main/java/net/starlark/java/syntax/NodeVisitor.java +++ b/src/main/java/net/starlark/java/syntax/NodeVisitor.java @@ -174,9 +174,7 @@ public void visit(@SuppressWarnings("unused") Comment node) {} public void visit(ConditionalExpression node) { visit(node.getCondition()); visit(node.getThenCase()); - if (node.getElseCase() != null) { - visit(node.getElseCase()); - } + visit(node.getElseCase()); } // methods dealing with sequences of nodes From 90607e1158fbae2951531441ada617f4b2b7e264 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sun, 29 May 2022 19:55:19 +0200 Subject: [PATCH 4/5] Add an LCOV coverage report generator for Starlark The generated coverage report includes function, line, and branch coverage information in LCOV format. --- .../java/net/starlark/java/eval/Eval.java | 40 ++- .../java/net/starlark/java/eval/Starlark.java | 12 + src/main/java/net/starlark/java/syntax/BUILD | 2 + .../java/syntax/CoverageRecorder.java | 334 ++++++++++++++++++ .../starlark/java/syntax/CoverageVisitor.java | 331 +++++++++++++++++ .../net/starlark/java/syntax/Program.java | 4 +- 6 files changed, 718 insertions(+), 5 deletions(-) create mode 100644 src/main/java/net/starlark/java/syntax/CoverageRecorder.java create mode 100644 src/main/java/net/starlark/java/syntax/CoverageVisitor.java diff --git a/src/main/java/net/starlark/java/eval/Eval.java b/src/main/java/net/starlark/java/eval/Eval.java index 594afd97f7705a..71dcdb7f00ef8e 100644 --- a/src/main/java/net/starlark/java/eval/Eval.java +++ b/src/main/java/net/starlark/java/eval/Eval.java @@ -29,6 +29,7 @@ import net.starlark.java.syntax.CallExpression; import net.starlark.java.syntax.Comprehension; import net.starlark.java.syntax.ConditionalExpression; +import net.starlark.java.syntax.CoverageRecorder; import net.starlark.java.syntax.DefStatement; import net.starlark.java.syntax.DictExpression; import net.starlark.java.syntax.DotExpression; @@ -45,6 +46,7 @@ import net.starlark.java.syntax.ListExpression; import net.starlark.java.syntax.LoadStatement; import net.starlark.java.syntax.Location; +import net.starlark.java.syntax.Parameter; import net.starlark.java.syntax.Resolver; import net.starlark.java.syntax.ReturnStatement; import net.starlark.java.syntax.SliceExpression; @@ -131,6 +133,7 @@ private static TokenKind execFor(StarlarkThread.Frame fr, ForStatement node) continue; case BREAK: // Finish loop, execute next statement after loop. + CoverageRecorder.getInstance().recordVirtualJump(node); return TokenKind.PASS; case RETURN: // Finish loop, return from function. @@ -145,6 +148,7 @@ private static TokenKind execFor(StarlarkThread.Frame fr, ForStatement node) } finally { EvalUtils.removeIterator(seq); } + CoverageRecorder.getInstance().recordVirtualJump(node); return TokenKind.PASS; } @@ -159,7 +163,9 @@ private static StarlarkFunction newFunction(StarlarkThread.Frame fr, Resolver.Fu int nparams = rfn.getParameters().size() - (rfn.hasKwargs() ? 1 : 0) - (rfn.hasVarargs() ? 1 : 0); for (int i = 0; i < nparams; i++) { - Expression expr = rfn.getParameters().get(i).getDefaultValue(); + Parameter parameter = rfn.getParameters().get(i); + CoverageRecorder.getInstance().recordCoverage(parameter.getIdentifier()); + Expression expr = parameter.getDefaultValue(); if (expr == null && defaults == null) { continue; // skip prefix of required parameters } @@ -169,6 +175,10 @@ private static StarlarkFunction newFunction(StarlarkThread.Frame fr, Resolver.Fu defaults[i - (nparams - defaults.length)] = expr == null ? StarlarkFunction.MANDATORY : eval(fr, expr); } + // Visit kwargs and varargs for coverage. + for (int i = nparams; i < rfn.getParameters().size(); i++) { + CoverageRecorder.getInstance().recordCoverage(rfn.getParameters().get(i).getIdentifier()); + } if (defaults == null) { defaults = EMPTY; } @@ -206,6 +216,7 @@ private static TokenKind execIf(StarlarkThread.Frame fr, IfStatement node) } else if (node.getElseBlock() != null) { return execStatements(fr, node.getElseBlock(), /*indented=*/ true); } + CoverageRecorder.getInstance().recordVirtualJump(node); return TokenKind.PASS; } @@ -262,6 +273,7 @@ private static TokenKind exec(StarlarkThread.Frame fr, Statement st) if (++fr.thread.steps >= fr.thread.stepLimit) { throw new EvalException("Starlark computation cancelled: too many steps"); } + CoverageRecorder.getInstance().recordCoverage(st); switch (st.kind()) { case ASSIGNMENT: @@ -330,6 +342,7 @@ private static void assign(StarlarkThread.Frame fr, Expression lhs, Object value private static void assignIdentifier(StarlarkThread.Frame fr, Identifier id, Object value) throws EvalException { + CoverageRecorder.getInstance().recordCoverage(id); Resolver.Binding bind = id.getBinding(); switch (bind.getScope()) { case LOCAL: @@ -477,6 +490,7 @@ private static Object eval(StarlarkThread.Frame fr, Expression expr) if (++fr.thread.steps >= fr.thread.stepLimit) { throw new EvalException("Starlark computation cancelled: too many steps"); } + CoverageRecorder.getInstance().recordCoverage(expr); // The switch cases have been split into separate functions // to reduce the stack usage during recursion, which is @@ -533,9 +547,17 @@ private static Object evalBinaryOperator(StarlarkThread.Frame fr, BinaryOperator // AND and OR require short-circuit evaluation. switch (binop.getOperator()) { case AND: - return Starlark.truth(x) ? eval(fr, binop.getY()) : x; + if (Starlark.truth(x)) { + return eval(fr, binop.getY()); + } + CoverageRecorder.getInstance().recordVirtualJump(binop); + return x; case OR: - return Starlark.truth(x) ? x : eval(fr, binop.getY()); + if (Starlark.truth(x)) { + CoverageRecorder.getInstance().recordVirtualJump(binop); + return x; + } + return eval(fr, binop.getY()); default: Object y = eval(fr, binop.getY()); try { @@ -577,7 +599,9 @@ private static Object evalDict(StarlarkThread.Frame fr, DictExpression dictexpr) private static Object evalDot(StarlarkThread.Frame fr, DotExpression dot) throws EvalException, InterruptedException { Object object = eval(fr, dot.getObject()); - String name = dot.getField().getName(); + Identifier field = dot.getField(); + CoverageRecorder.getInstance().recordCoverage(field); + String name = field.getName(); try { return Starlark.getattr( fr.thread.mutability(), fr.thread.getSemantics(), object, name, /*defaultValue=*/ null); @@ -627,6 +651,7 @@ private static Object evalCall(StarlarkThread.Frame fr, CallExpression call) Object[] positional = npos == 0 ? EMPTY : new Object[npos]; for (i = 0; i < npos; i++) { Argument arg = arguments.get(i); + CoverageRecorder.getInstance().recordCoverage(arg); Object value = eval(fr, arg.getValue()); positional[i] = value; } @@ -635,6 +660,7 @@ private static Object evalCall(StarlarkThread.Frame fr, CallExpression call) Object[] named = n == npos ? EMPTY : new Object[2 * (n - npos)]; for (int j = 0; i < n; i++) { Argument.Keyword arg = (Argument.Keyword) arguments.get(i); + CoverageRecorder.getInstance().recordCoverage(arg); Object value = eval(fr, arg.getValue()); named[j++] = arg.getName(); named[j++] = value; @@ -642,6 +668,7 @@ private static Object evalCall(StarlarkThread.Frame fr, CallExpression call) // f(*args) -- varargs if (star != null) { + CoverageRecorder.getInstance().recordCoverage(star); Object value = eval(fr, star.getValue()); if (!(value instanceof StarlarkIterable)) { fr.setErrorLocation(star.getStartLocation()); @@ -656,6 +683,7 @@ private static Object evalCall(StarlarkThread.Frame fr, CallExpression call) // f(**kwargs) if (starstar != null) { + CoverageRecorder.getInstance().recordCoverage(starstar); Object value = eval(fr, starstar.getValue()); if (!(value instanceof Dict)) { fr.setErrorLocation(starstar.getStartLocation()); @@ -791,6 +819,7 @@ void execClauses(int index) throws EvalException, InterruptedException { assign(fr, forClause.getVars(), elem); execClauses(index + 1); } + CoverageRecorder.getInstance().recordVirtualJump(clause); } catch (EvalException ex) { fr.setErrorLocation(forClause.getStartLocation()); throw ex; @@ -802,12 +831,15 @@ void execClauses(int index) throws EvalException, InterruptedException { Comprehension.If ifClause = (Comprehension.If) clause; if (Starlark.truth(eval(fr, ifClause.getCondition()))) { execClauses(index + 1); + } else { + CoverageRecorder.getInstance().recordVirtualJump(clause); } } return; } // base case: evaluate body and add to result. + CoverageRecorder.getInstance().recordCoverage(comp.getBody()); if (dict != null) { DictExpression.Entry body = (DictExpression.Entry) comp.getBody(); Object k = eval(fr, body.getKey()); diff --git a/src/main/java/net/starlark/java/eval/Starlark.java b/src/main/java/net/starlark/java/eval/Starlark.java index c014a11ece578c..8d2e813794e108 100644 --- a/src/main/java/net/starlark/java/eval/Starlark.java +++ b/src/main/java/net/starlark/java/eval/Starlark.java @@ -23,6 +23,7 @@ import com.google.errorprone.annotations.FormatMethod; import java.io.IOException; import java.io.OutputStream; +import java.io.PrintWriter; import java.lang.reflect.Method; import java.math.BigInteger; import java.time.Duration; @@ -30,12 +31,15 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.function.Function; +import java.util.regex.Pattern; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import net.starlark.java.annot.StarlarkAnnotations; import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.annot.StarlarkMethod; import net.starlark.java.spelling.SpellChecker; +import net.starlark.java.syntax.CoverageRecorder; import net.starlark.java.syntax.Expression; import net.starlark.java.syntax.FileOptions; import net.starlark.java.syntax.ParserInput; @@ -970,4 +974,12 @@ public static boolean startCpuProfile(OutputStream out, Duration period) { public static void stopCpuProfile() throws IOException { CpuProfiler.stop(); } + + public static void startCoverageCollection(Function filenameMatcher) { + CoverageRecorder.startCoverageCollection(filenameMatcher); + } + + public static void dumpCoverage(PrintWriter out) { + CoverageRecorder.getInstance().dump(out); + } } diff --git a/src/main/java/net/starlark/java/syntax/BUILD b/src/main/java/net/starlark/java/syntax/BUILD index 967fb3210b414d..88c857eaa096c7 100644 --- a/src/main/java/net/starlark/java/syntax/BUILD +++ b/src/main/java/net/starlark/java/syntax/BUILD @@ -21,6 +21,8 @@ java_library( "Comment.java", "Comprehension.java", "ConditionalExpression.java", + "CoverageRecorder.java", + "CoverageVisitor.java", "DefStatement.java", "DictExpression.java", "DotExpression.java", diff --git a/src/main/java/net/starlark/java/syntax/CoverageRecorder.java b/src/main/java/net/starlark/java/syntax/CoverageRecorder.java new file mode 100644 index 00000000000000..0264c611de53cf --- /dev/null +++ b/src/main/java/net/starlark/java/syntax/CoverageRecorder.java @@ -0,0 +1,334 @@ +// Copyright 2022 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 net.starlark.java.syntax; + +import java.io.PrintWriter; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.LongAdder; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import javax.annotation.Nullable; + +public interface CoverageRecorder { + + void register(Program program); + + void recordCoverage(Node node); + + void recordVirtualJump(Node node); + + void dump(PrintWriter out); + + static CoverageRecorder getInstance() { + return CoverageRecorderHolder.INSTANCE; + } + + /** + * Collect coverage for all {@link Program}s compiled after the call whose + * {@link Program#getFilename()} matches {@code filenameMatcher}. + */ + static void startCoverageCollection(Function filenameMatcher) { + CoverageRecorderHolder.INSTANCE = new LcovCoverageRecorder(filenameMatcher); + } + + class CoverageRecorderHolder { + + private static CoverageRecorder INSTANCE = new NoopCoverageRecorder(); + + private CoverageRecorderHolder() { + } + } +} + +final class NoopCoverageRecorder implements CoverageRecorder { + + @Override + public void register(Program program) { + } + + @Override + public void recordCoverage(Node node) { + } + + @Override + public void recordVirtualJump(Node node) { + } + + @Override + public void dump(PrintWriter out) { + } +} + +/** + * A {@link CoverageRecorder} that records function, line, and branch coverage for all Starlark + * {@link Program}s matching the provided {@code filenameMatcher}. Calling + * {@link LcovCoverageRecorder#dump(PrintWriter)} emits LCOV records for all matched files. + */ +final class LcovCoverageRecorder implements CoverageRecorder { + + private final Function filenameMatcher; + + /** + * Tracks the number of times a given {@link Node} has been executed. + */ + private final ConcurrentHashMap counts = new ConcurrentHashMap<>(); + + /** + * Tracks the number of times a conditional jump without a syntax tree representation has been + * executed which is associated with the given {@link Node}. Examples: - The "condition not + * satisfied" jump of an {@code if} without an {@code else}. - The "short-circuit" jump of an + * {@code and} or {@code or}. + */ + private final ConcurrentHashMap virtualJumpCounts = new ConcurrentHashMap<>(); + + private final Set registeredPrograms = ConcurrentHashMap.newKeySet(); + + LcovCoverageRecorder(Function filenameMatcher) { + this.filenameMatcher = filenameMatcher; + } + + @Override + public void register(Program program) { + if (!filenameMatcher.apply(program.getFilename())) { + return; + } + registeredPrograms.add(program); + for (Statement statement : program.getResolvedFunction().getBody()) { + statement.accept(new CoverageVisitor() { + @Override + protected void visitFunction(String identifier, Node defStatement, + Node firstBodyStatement) { + } + + @Override + protected void visitBranch(Node owner, Node condition, Node positiveUniqueSuccessor, + @Nullable Node negativeUniqueSuccessor) { + if (negativeUniqueSuccessor == null) { + virtualJumpCounts.put(owner, new LongAdder()); + } + // positiveUniqueSuccessor will be registered via a call to visitCode. + } + + @Override + protected void visitCode(Node node) { + counts.put(node, new LongAdder()); + } + }); + } + } + + @Override + public void recordCoverage(Node node) { + LongAdder counter = counts.get(node); + if (counter == null) { + return; + } + counter.increment(); + } + + @Override + public void recordVirtualJump(Node node) { + LongAdder counter = virtualJumpCounts.get(node); + if (counter == null) { + return; + } + counter.increment(); + } + + @Override + public void dump(PrintWriter out) { + registeredPrograms.stream() + .sorted(Comparator.comparing(Program::getFilename)) + .forEachOrdered(program -> { + CoverageNodeVisitor visitor = new CoverageNodeVisitor(program); + visitor.visitAll(program.getResolvedFunction().getBody()); + visitor.dump(out); + }); + out.close(); + } + + class CoverageNodeVisitor extends CoverageVisitor { + + private final String filename; + private final List functions = new ArrayList<>(); + private final List branches = new ArrayList<>(); + private final Map lines = new HashMap<>(); + + CoverageNodeVisitor(Program program) { + filename = program.getFilename(); + } + + @Override + protected void visitFunction(String identifier, Node defStatement, Node firstBodyStatement) { + functions.add(new FunctionInfo(identifier, defStatement.getStartLocation().line(), + counts.get(firstBodyStatement).sum())); + } + + @Override + protected void visitBranch(Node owner, Node condition, Node positiveUniqueSuccessor, + @Nullable Node negativeUniqueSuccessor) { + int ownerLine = owner.getStartLocation().line(); + if (counts.get(condition).sum() == 0) { + // The branch condition has never been executed. + branches.add(new BranchInfo(ownerLine, null, null)); + } else { + branches.add(new BranchInfo(ownerLine, + counts.get(positiveUniqueSuccessor).sum(), + negativeUniqueSuccessor != null + ? counts.get(negativeUniqueSuccessor).sum() + : virtualJumpCounts.get(owner).sum())); + } + } + + @Override + protected void visitCode(Node node) { + // Update the coverage count for the lines spanned by this node. This is correct since the + // CoverageVisitor visits nodes from outermost to innermost lexical scope. + linesToMarkCovered(node).forEach(line -> lines.put(line, counts.get(node).sum())); + } + + void dump(PrintWriter out) { + out.println(String.format("SF:%s", filename)); + + List sortedFunctions = functions.stream() + .sorted(Comparator.comparingInt(fi -> fi.line) + .thenComparing(fi -> fi.identifier)) + .collect(Collectors.toList()); + for (FunctionInfo info : sortedFunctions) { + out.println(String.format("FN:%d,%s", info.line, info.identifier)); + } + int numExecutedFunctions = 0; + for (FunctionInfo info : sortedFunctions) { + if (info.count > 0) { + numExecutedFunctions++; + } + out.println(String.format("FNDA:%d,%s", info.count, info.identifier)); + } + out.println(String.format("FNF:%d", functions.size())); + out.println(String.format("FNH:%d", numExecutedFunctions)); + + branches.sort(Comparator.comparing(lc -> lc.ownerLine)); + int numExecutedBranches = 0; + for (int id = 0; id < branches.size(); id++) { + BranchInfo info = branches.get(id); + if (info.positiveCount != null && info.positiveCount > 0) { + numExecutedBranches++; + } + if (info.negativeCount != null && info.negativeCount > 0) { + numExecutedBranches++; + } + // By assigning the same block id to both branches, the coverage viewer will know to group + // them together. + out.println(String.format("BRDA:%d,%d,%d,%s", + info.ownerLine, + id, + 0, + info.positiveCount == null ? "-" : info.positiveCount)); + out.println(String.format("BRDA:%d,%d,%d,%s", + info.ownerLine, + id, + 1, + info.negativeCount == null ? "-" : info.negativeCount)); + } + out.println(String.format("BRF:%d", branches.size())); + out.println(String.format("BRH:%d", numExecutedBranches)); + + List sortedLines = lines.keySet().stream().sorted().collect(Collectors.toList()); + int numExecutedLines = 0; + for (int line : sortedLines) { + long count = lines.get(line); + if (count > 0) { + numExecutedLines++; + } + out.println(String.format("DA:%d,%d", line, count)); + } + out.println(String.format("LF:%d", lines.size())); + out.println(String.format("LH:%d", numExecutedLines)); + + out.println("end_of_record"); + } + + /** + * Given a node in the AST, returns an {@link IntStream} that yields all source file lines to + * which the coverage information of {@code node} should be propagated. + *

+ * This usually returns all lines between the start and end location of {@code node}, but may + * return fewer lines for block statements such as {@code if}. + */ + private IntStream linesToMarkCovered(Node node) { + if (!(node instanceof Statement)) { + return IntStream.rangeClosed(node.getStartLocation().line(), node.getEndLocation().line()); + } + // Handle block statements specially so that they don't mark their entire scope as covered, + // which would also include comments and empty lines. + switch (((Statement) node).kind()) { + case IF: + return IntStream.rangeClosed(node.getStartLocation().line(), + ((IfStatement) node).getCondition().getEndLocation().line()); + case FOR: + return IntStream.rangeClosed(node.getStartLocation().line(), + ((ForStatement) node).getCollection().getEndLocation().line()); + case DEF: + DefStatement defStatement = (DefStatement) node; + if (defStatement.getParameters().isEmpty()) { + return IntStream.of(node.getStartLocation().line()); + } + Parameter lastParam = defStatement.getParameters() + .get(defStatement.getParameters().size() - 1); + return IntStream.rangeClosed(defStatement.getStartLocation().line(), + lastParam.getEndLocation().line()); + default: + return IntStream.rangeClosed(node.getStartLocation().line(), + node.getEndLocation().line()); + } + } + + private class FunctionInfo { + + final String identifier; + final int line; + final long count; + + FunctionInfo(String identifier, int line, long count) { + this.identifier = identifier; + this.line = line; + this.count = count; + } + } + + private class BranchInfo { + + final int ownerLine; + // Both positiveCount and negativeCount are null if the branch condition hasn't been executed. + // Otherwise, they give the number of times the positive case jump resp. the negative case + // jump was taken (and are in particular not null). + final Long positiveCount; + final Long negativeCount; + + BranchInfo(int ownerLine, @Nullable Long positiveCount, @Nullable Long negativeCount) { + this.ownerLine = ownerLine; + this.positiveCount = positiveCount; + this.negativeCount = negativeCount; + } + } + } +} diff --git a/src/main/java/net/starlark/java/syntax/CoverageVisitor.java b/src/main/java/net/starlark/java/syntax/CoverageVisitor.java new file mode 100644 index 00000000000000..e22312c3388b24 --- /dev/null +++ b/src/main/java/net/starlark/java/syntax/CoverageVisitor.java @@ -0,0 +1,331 @@ +// Copyright 2022 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 net.starlark.java.syntax; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; +import javax.annotation.Nullable; + +abstract class CoverageVisitor extends NodeVisitor { + + private static class FunctionFrame { + + final String name; + int lambdaCount; + + FunctionFrame(String name) { + this.name = name; + this.lambdaCount = 0; + } + } + + private final List functionStack = new ArrayList<>(); + + CoverageVisitor() { + functionStack.add(new FunctionFrame("")); + } + + /** + * Called for every (possibly nested, possibly lambda) function. + * + * @param identifier a human-readable identifier for the function that is unique within + * the entire source file + * @param defStatement the {@link Node} representing the function definition + * @param firstBodyStatement the {@link Node} representing the first statement of the function's + * body, which can be used to track how often the function has been + * executed + */ + abstract protected void visitFunction(String identifier, Node defStatement, + Node firstBodyStatement); + + /** + * Called for every conditional jump to either one of two successor nodes depending on a + * condition. + *

+ * Note: Any conditional branch in Starlark always has two successors, never more. + * + * @param owner the {@code Node} at whose location the branch should be + * reported + * @param condition the {@code Node} representing the branch condition + * @param positiveUniqueSuccessor a {@code Node} that is executed if and only if the "positive" + * branch has been taken (e.g., the if condition was satisfied or + * the iterable in a for loop has more elements). The node must not + * be executed in any other situation. + * @param negativeUniqueSuccessor a {@code Node} that is executed if and only if the "negative" + * branch has been taken (e.g., the if condition was not satisfied + * or the iterable in a for loop contains no more elements). The + * node must not be executed in any other situation. May be + * {@code null}, in which case the branch has to be marked as + * executed manually via a call to + * {@link CoverageRecorder#recordVirtualJump(Node)} with the + * argument {@code owner}. + */ + abstract protected void visitBranch(Node owner, Node condition, Node positiveUniqueSuccessor, + @Nullable Node negativeUniqueSuccessor); + + /** + * Called for every {@code Node} that corresponds to executable code. If node A contains node B in + * its lexical scope, then {@code visitCode(A)} is called before {@code visitCode(B)}. + */ + abstract protected void visitCode(Node node); + + private String enterFunction(Identifier identifier) { + String name = identifier != null + ? identifier.getName() + : "lambda " + functionStack.get(functionStack.size() - 1).lambdaCount++; + functionStack.add(new FunctionFrame(name)); + return functionStack.stream().skip(1).map(f -> f.name).collect(Collectors.joining(" > ")); + } + + private void leaveFunction() { + functionStack.remove(functionStack.size() - 1); + } + + @Override + final public void visit(Argument node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(Parameter node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(@Nullable Identifier node) { + // node can be null in the case of an anonymous vararg parameter, e.g.: + // def f(..., *, ...): ... + if (node != null) { + visitCode(node); + } + super.visit(node); + } + + @Override + final public void visit(BinaryOperatorExpression node) { + visitCode(node); + if (node.getOperator() == TokenKind.AND || node.getOperator() == TokenKind.OR) { + // Manually track the short-circuit case. + visitBranch(node, node.getX(), node.getY(), null); + } + super.visit(node); + } + + @Override + final public void visit(CallExpression node) { + visitCode(node); + super.visit(node); + } + + private Node getClauseCondition(Node clause) { + return clause instanceof Comprehension.If + ? ((Comprehension.If) clause).getCondition() + : ((Comprehension.For) clause).getIterable(); + } + + private void visitClauseBranches(Node clause, Node successor) { + Node condition = getClauseCondition(clause); + visitBranch(clause, condition, successor, null); + } + + @Override + final public void visit(Comprehension node) { + Comprehension.Clause lastClause = null; + for (Comprehension.Clause clause : node.getClauses()) { + visitCode(clause); + if (lastClause != null) { + visitClauseBranches(lastClause, getClauseCondition(clause)); + } + lastClause = clause; + visit(clause); + } + if (lastClause != null) { + visitClauseBranches(lastClause, node.getBody()); + } + visit(node.getBody()); + } + + @Override + final public void visit(Comprehension.For node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(Comprehension.If node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(ForStatement node) { + visitCode(node); + visitBranch(node, node.getCollection(), node.getBody().get(0), null); + super.visit(node); + } + + @Override + final public void visit(ListExpression node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(IntLiteral node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(FloatLiteral node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(StringLiteral node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(AssignmentStatement node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(ExpressionStatement node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(IfStatement node) { + visitCode(node); + visitBranch(node, + node.getCondition(), + node.getThenBlock().get(0), + node.getElseBlock() != null ? node.getElseBlock().get(0) : null); + super.visit(node); + } + + @Override + final public void visit(DefStatement node) { + visitCode(node); + visitFunction(enterFunction(node.getIdentifier()), node, node.getBody().get(0)); + super.visit(node); + leaveFunction(); + } + + @Override + final public void visit(ReturnStatement node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(FlowStatement node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(DictExpression node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(DictExpression.Entry node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(UnaryOperatorExpression node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(DotExpression node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(IndexExpression node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(LambdaExpression node) { + visitCode(node); + visitFunction(enterFunction(null), node, node.getBody()); + super.visit(node); + leaveFunction(); + } + + @Override + final public void visit(SliceExpression node) { + visitCode(node); + super.visit(node); + } + + @Override + final public void visit(ConditionalExpression node) { + visitCode(node); + visitBranch(node, node.getCondition(), node.getThenCase(), node.getElseCase()); + super.visit(node); + } + + // The following functions intentionally do not call visitCode as their nodes do not correspond to + // executable code or they already delegate to functions that do. + + @Override + final public void visit(LoadStatement node) { + super.visit(node); + } + + @Override + final public void visit(Comment node) { + super.visit(node); + } + + @Override + final public void visit(Node node) { + super.visit(node); + } + + @Override + final public void visit(StarlarkFile node) { + super.visit(node); + } + + @Override + final public void visitAll(List nodes) { + super.visitAll(nodes); + } + + @Override + final public void visitBlock(List statements) { + super.visitBlock(statements); + } +} diff --git a/src/main/java/net/starlark/java/syntax/Program.java b/src/main/java/net/starlark/java/syntax/Program.java index 1bf6547584f57d..9172957a0007b5 100644 --- a/src/main/java/net/starlark/java/syntax/Program.java +++ b/src/main/java/net/starlark/java/syntax/Program.java @@ -85,7 +85,9 @@ public static Program compileFile(StarlarkFile file, Resolver.Module env) } } - return new Program(file.getResolvedFunction(), loads.build(), loadLocations.build()); + Program program = new Program(file.getResolvedFunction(), loads.build(), loadLocations.build()); + CoverageRecorder.getInstance().register(program); + return program; } /** From b603de0e6f400cfc5800d96507db68d3df646325 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sun, 29 May 2022 19:55:19 +0200 Subject: [PATCH 5/5] Add `--starlark_coverage_report` If the new flag `--starlark_coverage_report` is set to a non-empty value, the Starlark coverage recorder will be enabled for all `.bzl` files under the output base executed during the Blaze command. At the end of the command, the collected coverage data will be emitted to the file path given as the flag argument in LCOV format. --- .../lib/runtime/BlazeCommandDispatcher.java | 19 +++++++++++++++++++ .../lib/runtime/CommonCommandOptions.java | 9 +++++++++ src/main/protobuf/failure_details.proto | 1 + 3 files changed, 29 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index 475850e4f6ac0a..76d28bd5a74c23 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -67,6 +67,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.io.PrintWriter; import java.time.Duration; import java.util.ArrayList; import java.util.List; @@ -371,6 +372,12 @@ private BlazeCommandResult execExclusively( message, FailureDetails.Command.Code.STARLARK_CPU_PROFILING_INITIALIZATION_FAILURE); } } + if (!commonOptions.starlarkCoverageReport.isEmpty()) { + // Record coverage for all `.bzl` files, excluding the built-ins, which don't have paths that + // could be resolved when a human-readable coverage report is generated. + Starlark.startCoverageCollection( + path -> !path.startsWith("/virtual_builtins_bzl") && path.endsWith(".bzl")); + } BlazeCommandResult result = createDetailedCommandResult( @@ -603,6 +610,18 @@ private BlazeCommandResult execExclusively( } } } + if (!commonOptions.starlarkCoverageReport.isEmpty()) { + FileOutputStream out; + try { + out = new FileOutputStream(commonOptions.starlarkCoverageReport); + } catch (IOException ex) { + String message = "Starlark coverage recorder: " + ex.getMessage(); + outErr.printErrLn(message); + return createDetailedCommandResult( + message, FailureDetails.Command.Code.STARLARK_COVERAGE_REPORT_DUMP_FAILURE); + } + Starlark.dumpCoverage(new PrintWriter(out)); + } needToCallAfterCommand = false; return runtime.afterCommand(env, result); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java index 006ba66192ad96..a7415355b7e6c9 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java @@ -293,6 +293,15 @@ public String getTypeDescription() { help = "Writes into the specified file a pprof profile of CPU usage by all Starlark threads.") public String starlarkCpuProfile; + @Option( + name = "starlark_coverage_report", + defaultValue = "", + documentationCategory = OptionDocumentationCategory.LOGGING, + effectTags = {OptionEffectTag.BAZEL_MONITORING}, + help = "Writes into the specified file an LCOV coverage report for all Starlark files " + + "executed for the requested Bazel command.") + public String starlarkCoverageReport; + @Option( name = "record_full_profiler_data", defaultValue = "false", diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index c8a87214765368..6f475f47baf43b 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -533,6 +533,7 @@ message Command { NOT_IN_WORKSPACE = 12 [(metadata) = { exit_code: 2 }]; SPACES_IN_WORKSPACE_PATH = 13 [(metadata) = { exit_code: 36 }]; IN_OUTPUT_DIRECTORY = 14 [(metadata) = { exit_code: 2 }]; + STARLARK_COVERAGE_REPORT_DUMP_FAILURE = 15 [(metadata) = { exit_code: 36 }]; } Code code = 1;