Merge pull request #15815 from JohnHamby/InstrumentLambda

Instrument the body of an expression-bodied lambda for dynamic analysis

src/Compilers/CSharp/Portable/Lowering/Instrumentation/DynamicAnalysisInjector.cs

@@ -26,7 +26,6 @@ internal sealed class DynamicAnalysisInjector : CompoundInstrumenter
         private readonly LocalSymbol _methodPayload;
         private readonly DiagnosticBag _diagnostics;
         private readonly DebugDocumentProvider _debugDocumentProvider;
-        private readonly bool _methodHasExplicitBlock;
         private readonly SyntheticBoundNodeFactory _methodBodyFactory;
 
         public static DynamicAnalysisInjector TryCreate(MethodSymbol method, BoundStatement methodBody, SyntheticBoundNodeFactory methodBodyFactory, DiagnosticBag diagnostics, DebugDocumentProvider debugDocumentProvider, Instrumenter previous)
@@ -74,7 +73,6 @@ private DynamicAnalysisInjector(MethodSymbol method, BoundStatement methodBody,
             _methodPayload = methodBodyFactory.SynthesizedLocal(_payloadType, kind: SynthesizedLocalKind.InstrumentationPayload, syntax: methodBody.Syntax);
             _diagnostics = diagnostics;
             _debugDocumentProvider = debugDocumentProvider;
-            _methodHasExplicitBlock = MethodHasExplicitBlock(method);
             _methodBodyFactory = methodBodyFactory;
 
             // The first point indicates entry into the method and has the span of the method definition.
@@ -199,15 +197,7 @@ public override BoundStatement InstrumentContinueStatement(BoundContinueStatemen
 
         public override BoundStatement InstrumentExpressionStatement(BoundExpressionStatement original, BoundStatement rewritten)
         {
-            rewritten = base.InstrumentExpressionStatement(original, rewritten);
-
-            if (!_methodHasExplicitBlock)
-            {
-                // The assignment statement for a property set method defined without a block is compiler generated, but requires instrumentation.
-                return CollectDynamicAnalysis(original, rewritten);
-            }
-
-            return AddDynamicAnalysis(original, rewritten);
+            return AddDynamicAnalysis(original, base.InstrumentExpressionStatement(original, rewritten));
         }
 
         public override BoundStatement InstrumentFieldOrPropertyInitializer(BoundExpressionStatement original, BoundStatement rewritten)
@@ -270,8 +260,10 @@ public override BoundStatement InstrumentReturnStatement(BoundReturnStatement or
             rewritten = base.InstrumentReturnStatement(original, rewritten);
 
             // A synthesized return statement that does not return a value never requires instrumentation.
-            // A property set method defined without a block has such a synthesized return statement.
-            if (!_methodHasExplicitBlock && ((BoundReturnStatement)original).ExpressionOpt != null)
+            // A property set defined without a block has such a synthesized return statement.
+            // A synthesized return statement that does return a value does require instrumentation.
+            // A method, property get, or lambda defined without a block has such a synthesized return statement.
+            if (ReturnsValueWithinExpressionBodiedConstruct(original))
             {
                 // The return statement for value-returning methods defined without a block is compiler generated, but requires instrumentation.
                 return CollectDynamicAnalysis(original, rewritten);
@@ -279,6 +271,25 @@ public override BoundStatement InstrumentReturnStatement(BoundReturnStatement or
 
             return AddDynamicAnalysis(original, rewritten);
         }
+        
+        private static bool ReturnsValueWithinExpressionBodiedConstruct(BoundReturnStatement returnStatement)
+        {
+            if (returnStatement.WasCompilerGenerated &&
+                returnStatement.ExpressionOpt != null &&
+                returnStatement.ExpressionOpt.Syntax != null)
+            {
+                SyntaxKind parentKind = returnStatement.ExpressionOpt.Syntax.Parent.Kind();
+                switch (parentKind)
+                {
+                    case SyntaxKind.ParenthesizedLambdaExpression:
+                    case SyntaxKind.SimpleLambdaExpression:
+                    case SyntaxKind.ArrowExpressionClause:
+                        return true;
+                }
+            }
+
+            return false;
+        }
 
         public override BoundStatement InstrumentSwitchStatement(BoundSwitchStatement original, BoundStatement rewritten)
         {
@@ -411,17 +422,6 @@ private static SyntaxNode SyntaxForSpan(BoundStatement statement)
             return syntaxForSpan;
         }
         
-        private static bool MethodHasExplicitBlock(MethodSymbol method)
-        {
-            SourceMethodSymbol asSourceMethod = method.OriginalDefinition as SourceMethodSymbol;
-            if ((object)asSourceMethod != null)
-            {
-                return asSourceMethod.BodySyntax is BlockSyntax;
-            }
-
-            return false;
-        }
-
         private static MethodSymbol GetCreatePayload(CSharpCompilation compilation, SyntaxNode syntax, DiagnosticBag diagnostics)
         {
             return (MethodSymbol)Binder.GetWellKnownTypeMember(compilation, WellKnownMember.Microsoft_CodeAnalysis_Runtime_Instrumentation__CreatePayload, diagnostics, syntax: syntax);

src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicAnalysisResourceTests.cs

@@ -53,6 +53,8 @@ public static int Wilma
     }
 
     public static int Betty { get; }
+
+    public static int Pebbles { get; set; }
 }
 ";
 
@@ -66,9 +68,9 @@ public void TestSpansPresentInResource()
             var reader = DynamicAnalysisDataReader.TryCreateFromPE(peReader, "<DynamicAnalysisData>");
 
             VerifyDocuments(reader, reader.Documents,
-                @"'C:\myproject\doc1.cs' B2-C1-91-21-17-72-39-D7-D8-C8-AC-3C-09-F6-3C-FF-B7-E5-97-8E (SHA1)");
+                @"'C:\myproject\doc1.cs' FF-9A-1F-F4-03-A5-A1-F7-8D-CD-00-15-67-0E-BA-F7-23-9D-3F-0F (SHA1)");
 
-            Assert.Equal(10, reader.Methods.Length);
+            Assert.Equal(12, reader.Methods.Length);
 
             string[] sourceLines = ExampleSource.Split('\n');
 
@@ -96,7 +98,15 @@ public void TestSpansPresentInResource()
                 new SpanResult(21, 4, 21, 36, "public static int Betty { get; }"),
                 new SpanResult(21, 30, 21, 34, "get"));
 
-            VerifySpans(reader, reader.Methods[6]);
+            VerifySpans(reader, reader.Methods[6], sourceLines,                         // Pebbles get
+                new SpanResult(23, 4, 23, 43, "public static int Pebbles { get; set; }"),
+                new SpanResult(23, 32, 23, 36, "get"));
+
+            VerifySpans(reader, reader.Methods[7], sourceLines,                         // Pebbles set
+                new SpanResult(23, 4, 23, 43, "public static int Pebbles { get; set; }"),
+                new SpanResult(23, 37, 23, 41, "set"));
+
+            VerifySpans(reader, reader.Methods[8]);
         }
 
         [Fact]
