[llvm-cov] Add coverage summary information to the HTML index page.

[rdp]

Summary

Added the information output by "llvm-cov report" to the "llvm-cov show -format html" index.html page. An example of the output can be seen at http://ellcc.org/coverage/

[vsk]

Thanks for the patch! I think this is the right way forward. I'd like to get to the point where the CoverageReport class exposes this functionality for use by createIndexFile, but that's a simple refactoring we can do later.

On a high-level, this patch is missing a lit-style test. Please write a test that generates a coverage index in html format and verifies that its counts are correct. The best way to do this is to copy llvm/test/tools/llvm-cov/style.test, pare it down (remove extraneous RUN/CHECK lines), and add checks to verify the contents of the index.

Also, this diff needs to be run through clang-format to match the llvm style.

My other comments are pretty minor.

SourceCoverageViewHTML.cpp

21

Nitpick: includes should be sorted.

212

We should move the logic for selecting the right CSS away from the logic to print out the header. Can you define something like: std::string getCSSForCoverage(PathToStyle = "", AlternateRowColor = false), and have the callers of emitPrelude pass in the right CSS?

287

I suggest renaming this function to better reflect what it's doing (maybe emitFormattedCount?). Also, parameter names should be capitalized, nullptr is preferred over NULL, and Optional<StringRef> is preferred over default null parameters.

300

Parameter names should be capitalized.

306

IIRC you can just use "%zu" for these and drop the casts. If not, it's OK as-is.

325

Delete.

SourceCoverageViewText.cpp

31

Since it isn't used in this implementation, const coverage::CoverageMapping &Coverage -> const coverage::CoverageMapping &.

[rdp]

Ran clang-format on the files. Made most requested changes. CSS style change still needed.

[rdp]

This is my first time using Phabricator. I hope I updated the review properly.

SourceCoverageViewHTML.cpp

212

I think I understand, but I'm not an HTML/CSS expert. I think I want the CSSForCoverage used for every page, and the AlternateRowColor added for just the index page. Are you suggesting that getCSSForCoverage return the string for the entire CSS?

[vsk]

This is the right way to update a review :).

SourceCoverageViewHTML.cpp

212

We're on the same page: with this change, CSSForCoverage will still be used in each file. I'm suggesting that getCSSForCoverage return either a link to a stylesheet or a full CSS blob. In either case, it should be able to optionally add the CSS you wrote to alternate row colors.

288

I'm curious: does it work to drop the template stuff and pass in a const format_object_base & instead of format_object<T>?

299

Can you make Name a const std::string & or a StringRef?

[MaggieYi]

CodeCoverage.cpp

655

You don’t need to pass the ‘SourceFiles’ as a parameter anymore since you can get them by calling Coverage->getUniqueSourceFiles().

[vsk]

@rdp I noticed that you held off on adding a test with your last update. Let me know if you'd like any help with that -- I can take a shot at it if needed.

CodeCoverage.cpp

655

Actually, I think we still need it here to ensure addCollectedPath() + -filename-equivalence continues working as expected.

[rdp]

The final source changes (I think!). Still need to re-learn lit and do the test case.

[vsk]

SourceCoverageView.h

145

The set of SourceFiles being passed to createIndexFile may not be the same as the Coverage.getUniqueSourceFiles(). Please keep the SourceFiles argument.

SourceCoverageViewHTML.cpp

268

Looks good.

SourceCoverageViewText.cpp

92

Ditto -- formatting-only changes belong in a separate commit.

165

Since this change doesn't affect your patch's functionality, let's do it in a follow-up commit.

[rdp]

Reverted SourceFiles changes. Removed unrelated formatting.

[vsk]

Hi @rdp, I went ahead and threaded information from CoverageReport into the text and html indices (see: r281062). Please take a look and let me know if there's anything you'd like changed.

CodeCoverage.cpp

