From d950043abde92f73492d2ca29805ac228c9e2fcc Mon Sep 17 00:00:00 2001
From: Alexander Markov <alexmarkov@google.com>
Date: Mon, 13 Dec 2021 19:47:04 +0000
Subject: [PATCH] [vm] Remove redirecting factories from source coverage report

Redirecting factories are never hit in source coverage report because
front-end replaces calls to redirecting factories with calls to their
targets. This change excludes redirecting factories from source
coverage reports.

TEST=vm/cc/SourceReport_Regress95008_RedirectingFactory
Fixes https://github.com/flutter/flutter/issues/95008

Change-Id: I0f6af291f7ee0c042521c92063092f990426b995
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/223600
Reviewed-by: Liam Appelbe <liama@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
---
 runtime/vm/kernel_loader.cc      |  1 +
 runtime/vm/object.h              |  3 +-
 runtime/vm/source_report.cc      |  2 +-
 runtime/vm/source_report_test.cc | 54 ++++++++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc
index 6236cf3275d..30188c27d13 100644
--- a/runtime/vm/kernel_loader.cc
+++ b/runtime/vm/kernel_loader.cc
@@ -1952,6 +1952,7 @@ void KernelLoader::LoadProcedure(const Library& library,
   }
   function.set_kernel_offset(procedure_offset);
   function.set_is_extension_member(is_extension_member);
+  function.set_is_redirecting_factory(procedure_helper.IsRedirectingFactory());
   if ((library.is_dart_scheme() &&
        H.IsPrivate(procedure_helper.canonical_name_)) ||
       (function.is_static() && (library.ptr() == Library::InternalLibrary()))) {
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index d1fe8448e68..ba6e15dcbc0 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -3800,7 +3800,8 @@ class Function : public Object {
   V(PolymorphicTarget, is_polymorphic_target)                                  \
   V(HasPragma, has_pragma)                                                     \
   V(IsSynthetic, is_synthetic)                                                 \
-  V(IsExtensionMember, is_extension_member)
+  V(IsExtensionMember, is_extension_member)                                    \
+  V(IsRedirectingFactory, is_redirecting_factory)
 // Bit that is updated after function is constructed, has to be updated in
 // concurrent-safe manner.
 #define FOR_EACH_FUNCTION_VOLATILE_KIND_BIT(V) V(Inlinable, is_inlinable)
diff --git a/runtime/vm/source_report.cc b/runtime/vm/source_report.cc
index ebb4788e07e..12f6096d9af 100644
--- a/runtime/vm/source_report.cc
+++ b/runtime/vm/source_report.cc
@@ -105,7 +105,7 @@ bool SourceReport::ShouldSkipFunction(const Function& func) {
       return true;
   }
   if (func.is_abstract() || func.IsImplicitConstructor() ||
-      func.is_synthetic()) {
+      func.is_synthetic() || func.is_redirecting_factory()) {
     return true;
   }
   if (func.IsNonImplicitClosureFunction() &&
diff --git a/runtime/vm/source_report_test.cc b/runtime/vm/source_report_test.cc
index b21c5880bd2..2670ac66bc9 100644
--- a/runtime/vm/source_report_test.cc
+++ b/runtime/vm/source_report_test.cc
@@ -1065,6 +1065,60 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_IssueCov341_LateFinalVars) {
       buffer);
 }
 
+ISOLATE_UNIT_TEST_CASE(SourceReport_Regress95008_RedirectingFactory) {
+  // WARNING: This MUST be big enough for the serialised JSON string.
+  const int kBufferSize = 1024;
+  char buffer[kBufferSize];
+  const char* kScript = R"(
+class A {
+  A();
+  factory A.foo(int i) = B; // LINE_A
+}
+
+class B extends A {
+  int i;
+  B(this.i); // LINE_B
+}
+
+main() {
+  A.foo(42);
+}
+)";
+
+  Library& lib = Library::Handle();
+  lib ^= ExecuteScript(kScript);
+  ASSERT(!lib.IsNull());
+  const Script& script =
+      Script::Handle(lib.LookupScript(String::Handle(String::New("test-lib"))));
+
+  SourceReport report(SourceReport::kCoverage);
+  JSONStream js;
+  report.PrintJSON(&js, script);
+  const char* json_str = js.ToCString();
+  ASSERT(strlen(json_str) < kBufferSize);
+  ElideJSONSubstring("classes", json_str, buffer);
+  ElideJSONSubstring("libraries", buffer, buffer);
+  EXPECT_STREQ(
+      "{\"type\":\"SourceReport\",\"ranges\":["
+
+      // A()
+      "{\"scriptIndex\":0,\"startPos\":13,\"endPos\":16,\"compiled\":true,"
+      "\"coverage\":{\"hits\":[13],\"misses\":[]}},"
+
+      // B()
+      "{\"scriptIndex\":0,\"startPos\":90,\"endPos\":99,\"compiled\":true,"
+      "\"coverage\":{\"hits\":[90],\"misses\":[]}},"
+
+      // main
+      "{\"scriptIndex\":0,\"startPos\":114,\"endPos\":136,\"compiled\":true,"
+      "\"coverage\":{\"hits\":[114,127],\"misses\":[]}}],"
+
+      // Only one script in the script table.
+      "\"scripts\":[{\"type\":\"@Script\",\"fixedId\":true,\"id\":\"\","
+      "\"uri\":\"file:\\/\\/\\/test-lib\",\"_kind\":\"kernel\"}]}",
+      buffer);
+}
+
 #endif  // !PRODUCT
 
 }  // namespace dart