@@ -803,6 +813,84 @@ partial struct E
                 new SpanResult(42, 27, 42, 56, "new C(() => { return 1567; })"));
         }
 
+        [Fact]
+        public void TestLocalFunctionWithLambdaSpans()
+        {
+            string source = @"
+using System;
+
+public class C
+{
+    public static void Main()                                   // Method 0
+    {
+        TestMain();
+    }
+
+    static void TestMain()                                      // Method 1
+    {
+        new D().M1();
+    }
+}
+
+public class D
+{
+    public void M1()                                            // Method 3
+    {
+        L1();
+        void L1()
+        {
+            var f = new Func<int>(
+                () => 1
+            );
+
+            f();
+
+            var f1 = new Func<int>(
+                () => { return 2; }
+            );
+
+            var f2 = new Func<int, int>(
+                (x) => x + 3
+            );
+
+            var f3 = new Func<int, int>(
+                x => x + 4
+            );
+        }
+    }
+}
+";
+
+            var c = CreateCompilationWithMscorlib(Parse(source + InstrumentationHelperSource, @"C:\myproject\doc1.cs"));
+            var peImage = c.EmitToArray(EmitOptions.Default.WithInstrumentationKinds(ImmutableArray.Create(InstrumentationKind.TestCoverage)));
+
+            var peReader = new PEReader(peImage);
+            var reader = DynamicAnalysisDataReader.TryCreateFromPE(peReader, "<DynamicAnalysisData>");
+
+            string[] sourceLines = source.Split('\n');
+
+            VerifySpans(reader, reader.Methods[0], sourceLines,
+                new SpanResult(5, 4, 8, 5, "public static void Main()"),
+                new SpanResult(7, 8, 7, 19, "TestMain()"));
+
+            VerifySpans(reader, reader.Methods[1], sourceLines,
+                new SpanResult(10, 4, 13, 5, "static void TestMain()"),
+                new SpanResult(12, 8, 12, 21, "new D().M1()"));
+
+            VerifySpans(reader, reader.Methods[3], sourceLines,
+                new SpanResult(18, 4, 41, 5, "public void M1()"),
+                new SpanResult(20, 8, 20, 13, "L1()"),
+                new SpanResult(24, 22, 24, 23, "1"),
+                new SpanResult(23, 12, 25, 14, "var f = new Func<int>"),
+                new SpanResult(27, 12, 27, 16, "f()"),
+                new SpanResult(30, 24, 30, 33, "return 2"),
+                new SpanResult(29, 12, 31, 14, "var f1 = new Func<int>"),
+                new SpanResult(34, 23, 34, 28, "x + 3"),
+                new SpanResult(33, 12, 35, 14, "var f2 = new Func<int, int>"),
+                new SpanResult(38, 21, 38, 26, "x + 4"),
+                new SpanResult(37, 12, 39, 14, "var f3 = new Func<int, int>"));
+        }
+
         [Fact]
         public void TestDynamicAnalysisResourceMissingWhenInstrumentationFlagIsDisabled()
         {

src/Compilers/CSharp/Test/Emit/Emit/DynamicAnalysis/DynamicInstrumentationTests.cs

@@ -703,13 +703,13 @@ public void ImplicitBlockMethodsCoverage()
 
 public class Program
 {
-    public static void Main(string[] args)
+    public static void Main(string[] args)                                  // Method 1
     {
         TestMain();
         Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
     }
     
-    static void TestMain()
+    static void TestMain()                                                  // Method 2
     {
         int x = Count;
         x += Prop;
@@ -749,6 +749,7 @@ File 1
 True
 True
 True
+True
 Method 3
 File 1
 True
@@ -799,6 +800,101 @@ File 1
             verifier.VerifyDiagnostics();
         }
 
+        [Fact]
+        public void LocalFunctionWithLambdaCoverage()
+        {
+            string source = @"
+using System;
+
+public class Program
+{
+    public static void Main(string[] args)                                  // Method 1
+    {
+        TestMain();
+        Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
+    }
+    
+    static void TestMain()                                                  // Method 2
+    {
+        new D().M1();
+    } 
+}
+
+public class D
+{
+    public void M1()                                                        // Method 4
+    {
+        L1();
+        void L1()
+        {
+            var f = new Func<int>(
+                () => 1
+            );
+
+            var f1 = new Func<int>(
+                () => 2
+            );
+
+            var f2 = new Func<int, int>(
+                (x) => x + 3
+            );
+
+            var f3 = new Func<int, int>(
+                x => x + 4
+            );
+
+            f();
+            f3(2);
+        }
+    }
+
+    // Method 5 is the synthesized instance constructor for D.
+}
+" + InstrumentationHelperSource;
+
+            var checker = new CSharpInstrumentationChecker();
+            checker.Method(1, 1, "public static void Main")
+                .True("TestMain();")
+                .True("Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();");
+            checker.Method(2, 1, "static void TestMain")
+                .True("new D().M1();");
+            checker.Method(4, 1, "public void M1()")
+                .True("L1();")
+                .True("1")
+                .True("var f = new Func<int>")
+                .False("2")
+                .True("var f1 = new Func<int>")
+                .False("x + 3")
+                .True("var f2 = new Func<int, int>")
+                .True("x + 4")
+                .True("var f3 = new Func<int, int>")
+                .True("f();")
+                .True("f3(2);");
+            checker.Method(5, 1, snippet: null, expectBodySpan: false);
+            checker.Method(7, 1)
+                .True()
+                .False()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True()
+                .True();
+
+            CompilationVerifier verifier = CompileAndVerify(source, expectedOutput: checker.ExpectedOutput, options: TestOptions.ReleaseExe);
+            verifier.VerifyDiagnostics();
+            checker.CompleteCheck(verifier.Compilation, source);
+
+            verifier = CompileAndVerify(source, expectedOutput: checker.ExpectedOutput, options: TestOptions.DebugExe);
+            verifier.VerifyDiagnostics();
+            checker.CompleteCheck(verifier.Compilation, source);
+        }
+
         [Fact]
         public void MultipleFilesCoverage()
         {
@@ -1592,18 +1688,18 @@ public void AsyncCoverage()
 
 public class Program
 {
-    public static void Main(string[] args)
+    public static void Main(string[] args)                                  // Method 1
     {
         TestMain();
         Microsoft.CodeAnalysis.Runtime.Instrumentation.FlushPayload();
     }
 
-    static void TestMain()
+    static void TestMain()                                                  // Method 2
     {
         Console.WriteLine(Outer(""Goo"").Result);
     }
 
-    async static Task<string> Outer(string s)
+    async static Task<string> Outer(string s)                               // Method 3
     {
         string s1 = await First(s);
         string s2 = await Second(s);
@@ -1611,7 +1707,7 @@ async static Task<string> Outer(string s)
         return s1 + s2;
     }
 
-    async static Task<string> First(string s)
+    async static Task<string> First(string s)                               // Method 4
     {
         string result = await Second(s) + ""Glue"";
         if (result.Length > 2)
@@ -1620,7 +1716,7 @@ async static Task<string> First(string s)
             return ""Too short"";
     }
 
-    async static Task<string> Second(string s)
+    async static Task<string> Second(string s)                              // Method 5
     {
         string doubled = """";
         if (s.Length > 2)
@@ -1663,6 +1759,7 @@ File 1
 False
 True
 True
+True
 Method 8
 File 1
 True

src/Test/Utilities/Desktop/InstrumentationChecker.cs

@@ -272,18 +272,23 @@ protected BaseInstrumentationChecker()
         /// It will be verified against the start of the actual method source span.
         /// If no snippet is passed in, then snippet validation will be disabled for the whole method (subsequent calls to `True` or `False`).
         /// </param>
-        public MethodChecker Method(int method, int file, string snippet = null)
+        public MethodChecker Method(int method, int file, string snippet = null, bool expectBodySpan = true)
         {
             AddConsoleExpectation($"Method {method}");
             AddConsoleExpectation($"File {file}");
+            var result = new MethodChecker(this, noSnippets: snippet == null);
 
-            if (snippet == null)
+            // Most methods have a span that indicates that the method has been entered.
+            if (expectBodySpan)
             {
-                return new MethodChecker(this, noSnippets: true).True();
+                result = result.True(snippet);
+            }
+
+            if (snippet != null)
+            {
+                _spanExpectations.Add(method, result);
             }
 
-            var result = new MethodChecker(this).True(snippet);
-            _spanExpectations.Add(method, result);
             return result;
         }