int CodeCoverageTool::show(int argc, const char **argv,
 
   if (SourceFiles.empty())
     // Get the source files from the function coverage mapping.
     for (StringRef Filename : Coverage->getUniqueSourceFiles())
       SourceFiles.push_back(Filename);
 
   // Create an index out of the source files.
   if (ViewOpts.hasOutputDirectory()) {
-    if (Error E = Printer->createIndexFile(SourceFiles)) {
+    if (Error E = Printer->createIndexFile(*Coverage)) {
       error("Could not create index file!", toString(std::move(E)));
       return 1;
     }
   }
 
   // In -output-dir mode, it's safe to use multiple threads to print files.
   unsigned ThreadCount = 1;
   if (ViewOpts.hasOutputDirectory())

SourceCoverageView.h

public:
   /// \brief Create a file to print a coverage view into.
   virtual Expected<OwnedStream> createViewFile(StringRef Path,
                                                bool InToplevel) = 0;
 
   /// \brief Close a file which has been used to print a coverage view.
   virtual void closeViewFile(OwnedStream OS) = 0;
 
   /// \brief Create an index which lists reports for the given source files.
-  virtual Error createIndexFile(ArrayRef<StringRef> SourceFiles) = 0;
+  virtual Error createIndexFile(const coverage::CoverageMapping &Coverage) = 0;
 
   /// @}
 };
 
 /// \brief A code coverage view of a source file or function.
 ///
 /// A source coverage view and its nested sub-views form a file-oriented
 /// representation of code coverage data. This view can be printed out by a

SourceCoverageViewHTML.h

 /// \brief A coverage printer for html output.
 class CoveragePrinterHTML : public CoveragePrinter {
 public:
   Expected<OwnedStream> createViewFile(StringRef Path,
                                        bool InToplevel) override;
 
   void closeViewFile(OwnedStream OS) override;
 
-  Error createIndexFile(ArrayRef<StringRef> SourceFiles) override;
+  Error createIndexFile(const coverage::CoverageMapping &Coverage) override;
 
   CoveragePrinterHTML(const CoverageViewOptions &Opts)
       : CoveragePrinter(Opts) {}
 };
 
 /// \brief A code coverage view which supports html-based rendering.
 class SourceCoverageViewHTML : public SourceCoverageView {
   void renderViewHeader(raw_ostream &OS) override;

SourceCoverageViewHTML.cpp

 //===- SourceCoverageViewHTML.cpp - A html code coverage view -------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file This file implements the html coverage renderer.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "SourceCoverageViewHTML.h"
+#include "CoverageSummaryInfo.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/Path.h"
 
 using namespace llvm;
 
 namespace {
 
 // Return a string with the special characters in \p Str escaped.
 std::string escape(StringRef Str, const CoverageViewOptions &Opts) {
   std::string Result;
   unsigned ColNum = 0; // Record the column number.

 
 // Create an anchor to \p Link with the label \p Str.
 std::string a(const std::string &Link, const std::string &Str,
               const std::string &TargetType = "href") {
   return "<a " + TargetType + "='" + Link + "'>" + Str + "</a>";
 }
 
 const char *BeginHeader =
     "<head>"
     "<meta name='viewport' content='width=device-width,initial-scale=1'>"
     "<meta charset='UTF-8'>";
 
 const char *CSSForCoverage =
     R"(.red {
   background-color: #FFD0D0;
 }
 .cyan {
std::string getPathToStyle(StringRef ViewPath) {
   std::string PathToStyle = "";
   std::string PathSep = sys::path::get_separator();
   unsigned NumSeps = ViewPath.count(PathSep);
   for (unsigned I = 0, E = NumSeps; I < E; ++I)
     PathToStyle += ".." + PathSep;
   return PathToStyle + "style.css";
 }
 
-void emitPrelude(raw_ostream &OS, const CoverageViewOptions &Opts,
-                 const std::string &PathToStyle = "") {
+static std::string getCSSForCoverage(const CoverageViewOptions &Opts,
+                                     const std::string &PathToStyle = "",
+                                     bool AlternateRowColor = false) {
+  std::string result;
+
+  // Link to a stylesheet if one is available. Otherwise, use the default style.
+  if (PathToStyle.empty()) {
+    result = "<style>";
+    result += CSSForCoverage;
+    result += "</style>";
+  } else {
+    result = "<link rel='stylesheet' type='text/css' href='" +
+             escape(PathToStyle, Opts) + "'>";
+  }
+  if (AlternateRowColor) {
+    result += "<style>"
+              "tr:nth-child(even) {"
+              "  background: #CCC"
+              "}"
+              "tr:nth-child(odd) {"
+              "  background: #FFF"
+              "}"
+              "</style>";
+  }
+
+  return result;
+}
+
+void emitPrelude(raw_ostream &OS, const std::string &CSS,
+                 bool trAlternate = false) {
   OS << "<!doctype html>"
         "<html>"
      << BeginHeader;
 
-  // Link to a stylesheet if one is available. Otherwise, use the default style.
-  if (PathToStyle.empty())
-    OS << "<style>" << CSSForCoverage << "</style>";
-  else
-    OS << "<link rel='stylesheet' type='text/css' href='"
-       << escape(PathToStyle, Opts) << "'>";
-
+  OS << CSS;
   OS << EndHeader << "<body>" << BeginCenteredDiv;
 }
 
 void emitEpilog(raw_ostream &OS) {
   OS << EndCenteredDiv << "</body>"
                           "</html>";
 }
 
 } // anonymous namespace
 
 Expected<CoveragePrinter::OwnedStream>
 CoveragePrinterHTML::createViewFile(StringRef Path, bool InToplevel) {
   auto OSOrErr = createOutputStream(Path, "html", InToplevel);
   if (!OSOrErr)
     return OSOrErr;
 
   OwnedStream OS = std::move(OSOrErr.get());
 
   if (!Opts.hasOutputDirectory()) {
-    emitPrelude(*OS.get(), Opts);
+    emitPrelude(*OS.get(), getCSSForCoverage(Opts));
   } else {
     std::string ViewPath = getOutputPath(Path, "html", InToplevel);
-    emitPrelude(*OS.get(), Opts, getPathToStyle(ViewPath));
+    emitPrelude(*OS.get(), getCSSForCoverage(Opts, getPathToStyle(ViewPath)));
   }
 
   return std::move(OS);
 }
 
 void CoveragePrinterHTML::closeViewFile(OwnedStream OS) {
   emitEpilog(*OS.get());
 }
 
-Error CoveragePrinterHTML::createIndexFile(ArrayRef<StringRef> SourceFiles) {
+/// \brief Return the color which correponds to the coverage
+/// percentage of a certain metric.
+template <typename T>
+static const char *determineCoveragePercentageColor(const T &Info) {
+  if (Info.isFullyCovered())
+    return "green";
+  return Info.getPercentCovered() >= 80.0 ? "yellow" : "red";
+}
+
+void emitFormattedCount(raw_ostream &OSRef, const format_object_base &Value,
+                        const char *Align, const char *Color = NULL) {
+  OSRef << "<td align=\"" << Align << "\"";
+  if (Color) {
+    OSRef << "style=\"color: " << Color << ";\"";
+  }
+  OSRef << ">";
+  OSRef << Value;
+  OSRef << "</td>";
+}
+
+static void showSummary(raw_ostream &OSRef, StringRef Name,
+                        FileCoverageSummary &File) {
+  OSRef << "<tr>";
+  OSRef << tag("td", Name);
+
+  emitFormattedCount(OSRef, format("%u", File.RegionCoverage.NumRegions),
+                     "right");
+  emitFormattedCount(OSRef, format("%u", File.RegionCoverage.NotCovered),
+                     "right",
+                     File.RegionCoverage.isFullyCovered() ? "green" : "red");
+  emitFormattedCount(
+      OSRef, format("%.2f%%", File.RegionCoverage.getPercentCovered()), "right",
+      determineCoveragePercentageColor(File.RegionCoverage));
+  emitFormattedCount(OSRef, format("%u", File.FunctionCoverage.NumFunctions),
+                     "right");
+  emitFormattedCount(OSRef, format("%u", File.FunctionCoverage.NumFunctions -
+                                             File.FunctionCoverage.Executed),
+                     "right");
+  emitFormattedCount(
+      OSRef, format("%.2f%%", File.FunctionCoverage.getPercentCovered()),
+      "right", determineCoveragePercentageColor(File.FunctionCoverage));
+  emitFormattedCount(OSRef, format("%u", File.LineCoverage.NumLines), "right");
+  emitFormattedCount(OSRef, format("%u", File.LineCoverage.NotCovered), "right",
+                     File.LineCoverage.isFullyCovered() ? "green" : "red");
+  emitFormattedCount(
+      OSRef, format("%.2f%%", File.LineCoverage.getPercentCovered()), "right",
+      determineCoveragePercentageColor(File.LineCoverage));
+  OSRef << "</tr>";
+}
+
+Error CoveragePrinterHTML::createIndexFile(
+    const coverage::CoverageMapping &Coverage) {
   auto OSOrErr = createOutputStream("index", "html", /*InToplevel=*/true);
   if (Error E = OSOrErr.takeError())
     return E;
   auto OS = std::move(OSOrErr.get());
   raw_ostream &OSRef = *OS.get();
 
   // Emit a table containing links to reports for each file in the covmapping.
   assert(Opts.hasOutputDirectory() && "No output directory for index file");
-  emitPrelude(OSRef, Opts, getPathToStyle(""));
+  emitPrelude(OSRef, getCSSForCoverage(Opts, getPathToStyle(""), true));
   OSRef << BeginSourceNameDiv << "Index" << EndSourceNameDiv;
   OSRef << BeginTable;
-  for (StringRef SF : SourceFiles) {
+  OSRef << "<tr>";
+  OSRef << tag("th", "Filename") << tag("th", "Regions");
+  OSRef << tag("th", "Missed Regions") << tag("th", "Cover");
+  OSRef << tag("th", "Functions") << tag("th", "Missed Functions");
+  OSRef << tag("th", "Executed") + tag("th", "Lines");
+  OSRef << tag("th", "Missed Lines") + tag("th", "Cover");
+  OSRef << "</tr>";
+
+  FileCoverageSummary Totals("TOTAL");
+  for (StringRef SF : Coverage.getUniqueSourceFiles()) {
+    FileCoverageSummary Summary(SF);
+    for (const auto &F : Coverage.getCoveredFunctions(SF)) {
+      FunctionCoverageSummary Function = FunctionCoverageSummary::get(F);
+      Summary.addFunction(Function);
+      Totals.addFunction(Function);
+    }
+
     std::string LinkText = escape(sys::path::relative_path(SF), Opts);
     std::string LinkTarget =
         escape(getOutputPath(SF, "html", /*InToplevel=*/false), Opts);
-    OSRef << tag("tr", tag("td", tag("pre", a(LinkTarget, LinkText), "code")));
+    showSummary(OSRef, tag("pre", a(LinkTarget, LinkText), "code"), Summary);
   }
+
+  showSummary(OSRef, "TOTAL", Totals);
   OSRef << EndTable;
   emitEpilog(OSRef);
 
   // Emit the default stylesheet.
   auto CSSOrErr = createOutputStream("style", "css", /*InToplevel=*/true);
   if (Error E = CSSOrErr.takeError())
     return E;
 

SourceCoverageViewText.h

 /// \brief A coverage printer for text output.
 class CoveragePrinterText : public CoveragePrinter {
 public:
   Expected<OwnedStream> createViewFile(StringRef Path,
                                        bool InToplevel) override;
 
   void closeViewFile(OwnedStream OS) override;
 
-  Error createIndexFile(ArrayRef<StringRef> SourceFiles) override;
+  Error createIndexFile(const coverage::CoverageMapping &Coverage) override;
 
   CoveragePrinterText(const CoverageViewOptions &Opts)
       : CoveragePrinter(Opts) {}
 };
 
 /// \brief A code coverage view which supports text-based rendering.
 class SourceCoverageViewText : public SourceCoverageView {
   void renderViewHeader(raw_ostream &OS) override;

SourceCoverageViewText.cpp

 CoveragePrinterText::createViewFile(StringRef Path, bool InToplevel) {
   return createOutputStream(Path, "txt", InToplevel);
 }
 
 void CoveragePrinterText::closeViewFile(OwnedStream OS) {
   OS->operator<<('\n');
 }
 
-Error CoveragePrinterText::createIndexFile(ArrayRef<StringRef> SourceFiles) {
+Error CoveragePrinterText::createIndexFile(
+    const coverage::CoverageMapping &Coverage) {
   auto OSOrErr = createOutputStream("index", "txt", /*InToplevel=*/true);
   if (Error E = OSOrErr.takeError())
     return E;
   auto OS = std::move(OSOrErr.get());
   raw_ostream &OSRef = *OS.get();
 
-  for (StringRef SF : SourceFiles)
+  for (StringRef SF : Coverage.getUniqueSourceFiles())
     OSRef << getOutputPath(SF, "txt", /*InToplevel=*/false) << '\n';
 
   return Error::success();
 }
 
 namespace {
 
 static const unsigned LineCoverageColumnWidth = 7;
void SourceCoverageViewText::renderViewDivider(raw_ostream &OS,
   OS.indent(2);
   unsigned Length = getDividerWidth(getOptions());
   for (unsigned I = 0; I < Length; ++I)
     OS << '-';
   OS << '\n';
 }
 
 void SourceCoverageViewText::renderLine(
-    raw_ostream &OS, LineRef L,
-    const coverage::CoverageSegment *WrappedSegment,
+    raw_ostream &OS, LineRef L, const coverage::CoverageSegment *WrappedSegment,
     CoverageSegmentArray Segments, unsigned ExpansionCol, unsigned ViewDepth) {
   StringRef Line = L.Line;
   unsigned LineNumber = L.LineNo;
 
   Optional<raw_ostream::Colors> Highlight;
   SmallVector<std::pair<unsigned, unsigned>, 2> HighlightedRanges;
 
   // The first segment overlaps from a previous line, so we treat it specially.
void SourceCoverageViewText::renderLineNumberColumn(raw_ostream &OS,
   raw_svector_ostream BufferOS(Buffer);
   BufferOS << LineNo;
   auto Str = BufferOS.str();
   // Trim and align to the right.
   Str = Str.substr(0, std::min(Str.size(), (size_t)LineNumberColumnWidth));
   OS.indent(LineNumberColumnWidth - Str.size()) << Str << '|';
 }
 
-void SourceCoverageViewText::renderRegionMarkers(
-    raw_ostream &OS, CoverageSegmentArray Segments, unsigned ViewDepth) {
+void SourceCoverageViewText::renderRegionMarkers(raw_ostream &OS,
+                                                 CoverageSegmentArray Segments,
+                                                 unsigned ViewDepth) {
   renderLinePrefix(OS, ViewDepth);
   OS.indent(getCombinedColumnWidth(getOptions()));
 
   unsigned PrevColumn = 1;
   for (const auto *S : Segments) {
     if (!S->IsRegionEntry)
       continue;
     // Skip to the new region.