diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 2c701f14f..65074ac49 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -189,6 +189,63 @@ public CoverageResult GetCoverageResult() InstrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier); } + // In case of anonymous delegate compiler generate a custom class and passes it as type.method delegate. + // If in delegate method we've a branches we need to move these to "actual" class/method that use it. + // We search "method" with same "Line" of closure class method and add missing branches to it, + // in this way we correctly report missing branch inside compiled generated anonymous delegate. + List compileGeneratedClassToRemove = null; + foreach (var module in modules) + { + foreach (var document in module.Value) + { + foreach (var @class in document.Value) + { + foreach (var method in @class.Value) + { + foreach (var branch in method.Value.Branches) + { + if (BranchInCompilerGeneratedClass(method.Key)) + { + Method actualMethod = GetMethodWithSameLineInSameDocument(document.Value, @class.Key, branch.Line); + + if (actualMethod is null) + { + continue; + } + + actualMethod.Branches.Add(branch); + + if (compileGeneratedClassToRemove is null) + { + compileGeneratedClassToRemove = new List(); + } + + if (!compileGeneratedClassToRemove.Contains(@class.Key)) + { + compileGeneratedClassToRemove.Add(@class.Key); + } + } + } + } + } + } + } + + // After method/branches analysis of compiled generated class we can remove noise from reports + if (!(compileGeneratedClassToRemove is null)) + { + foreach (var module in modules) + { + foreach (var document in module.Value) + { + foreach (var classToRemove in compileGeneratedClassToRemove) + { + document.Value.Remove(classToRemove); + } + } + } + } + var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results }; if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && File.Exists(_mergeWith)) @@ -200,6 +257,41 @@ public CoverageResult GetCoverageResult() return coverageResult; } + private Method GetMethodWithSameLineInSameDocument(Classes documentClasses, string compilerGeneratedClassName, int branchLine) + { + foreach (var @class in documentClasses) + { + if (@class.Key == compilerGeneratedClassName) + { + continue; + } + + foreach (var method in @class.Value) + { + foreach (var line in method.Value.Lines) + { + if (line.Key == branchLine) + { + return method.Value; + } + } + } + } + return null; + } + + private bool BranchInCompilerGeneratedClass(string methodName) + { + foreach (var instrumentedResult in _results) + { + if (instrumentedResult.BranchesInCompiledGeneratedClass.Contains(methodName)) + { + return true; + } + } + return false; + } + private void CalculateCoverage() { foreach (var result in _results) @@ -221,6 +313,10 @@ private void CalculateCoverage() } } + List<(int docIndex, int line)> zeroHitsLines = new List<(int docIndex, int line)>(); + + var documentsList = result.Documents.Values.ToList(); + using (var fs = new FileStream(result.HitsFilePath, FileMode.Open)) using (var br = new BinaryReader(fs)) { @@ -228,8 +324,6 @@ private void CalculateCoverage() // TODO: hitCandidatesCount should be verified against result.HitCandidates.Count - var documentsList = result.Documents.Values.ToList(); - for (int i = 0; i < hitCandidatesCount; ++i) { var hitLocation = result.HitCandidates[i]; @@ -247,57 +341,33 @@ private void CalculateCoverage() { var line = document.Lines[j]; line.Hits += hits; + + // We register 0 hit lines for later cleanup false positive + if (hits == 0) + { + zeroHitsLines.Add((hitLocation.docIndex, line.Number)); + } } } } } - // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) - // we'll remove all MoveNext() not covered branch - foreach (var document in result.Documents) + // Cleanup nested state machine false positive hits + foreach (var (docIndex, line) in zeroHitsLines) { - List> branchesToRemove = new List>(); - foreach (var branch in document.Value.Branches) + foreach (var lineToCheck in documentsList[docIndex].Lines) { - //if one branch is covered we search the other one only if it's not covered - if (IsAsyncStateMachineMethod(branch.Value.Method) && branch.Value.Hits > 0) + if (lineToCheck.Key == line) { - foreach (var moveNextBranch in document.Value.Branches) - { - if (moveNextBranch.Value.Method == branch.Value.Method && moveNextBranch.Value != branch.Value && moveNextBranch.Value.Hits == 0) - { - branchesToRemove.Add(moveNextBranch); - } - } + lineToCheck.Value.Hits = 0; } } - foreach (var branchToRemove in branchesToRemove) - { - document.Value.Branches.Remove(branchToRemove.Key); - } } InstrumentationHelper.DeleteHitsFile(result.HitsFilePath); } } - private bool IsAsyncStateMachineMethod(string method) - { - if (!method.EndsWith("::MoveNext()")) - { - return false; - } - - foreach (var instrumentationResult in _results) - { - if (instrumentationResult.AsyncMachineStateMethod.Contains(method)) - { - return true; - } - } - return false; - } - private string GetSourceLinkUrl(Dictionary sourceLinkDocuments, string document) { if (sourceLinkDocuments.TryGetValue(document, out string url)) diff --git a/src/coverlet.core/CoverageResult.cs b/src/coverlet.core/CoverageResult.cs index b6c02eade..f5af4fb67 100644 --- a/src/coverlet.core/CoverageResult.cs +++ b/src/coverlet.core/CoverageResult.cs @@ -108,57 +108,6 @@ internal void Merge(Modules modules) } } } - - // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) - // we'll remove all MoveNext() not covered branch - List branchesToRemove = new List(); - foreach (var module in this.Modules) - { - foreach (var document in module.Value) - { - foreach (var @class in document.Value) - { - foreach (var method in @class.Value) - { - foreach (var branch in method.Value.Branches) - { - //if one branch is covered we search the other one only if it's not covered - if (IsAsyncStateMachineMethod(method.Key) && branch.Hits > 0) - { - foreach (var moveNextBranch in method.Value.Branches) - { - if (moveNextBranch != branch && moveNextBranch.Hits == 0) - { - branchesToRemove.Add(moveNextBranch); - } - } - } - } - foreach (var branchToRemove in branchesToRemove) - { - method.Value.Branches.Remove(branchToRemove); - } - } - } - } - } - } - - private bool IsAsyncStateMachineMethod(string method) - { - if (!method.EndsWith("::MoveNext()")) - { - return false; - } - - foreach (var instrumentedResult in InstrumentedResults) - { - if (instrumentedResult.AsyncMachineStateMethod.Contains(method)) - { - return true; - } - } - return false; } public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summary, double threshold, ThresholdTypeFlags thresholdTypes, ThresholdStatistic thresholdStat) diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index db28bc041..b385aa781 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -4,7 +4,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; - +using System.Runtime.CompilerServices; using Coverlet.Core.Attributes; using Coverlet.Core.Helpers; using Coverlet.Core.Logging; @@ -35,7 +35,7 @@ internal class Instrumenter private TypeDefinition _customTrackerTypeDef; private MethodReference _customTrackerRegisterUnloadEventsMethod; private MethodReference _customTrackerRecordHitMethod; - private List _asyncMachineStateMethod; + private List _branchesInCompiledGeneratedClass; public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles, string[] excludedAttributes, bool singleHit, ILogger logger) { @@ -68,7 +68,7 @@ public InstrumenterResult Instrument() InstrumentModule(); - _result.AsyncMachineStateMethod = _asyncMachineStateMethod == null ? Array.Empty() : _asyncMachineStateMethod.ToArray(); + _result.BranchesInCompiledGeneratedClass = _branchesInCompiledGeneratedClass == null ? Array.Empty() : _branchesInCompiledGeneratedClass.ToArray(); return _result; } @@ -336,13 +336,17 @@ private void InstrumentIL(MethodDefinition method) continue; var target = AddInstrumentationCode(method, processor, instruction, _branchTarget); - foreach (var _instruction in processor.Body.Instructions) - ReplaceInstructionTarget(_instruction, instruction, target); - foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers) - ReplaceExceptionHandlerBoundary(handler, instruction, target); + if (!(target is null)) + { + foreach (var _instruction in processor.Body.Instructions) + ReplaceInstructionTarget(_instruction, instruction, target); - index += 2; + foreach (ExceptionHandler handler in processor.Body.ExceptionHandlers) + ReplaceExceptionHandlerBoundary(handler, instruction, target); + + index += 2; + } } index++; @@ -374,6 +378,12 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor processor, Instruction instruction, BranchPoint branchPoint) { + // Skip to instrument async branch of async state machine + if (IsAsyncStateMachineAsyncBranch(method.DeclaringType, method, instruction, processor)) + { + return null; + } + if (!_result.Documents.TryGetValue(branchPoint.Document, out var document)) { document = new Document { Path = branchPoint.Document }; @@ -397,16 +407,16 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor } ); - if (IsAsyncStateMachineBranch(method.DeclaringType, method)) + if (method.DeclaringType.CustomAttributes.Any(x => x.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName)) { - if (_asyncMachineStateMethod == null) + if (_branchesInCompiledGeneratedClass == null) { - _asyncMachineStateMethod = new List(); + _branchesInCompiledGeneratedClass = new List(); } - if (!_asyncMachineStateMethod.Contains(method.FullName)) + if (!_branchesInCompiledGeneratedClass.Contains(method.FullName)) { - _asyncMachineStateMethod.Add(method.FullName); + _branchesInCompiledGeneratedClass.Add(method.FullName); } } } @@ -417,8 +427,29 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count - 1); } - private bool IsAsyncStateMachineBranch(TypeDefinition typeDef, MethodDefinition method) + private bool IsAsyncStateMachineAsyncBranch(TypeDefinition typeDef, MethodDefinition method, Instruction currentInstruction, ILProcessor processor) { + /* + We check if instruction is on async path branch of MoveNext() + + ... + L_0043: call instance bool [System.Runtime]System.Runtime.CompilerServices.TaskAwaiter`1::get_IsCompleted() + L_0048: brtrue.s L_008d + ...enqueue callback that will be awakened + L_006c: leave L_0103 <- method returns + ... + + If code run async callback will be enqued and method returns: + + ... + this.<>t__builder.AwaitUnsafeOnCompleted, DemoAsyncClass.d__3>(ref awaiter, ref d__); + return; + ... + + However we're not interested in this branch check and could lead to uncovered branch in case of sync run + so we skip to instrument async part of async state machine, here there is no interesting code only async callback enqued + */ + if (!method.FullName.EndsWith("::MoveNext()")) { return false; @@ -428,7 +459,30 @@ private bool IsAsyncStateMachineBranch(TypeDefinition typeDef, MethodDefinition { if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine") { - return true; + Instruction startAsyncBranch = null; + Instruction leaveAsyncBranch = null; + foreach (var bodyInstruction in processor.Body.Instructions) + { + if (startAsyncBranch is null && + bodyInstruction.Operand is Mono.Cecil.MethodReference operand && + operand.Name == "get_IsCompleted" && + operand.DeclaringType.FullName.StartsWith("System.Runtime.CompilerServices.TaskAwaiter") && + operand.DeclaringType.Scope.Name == "System.Runtime") + { + startAsyncBranch = bodyInstruction; + } + if (!(startAsyncBranch is null) && bodyInstruction.OpCode == OpCodes.Leave) + { + leaveAsyncBranch = bodyInstruction; + } + if (!(startAsyncBranch is null) && !(leaveAsyncBranch is null)) + { + if (currentInstruction.Offset >= startAsyncBranch.Offset && currentInstruction.Offset <= leaveAsyncBranch.Offset) + { + return true; + } + } + } } } return false; diff --git a/src/coverlet.core/Instrumentation/InstrumenterResult.cs b/src/coverlet.core/Instrumentation/InstrumenterResult.cs index 5c3d374e2..db2760571 100644 --- a/src/coverlet.core/Instrumentation/InstrumenterResult.cs +++ b/src/coverlet.core/Instrumentation/InstrumenterResult.cs @@ -41,7 +41,7 @@ public InstrumenterResult() } public string Module; - public string[] AsyncMachineStateMethod; + public string[] BranchesInCompiledGeneratedClass; public string HitsFilePath; public string ModulePath; public string SourceLink;