diff --git a/Documentation/GlobalTool.md b/Documentation/GlobalTool.md index bb76dc9e5..283afcb6b 100644 --- a/Documentation/GlobalTool.md +++ b/Documentation/GlobalTool.md @@ -17,26 +17,27 @@ Arguments: Path to the test assembly. Options: - -h|--help Show help information - -v|--version Show version information - -t|--target Path to the test runner application. - -a|--targetargs Arguments to be passed to the test runner. - -o|--output Output of the generated coverage report - -v|--verbosity Sets the verbosity level of the command. Allowed values are quiet, minimal, normal, detailed. - -f|--format Format of the generated coverage report[multiple value]. - --threshold Exits with error if the coverage % is below value. - --threshold-type Coverage type to apply the threshold to[multiple value]. - --threshold-stat Coverage statistic used to enforce the threshold value. - --exclude Filter expressions to exclude specific modules and types[multiple value]. - --include Filter expressions to include specific modules and types[multiple value]. - --include-directory Include directories containing additional assemblies to be instrumented[multiple value]. - --exclude-by-file Glob patterns specifying source files to exclude[multiple value]. - --exclude-by-attribute Attributes to exclude from code coverage[multiple value]. - --include-test-assembly Specifies whether to report code coverage of the test assembly. - --single-hit Specifies whether to limit code coverage hit reporting to a single hit for each location. - --merge-with Path to existing coverage result to merge. - --use-source-link Specifies whether to use SourceLink URIs in place of file system paths. - --skipautoprops Neither track nor record auto-implemented properties. + -h|--help Show help information + -v|--version Show version information + -t|--target Path to the test runner application. + -a|--targetargs Arguments to be passed to the test runner. + -o|--output Output of the generated coverage report + -v|--verbosity Sets the verbosity level of the command. Allowed values are quiet, minimal, normal, detailed. + -f|--format Format of the generated coverage report[multiple value]. + --threshold Exits with error if the coverage % is below value. + --threshold-type Coverage type to apply the threshold to[multiple value]. + --threshold-stat Coverage statistic used to enforce the threshold value. + --exclude Filter expressions to exclude specific modules and types[multiple value]. + --include Filter expressions to include specific modules and types[multiple value]. + --include-directory Include directories containing additional assemblies to be instrumented[multiple value]. + --exclude-by-file Glob patterns specifying source files to exclude[multiple value]. + --exclude-by-attribute Attributes to exclude from code coverage[multiple value]. + --include-test-assembly Specifies whether to report code coverage of the test assembly. + --single-hit Specifies whether to limit code coverage hit reporting to a single hit for each location. + --merge-with Path to existing coverage result to merge. + --use-source-link Specifies whether to use SourceLink URIs in place of file system paths. + --skipautoprops Neither track nor record auto-implemented properties. + --does-not-return-attribute Attributes that mark methods that do not return[multiple value]. ``` NB. For a [multiple value] options you have to specify values multiple times i.e. diff --git a/Documentation/MSBuildIntegration.md b/Documentation/MSBuildIntegration.md index cd54a8e9d..7d6631693 100644 --- a/Documentation/MSBuildIntegration.md +++ b/Documentation/MSBuildIntegration.md @@ -162,6 +162,13 @@ You can also include coverage of the test assembly itself by setting `/p:Include Neither track nor record auto-implemented properties. Syntax: `/p:SkipAutoProps=true` +### Methods that do not return + +Methods that do not return can be marked with attributes to cause statements after them to be excluded from coverage. `DoesNotReturnAttribute` is included by default. + +Attributes can be specified with the following syntax. +Syntax: `/p:DoesNotReturnAttribute="DoesNotReturnAttribute,OtherAttribute"` + ### Note for Powershell / VSTS users To exclude or include multiple assemblies when using Powershell scripts or creating a .yaml file for a VSTS build ```%2c``` should be used as a separator. Msbuild will translate this symbol to ```,```. diff --git a/Documentation/VSTestIntegration.md b/Documentation/VSTestIntegration.md index cff5e299a..47fa12f0d 100644 --- a/Documentation/VSTestIntegration.md +++ b/Documentation/VSTestIntegration.md @@ -80,6 +80,7 @@ These are a list of options that are supported by coverlet. These can be specifi |UseSourceLink | Specifies whether to use SourceLink URIs in place of file system paths. | |IncludeTestAssembly | Include coverage of the test assembly. | |SkipAutoProps | Neither track nor record auto-implemented properties. | +|DoesNotReturnAttribute | Methods marked with these attributes are known not to return, statements following them will be excluded from coverage | How to specify these options via runsettings? ``` diff --git a/src/coverlet.collector/DataCollection/CoverageWrapper.cs b/src/coverlet.collector/DataCollection/CoverageWrapper.cs index d99e3bd8d..c56d9982e 100644 --- a/src/coverlet.collector/DataCollection/CoverageWrapper.cs +++ b/src/coverlet.collector/DataCollection/CoverageWrapper.cs @@ -28,7 +28,8 @@ public Coverage CreateCoverage(CoverletSettings settings, ILogger coverletLogger SingleHit = settings.SingleHit, MergeWith = settings.MergeWith, UseSourceLink = settings.UseSourceLink, - SkipAutoProps = settings.SkipAutoProps + SkipAutoProps = settings.SkipAutoProps, + DoesNotReturnAttributes = settings.DoesNotReturnAttributes }; return new Coverage( diff --git a/src/coverlet.collector/DataCollection/CoverletSettings.cs b/src/coverlet.collector/DataCollection/CoverletSettings.cs index eaa1afbb6..85747612b 100644 --- a/src/coverlet.collector/DataCollection/CoverletSettings.cs +++ b/src/coverlet.collector/DataCollection/CoverletSettings.cs @@ -68,6 +68,11 @@ internal class CoverletSettings /// public bool SkipAutoProps { get; set; } + /// + /// Attributes that mark methods that never return. + /// + public string[] DoesNotReturnAttributes { get; set; } + public override string ToString() { var builder = new StringBuilder(); @@ -83,6 +88,7 @@ public override string ToString() builder.AppendFormat("SingleHit: '{0}'", SingleHit); builder.AppendFormat("IncludeTestAssembly: '{0}'", IncludeTestAssembly); builder.AppendFormat("SkipAutoProps: '{0}'", SkipAutoProps); + builder.AppendFormat("DoesNotReturnAttributes: '{0}'", string.Join(",", DoesNotReturnAttributes ?? Enumerable.Empty())); return builder.ToString(); } diff --git a/src/coverlet.collector/DataCollection/CoverletSettingsParser.cs b/src/coverlet.collector/DataCollection/CoverletSettingsParser.cs index ce37c237c..a3b1b1124 100644 --- a/src/coverlet.collector/DataCollection/CoverletSettingsParser.cs +++ b/src/coverlet.collector/DataCollection/CoverletSettingsParser.cs @@ -43,6 +43,7 @@ public CoverletSettings Parse(XmlElement configurationElement, IEnumerable + /// Parse attributes that mark methods that do not return. + /// + /// Configuration element + /// DoesNotReturn attributes + private string[] ParseDoesNotReturnAttributes(XmlElement configurationElement) + { + XmlElement doesNotReturnAttributesElement = configurationElement[CoverletConstants.DoesNotReturnAttributesElementName]; + return this.SplitElement(doesNotReturnAttributesElement); + } + /// /// Splits a comma separated elements into an array /// diff --git a/src/coverlet.collector/Utilities/CoverletConstants.cs b/src/coverlet.collector/Utilities/CoverletConstants.cs index c89846a59..8e4313875 100644 --- a/src/coverlet.collector/Utilities/CoverletConstants.cs +++ b/src/coverlet.collector/Utilities/CoverletConstants.cs @@ -21,5 +21,6 @@ internal static class CoverletConstants public const string DefaultExcludeFilter = "[coverlet.*]*"; public const string InProcDataCollectorName = "CoverletInProcDataCollector"; public const string SkipAutoProps = "SkipAutoProps"; + public const string DoesNotReturnAttributesElementName = "DoesNotReturnAttribute"; } } diff --git a/src/coverlet.console/Program.cs b/src/coverlet.console/Program.cs index 0099b4db0..1b4cbf219 100644 --- a/src/coverlet.console/Program.cs +++ b/src/coverlet.console/Program.cs @@ -63,6 +63,7 @@ static int Main(string[] args) CommandOption skipAutoProp = app.Option("--skipautoprops", "Neither track nor record auto-implemented properties.", CommandOptionType.NoValue); CommandOption mergeWith = app.Option("--merge-with", "Path to existing coverage result to merge.", CommandOptionType.SingleValue); CommandOption useSourceLink = app.Option("--use-source-link", "Specifies whether to use SourceLink URIs in place of file system paths.", CommandOptionType.NoValue); + CommandOption doesNotReturnAttributes = app.Option("--does-not-return-attribute", "Attributes that mark methods that do not return.", CommandOptionType.MultipleValue); app.OnExecute(() => { @@ -89,7 +90,8 @@ static int Main(string[] args) SingleHit = singleHit.HasValue(), MergeWith = mergeWith.Value(), UseSourceLink = useSourceLink.HasValue(), - SkipAutoProps = skipAutoProp.HasValue() + SkipAutoProps = skipAutoProp.HasValue(), + DoesNotReturnAttributes = doesNotReturnAttributes.Values.ToArray() }; Coverage coverage = new Coverage(module.Value, diff --git a/src/coverlet.core/Attributes/DoesNotReturnAttribute.cs b/src/coverlet.core/Attributes/DoesNotReturnAttribute.cs new file mode 100644 index 000000000..a35be1336 --- /dev/null +++ b/src/coverlet.core/Attributes/DoesNotReturnAttribute.cs @@ -0,0 +1,7 @@ +using System; + +namespace Coverlet.Core.Attributes +{ + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class)] + internal class DoesNotReturnAttribute : Attribute { } +} diff --git a/src/coverlet.core/Coverage.cs b/src/coverlet.core/Coverage.cs index 7d6c6695c..b71bb46d8 100644 --- a/src/coverlet.core/Coverage.cs +++ b/src/coverlet.core/Coverage.cs @@ -23,6 +23,7 @@ internal class CoverageParameters public bool SingleHit { get; set; } public string MergeWith { get; set; } public bool UseSourceLink { get; set; } + public string[] DoesNotReturnAttributes { get; set; } public bool SkipAutoProps { get; set; } } @@ -39,6 +40,7 @@ internal class Coverage private bool _singleHit; private string _mergeWith; private bool _useSourceLink; + private string[] _doesNotReturnAttributes; private bool _skipAutoProps; private ILogger _logger; private IInstrumentationHelper _instrumentationHelper; @@ -70,6 +72,7 @@ public Coverage(string module, _singleHit = parameters.SingleHit; _mergeWith = parameters.MergeWith; _useSourceLink = parameters.UseSourceLink; + _doesNotReturnAttributes = parameters.DoesNotReturnAttributes; _logger = logger; _instrumentationHelper = instrumentationHelper; _fileSystem = fileSystem; @@ -124,6 +127,7 @@ public CoveragePrepareResult PrepareModules() _includeFilters, _excludedSourceFiles, _excludeAttributes, + _doesNotReturnAttributes, _singleHit, _skipAutoProps, _logger, diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 6f99eabce..e74dc051d 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Runtime.CompilerServices; +using Coverlet.Core.Instrumentation.Reachability; using Coverlet.Core.Abstractions; using Coverlet.Core.Attributes; using Coverlet.Core.Symbols; @@ -45,6 +46,8 @@ internal class Instrumenter private List _branchesInCompiledGeneratedClass; private List<(MethodDefinition, int)> _excludedMethods; private List _excludedCompilerGeneratedTypes; + private readonly string[] _doesNotReturnAttributes; + private ReachabilityHelper _reachabilityHelper; public bool SkipModule { get; set; } = false; @@ -55,6 +58,7 @@ public Instrumenter( string[] includeFilters, string[] excludedFiles, string[] excludedAttributes, + string[] doesNotReturnAttributes, bool singleHit, bool skipAutoProps, ILogger logger, @@ -68,17 +72,7 @@ public Instrumenter( _excludeFilters = excludeFilters; _includeFilters = includeFilters; _excludedFilesHelper = new ExcludedFilesHelper(excludedFiles, logger); - _excludedAttributes = (excludedAttributes ?? Array.Empty()) - // In case the attribute class ends in "Attribute", but it wasn't specified. - // Both names are included (if it wasn't specified) because the attribute class might not actually end in the prefix. - .SelectMany(a => a.EndsWith("Attribute") ? new[] { a } : new[] { a, $"{a}Attribute" }) - // The default custom attributes used to exclude from coverage. - .Union(new List() - { - nameof(ExcludeFromCoverageAttribute), - nameof(ExcludeFromCodeCoverageAttribute) - }) - .ToArray(); + _excludedAttributes = PrepareAttributes(excludedAttributes, nameof(ExcludeFromCoverageAttribute), nameof(ExcludeFromCodeCoverageAttribute)); _singleHit = singleHit; _isCoreLibrary = Path.GetFileNameWithoutExtension(_module) == "System.Private.CoreLib"; _logger = logger; @@ -86,9 +80,22 @@ public Instrumenter( _fileSystem = fileSystem; _sourceRootTranslator = sourceRootTranslator; _cecilSymbolHelper = cecilSymbolHelper; + _doesNotReturnAttributes = PrepareAttributes(doesNotReturnAttributes, nameof(DoesNotReturnAttribute)); _skipAutoProps = skipAutoProps; } + private static string[] PrepareAttributes(IEnumerable providedAttrs, params string[] defaultAttrs) + { + return + (providedAttrs ?? Array.Empty()) + // In case the attribute class ends in "Attribute", but it wasn't specified. + // Both names are included (if it wasn't specified) because the attribute class might not actually end in the prefix. + .SelectMany(a => a.EndsWith("Attribute") ? new[] { a } : new[] { a, $"{a}Attribute" }) + // The default custom attributes used to exclude from coverage. + .Union(defaultAttrs) + .ToArray(); + } + public bool CanInstrument() { try @@ -183,8 +190,31 @@ private bool Is_System_Threading_Interlocked_CoreLib_Type(TypeDefinition type) return _isCoreLibrary && type.FullName == "System.Threading.Interlocked"; } + // Have to do this before we start writing to a module, as we'll get into file + // locking issues if we do it while writing. + private void CreateReachabilityHelper() + { + using (var stream = _fileSystem.NewFileStream(_module, FileMode.Open, FileAccess.Read)) + using (var resolver = new NetstandardAwareAssemblyResolver(_module, _logger)) + { + resolver.AddSearchDirectory(Path.GetDirectoryName(_module)); + var parameters = new ReaderParameters { ReadSymbols = true, AssemblyResolver = resolver }; + if (_isCoreLibrary) + { + parameters.MetadataImporterProvider = new CoreLibMetadataImporterProvider(); + } + + using (var module = ModuleDefinition.ReadModule(stream, parameters)) + { + _reachabilityHelper = ReachabilityHelper.CreateForModule(module, _doesNotReturnAttributes, _logger); + } + } + } + private void InstrumentModule() { + CreateReachabilityHelper(); + using (var stream = _fileSystem.NewFileStream(_module, FileMode.Open, FileAccess.ReadWrite)) using (var resolver = new NetstandardAwareAssemblyResolver(_module, _logger)) { @@ -509,14 +539,32 @@ private void InstrumentIL(MethodDefinition method) var branchPoints = _cecilSymbolHelper.GetBranchPoints(method); + var unreachableRanges = _reachabilityHelper.FindUnreachableIL(processor.Body.Instructions, processor.Body.ExceptionHandlers); + var currentUnreachableRangeIx = 0; + for (int n = 0; n < count; n++) { var instruction = processor.Body.Instructions[index]; var sequencePoint = method.DebugInformation.GetSequencePoint(instruction); var targetedBranchPoints = branchPoints.Where(p => p.EndOffset == instruction.Offset); - // Check if the instruction is coverable - if (_cecilSymbolHelper.SkipNotCoverableInstruction(method, instruction)) + // make sure we're looking at the correct unreachable range (if any) + var instrOffset = instruction.Offset; + while (currentUnreachableRangeIx < unreachableRanges.Length && instrOffset > unreachableRanges[currentUnreachableRangeIx].EndOffset) + { + currentUnreachableRangeIx++; + } + + // determine if the unreachable + var isUnreachable = false; + if (currentUnreachableRangeIx < unreachableRanges.Length) + { + var range = unreachableRanges[currentUnreachableRangeIx]; + isUnreachable = instrOffset >= range.StartOffset && instrOffset <= range.EndOffset; + } + + // Check is both reachable, _and_ coverable + if (isUnreachable || _cecilSymbolHelper.SkipNotCoverableInstruction(method, instruction)) { index++; continue; diff --git a/src/coverlet.core/Instrumentation/ReachabilityHelper.cs b/src/coverlet.core/Instrumentation/ReachabilityHelper.cs new file mode 100644 index 000000000..4d7474ea6 --- /dev/null +++ b/src/coverlet.core/Instrumentation/ReachabilityHelper.cs @@ -0,0 +1,813 @@ +using Coverlet.Core.Abstractions; +using Mono.Cecil; +using Mono.Cecil.Cil; +using Mono.Collections.Generic; +using System; +using System.Collections.Immutable; +using System.Linq; + +namespace Coverlet.Core.Instrumentation.Reachability +{ + /// + /// Helper for find unreachable IL instructions. + /// + internal class ReachabilityHelper + { + internal readonly struct UnreachableRange + { + /// + /// Offset of first unreachable instruction. + /// + public int StartOffset { get; } + /// + /// Offset of last unreachable instruction. + /// + public int EndOffset { get; } + + public UnreachableRange(int start, int end) + { + StartOffset = start; + EndOffset = end; + } + + public override string ToString() + => $"[IL_{StartOffset:x4}, IL_{EndOffset:x4}]"; + } + + private class BasicBlock + { + /// + /// Offset of the instruction that starts the block + /// + public int StartOffset { get; } + /// + /// Whether it is possible for control to flow past the end of the block, + /// ie. whether it's tail is reachable + /// + public bool TailReachable => UnreachableAfter == null; + /// + /// If control flows to the end of the block, where it can flow to + /// + public ImmutableArray BranchesTo { get; } + + /// + /// If this block contains a call(i) to a method that does not return + /// this will be the first such call. + /// + public Instruction UnreachableAfter { get; } + + /// + /// If an exception is raised in this block, where control might branch to. + /// + /// Note that this can happen even if the block's end is unreachable. + /// + public ImmutableArray ExceptionBranchesTo { get; } + + /// + /// Mutable, records whether control can flow into the block, + /// ie. whether it's head is reachable + /// + public bool HeadReachable { get; set; } + + public BasicBlock(int startOffset, Instruction unreachableAfter, ImmutableArray branchesTo, ImmutableArray exceptionBranchesTo) + { + StartOffset = startOffset; + UnreachableAfter = unreachableAfter; + BranchesTo = branchesTo; + ExceptionBranchesTo = exceptionBranchesTo; + } + + public override string ToString() + => $"{nameof(StartOffset)}=IL_{StartOffset:x4}, {nameof(HeadReachable)}={HeadReachable}, {nameof(TailReachable)}={TailReachable}, {nameof(BranchesTo)}=({string.Join(", ", BranchesTo.Select(b => $"IL_{b:x4}"))}), {nameof(ExceptionBranchesTo)}=({string.Join(", ", ExceptionBranchesTo.Select(b => $"IL_{b:x4}"))}), {nameof(UnreachableAfter)}={(UnreachableAfter != null ? $"IL_{UnreachableAfter:x4}" : "")}"; + } + + /// + /// Represents an Instruction that transitions control flow (ie. branches). + /// + /// This is _different_ from other branch types, like Branch and BranchPoint + /// because it includes unconditional branches too. + /// + private readonly struct BranchInstruction + { + /// + /// Location of the branching instruction + /// + public int Offset { get; } + + /// + /// Returns true if this branch has multiple targets. + /// + public bool HasMultiTargets => _TargetOffset == -1; + + private readonly int _TargetOffset; + + /// + /// Target of the branch, assuming it has a single target. + /// + /// It is illegal to access this if there are multiple targets. + /// + public int TargetOffset + { + get + { + if (HasMultiTargets) + { + throw new InvalidOperationException($"{HasMultiTargets} is true"); + } + + return _TargetOffset; + } + } + + private readonly ImmutableArray _TargetOffsets; + + /// + /// Targets of the branch, assuming it has multiple targets. + /// + /// It is illegal to access this if there is a single target. + /// + public ImmutableArray TargetOffsets + { + get + { + if (!HasMultiTargets) + { + throw new InvalidOperationException($"{HasMultiTargets} is false"); + } + + return _TargetOffsets; + } + } + + public BranchInstruction(int offset, int targetOffset) + { + Offset = offset; + _TargetOffset = targetOffset; + _TargetOffsets = ImmutableArray.Empty; + } + + public BranchInstruction(int offset, ImmutableArray targetOffset) + { + if (targetOffset.Length == 1) + { + throw new ArgumentException("Use single entry constructor for single targets", nameof(targetOffset)); + } + + Offset = offset; + _TargetOffset = -1; + _TargetOffsets = targetOffset; + } + + public override string ToString() + => $"IL_{Offset:x4}: {(HasMultiTargets ? string.Join(", ", TargetOffsets.Select(x => $"IL_{x:x4}")) : $"IL_{TargetOffset:x4}")}"; + } + + /// + /// OpCodes that transfer control code, even if they do not + /// introduce branch points. + /// + private static readonly ImmutableHashSet BRANCH_OPCODES = + ImmutableHashSet.CreateRange( + new[] + { + OpCodes.Beq, + OpCodes.Beq_S, + OpCodes.Bge, + OpCodes.Bge_S, + OpCodes.Bge_Un, + OpCodes.Bge_Un_S, + OpCodes.Bgt, + OpCodes.Bgt_S, + OpCodes.Bgt_Un, + OpCodes.Bgt_Un_S, + OpCodes.Ble, + OpCodes.Ble_S, + OpCodes.Ble_Un, + OpCodes.Ble_Un_S, + OpCodes.Blt, + OpCodes.Blt_S, + OpCodes.Blt_Un, + OpCodes.Blt_Un_S, + OpCodes.Bne_Un, + OpCodes.Bne_Un_S, + OpCodes.Br, + OpCodes.Br_S, + OpCodes.Brfalse, + OpCodes.Brfalse_S, + OpCodes.Brtrue, + OpCodes.Brtrue_S, + OpCodes.Switch, + + // These are forms of Br(_S) that are legal to use to exit + // an exception block + // + // So they're "weird" but not too weird for our purposes + // + // The somewhat nasty subtlety is that, within an exception block, + // it's perfectly legal to replace all normal branches with Leaves + // even if they don't actually exit the block. + OpCodes.Leave, + OpCodes.Leave_S, + + // these implicitly branch at the end of a filter or finally block + // their operands do not encode anything interesting, we have to + // look at exception handlers to figure out where they go to + OpCodes.Endfilter, + OpCodes.Endfinally + } + ); + + /// + /// OpCodes that unconditionally transfer control, so there + /// is not "fall through" branch target. + /// + private static readonly ImmutableHashSet UNCONDITIONAL_BRANCH_OPCODES = + ImmutableHashSet.CreateRange( + new[] + { + OpCodes.Br, + OpCodes.Br_S, + OpCodes.Leave, + OpCodes.Leave_S + } + ); + + private readonly ImmutableHashSet DoesNotReturnMethods; + + private ReachabilityHelper(ImmutableHashSet doesNotReturnMethods) + { + DoesNotReturnMethods = doesNotReturnMethods; + } + + /// + /// Build a ReachabilityHelper for the given module. + /// + /// Predetermines methods that will not return, as + /// indicated by the presense of the given attributes. + /// + public static ReachabilityHelper CreateForModule(ModuleDefinition module, string[] doesNotReturnAttributes, ILogger logger) + { + if (doesNotReturnAttributes.Length == 0) + { + return new ReachabilityHelper(ImmutableHashSet.Empty); + } + + var processedMethods = ImmutableHashSet.Empty; + var doNotReturn = ImmutableHashSet.CreateBuilder(); + foreach (var type in module.Types) + { + foreach (var mtd in type.Methods) + { + if (mtd.IsNative) + { + continue; + } + + MethodBody body; + try + { + if (!mtd.HasBody) + { + continue; + } + + body = mtd.Body; + } + catch + { + continue; + } + + foreach (var instr in body.Instructions) + { + if (!IsCall(instr, out var calledMtd)) + { + continue; + } + + var token = calledMtd.MetadataToken; + if (processedMethods.Contains(token)) + { + continue; + } + + processedMethods = processedMethods.Add(token); + + MethodDefinition mtdDef; + try + { + mtdDef = calledMtd.Resolve(); + } + catch + { + logger.LogWarning($"Unable to resolve method reference \"{calledMtd.FullName}\", assuming calls to will return"); + mtdDef = null; + } + + if (mtdDef == null) + { + continue; + } + + if (!mtdDef.HasCustomAttributes) + { + continue; + } + + var hasDoesNotReturnAttribute = false; + foreach (var attr in mtdDef.CustomAttributes) + { + if (Array.IndexOf(doesNotReturnAttributes, attr.AttributeType.Name) != -1) + { + hasDoesNotReturnAttribute = true; + break; + } + } + + if (hasDoesNotReturnAttribute) + { + logger.LogVerbose($"Determined call to \"{calledMtd.FullName}\" will not return"); + doNotReturn.Add(token); + } + } + } + } + + var doNoReturnTokens = doNotReturn.ToImmutable(); + + return new ReachabilityHelper(doNoReturnTokens); + } + + /// + /// Calculates which IL instructions are reachable given an instruction stream and branch points extracted from a method. + /// + /// The algorithm works like so: + /// 1. determine the "blocks" that make up a function + /// * A block starts with either the start of the method, or a branch _target_ + /// * blocks are "where some other code might jump to" + /// * blocks end with either another branch, another branch target, or the end of the method + /// * this means blocks contain no control flow, except (maybe) as the very last instruction + /// 2. blocks have head and tail reachability + /// * a block has head reachablility if some other block that is reachable can branch to it + /// * a block has tail reachability if it contains no calls to methods that never return + /// 4. push the first block onto a stack + /// 5. while the stack is not empty + /// a. pop a block off the stack + /// b. give it head reachability + /// c. if the pop'd block is tail reachable, push the blocks it can branch to onto the stack + /// 6. consider each block + /// * if it is head and tail reachable, all instructions in it are reachable + /// * if it is not head reachable (regardless of tail reachability), no instructions in it are reachable + /// * if it is only head reachable, all instructions up to and including the first call to a method that does not return are reachable + /// + public ImmutableArray FindUnreachableIL(Collection instrs, Collection exceptionHandlers) + { + // no instructions, means nothing to... not reach + if (instrs.Count == 0) + { + return ImmutableArray.Empty; + } + + // no known methods that do not return, so everything is reachable by definition + if (DoesNotReturnMethods.IsEmpty) + { + return ImmutableArray.Empty; + } + + var (mayContainUnreachableCode, branches) = AnalyzeInstructions(instrs, exceptionHandlers); + + // no need to do any more work, nothing unreachable here + if (!mayContainUnreachableCode) + { + return ImmutableArray.Empty; + } + + var lastInstr = instrs[instrs.Count - 1]; + + var blocks = CreateBasicBlocks(instrs, exceptionHandlers, branches); + + DetermineHeadReachability(blocks); + return DetermineUnreachableRanges(blocks, lastInstr.Offset); + } + + /// + /// Analyzes the instructiona and exception handlers provided to find branches and determine if + /// it is possible for their to be unreachable code. + /// + private (bool MayContainUnreachableCode, ImmutableArray Branches) AnalyzeInstructions(Collection instrs, Collection exceptionHandlers) + { + var containsDoesNotReturnCall = false; + + var ret = ImmutableArray.CreateBuilder(); + foreach (var i in instrs) + { + containsDoesNotReturnCall = containsDoesNotReturnCall || DoesNotReturn(i); + + if (BRANCH_OPCODES.Contains(i.OpCode)) + { + var (singleTargetOffset, multiTargetOffsets) = GetInstructionTargets(i, exceptionHandlers); + + if (singleTargetOffset != null) + { + ret.Add(new BranchInstruction(i.Offset, singleTargetOffset.Value)); + } + else + { + ret.Add(new BranchInstruction(i.Offset, multiTargetOffsets)); + } + } + } + + return (containsDoesNotReturnCall, ret.ToImmutable()); + } + + /// + /// For a single instruction, determines all the places it might branch to. + /// + private static (int? SingleTargetOffset, ImmutableArray MultiTargetOffsets) GetInstructionTargets(Instruction i, Collection exceptionHandlers) + { + int? singleTargetOffset; + ImmutableArray multiTargetOffsets; + + if (i.Operand is Instruction[] multiTarget) + { + // it's a switch + singleTargetOffset = null; + + multiTargetOffsets = ImmutableArray.Create(i.Next.Offset); + foreach (var instr in multiTarget) + { + // in practice these are small arrays, so a scan should be fine + if (multiTargetOffsets.Contains(instr.Offset)) + { + continue; + } + + multiTargetOffsets = multiTargetOffsets.Add(instr.Offset); + } + } + else if (i.Operand is Instruction targetInstr) + { + // it's any of the B.*(_S)? or Leave(_S)? instructions + + if (UNCONDITIONAL_BRANCH_OPCODES.Contains(i.OpCode)) + { + multiTargetOffsets = ImmutableArray.Empty; + singleTargetOffset = targetInstr.Offset; + } + else + { + singleTargetOffset = null; + multiTargetOffsets = ImmutableArray.Create(i.Next.Offset, targetInstr.Offset); + } + } + else if (i.OpCode == OpCodes.Endfilter) + { + // Endfilter is always the last instruction in a filter block, and no sort of control + // flow is allowed so we can scan backwards to see find the block + + ExceptionHandler filterForHandler = null; + foreach (var handler in exceptionHandlers) + { + if (handler.FilterStart == null) + { + continue; + } + + var startsAt = handler.FilterStart; + var cur = startsAt; + while (cur != null && cur.Offset < i.Offset) + { + cur = cur.Next; + } + + if (cur != null && cur.Offset == i.Offset) + { + filterForHandler = handler; + break; + } + } + + if (filterForHandler == null) + { + throw new InvalidOperationException($"Could not find ExceptionHandler associated with {i}"); + } + + // filter can do one of two things: + // - branch into handler + // - percolate to another catch block, which might not be in this method + // + // so we chose to model this as an unconditional branch into the handler + singleTargetOffset = filterForHandler.HandlerStart.Offset; + multiTargetOffsets = ImmutableArray.Empty; + } + else if (i.OpCode == OpCodes.Endfinally) + { + // Endfinally is very weird + // + // what it does, effectively is "take whatever branch would normally happen after the instruction + // that left the paired try + // + // practically, this makes endfinally a branch with no target + + singleTargetOffset = null; + multiTargetOffsets = ImmutableArray.Empty; + } + else + { + throw new InvalidOperationException($"Unexpected operand when processing branch {i}"); + } + + return (singleTargetOffset, multiTargetOffsets); + } + + /// + /// Calculates which ranges of IL are unreachable, given blocks which have head and tail reachability calculated. + /// + private ImmutableArray DetermineUnreachableRanges(ImmutableArray blocks, int lastInstructionOffset) + { + var ret = ImmutableArray.CreateBuilder(); + + var endOfMethodOffset = lastInstructionOffset + 1; // add 1 so we point _past_ the end of the method + + for (var curBlockIx = 0; curBlockIx < blocks.Length; curBlockIx++) + { + var curBlock = blocks[curBlockIx]; + + int endOfCurBlockOffset; + if (curBlockIx == blocks.Length - 1) + { + endOfCurBlockOffset = endOfMethodOffset; + } + else + { + endOfCurBlockOffset = blocks[curBlockIx + 1].StartOffset - 1; // minus 1 so we don't include anything of the following block + } + + if (curBlock.HeadReachable) + { + if (curBlock.TailReachable) + { + // it's all reachable + continue; + } + + // tail isn't reachable, which means there's a call to something that doesn't return... + var doesNotReturnInstr = curBlock.UnreachableAfter; + + // and it's everything _after_ the following instruction that is unreachable + // so record the following instruction through the end of the block + var followingInstr = doesNotReturnInstr.Next; + + ret.Add(new UnreachableRange(followingInstr.Offset, endOfCurBlockOffset)); + } + else + { + // none of it is reachable + ret.Add(new UnreachableRange(curBlock.StartOffset, endOfCurBlockOffset)); + } + } + + return ret.ToImmutable(); + } + + /// + /// Process all the blocks and determine if their first instruction is reachable, + /// that is if they have "head reachability". + /// + /// "Tail reachability" will have already been determined in CreateBlocks. + /// + private void DetermineHeadReachability(ImmutableArray blocks) + { + var blockLookup = blocks.ToImmutableDictionary(b => b.StartOffset); + + var headBlock = blockLookup[0]; + + var knownLive = ImmutableStack.Create(headBlock); + + while (!knownLive.IsEmpty) + { + knownLive = knownLive.Pop(out var block); + + if (block.HeadReachable) + { + // already seen this block + continue; + } + + // we can reach this block, clearly + block.HeadReachable = true; + + if (block.TailReachable) + { + // we can reach all the blocks it might flow to + foreach (var reachableOffset in block.BranchesTo) + { + var reachableBlock = blockLookup[reachableOffset]; + knownLive = knownLive.Push(reachableBlock); + } + } + + // if the block is covered by an exception handler, then executing _any_ instruction in it + // could conceivably cause those handlers to be visited + foreach (var exceptionHandlerOffset in block.ExceptionBranchesTo) + { + var reachableHandler = blockLookup[exceptionHandlerOffset]; + knownLive = knownLive.Push(reachableHandler); + } + } + } + + /// + /// Create BasicBlocks from an instruction stream, exception blocks, and branches. + /// + /// Each block starts either at the start of the method, immediately after a branch or at a target for a branch, + /// and ends with another branch, another branch target, or the end of the method. + /// + /// "Tail reachability" is also calculated, which is whether the block can ever actually get past its last instruction. + /// + private ImmutableArray CreateBasicBlocks(Collection instrs, Collection exceptionHandlers, ImmutableArray branches) + { + // every branch-like instruction starts or stops a block + var branchInstrLocs = branches.ToLookup(i => i.Offset); + var branchInstrOffsets = branchInstrLocs.Select(k => k.Key).ToImmutableHashSet(); + + // every target that might be branched to starts or stops a block + var branchTargetOffsetsBuilder = ImmutableHashSet.CreateBuilder(); + foreach (var branch in branches) + { + if (branch.HasMultiTargets) + { + foreach (var target in branch.TargetOffsets) + { + branchTargetOffsetsBuilder.Add(target); + } + } + else + { + branchTargetOffsetsBuilder.Add(branch.TargetOffset); + } + } + + // every exception handler an entry point + // either it's handler, or it's filter (if present) + foreach (var handler in exceptionHandlers) + { + if (handler.FilterStart != null) + { + branchTargetOffsetsBuilder.Add(handler.FilterStart.Offset); + } + else + { + branchTargetOffsetsBuilder.Add(handler.HandlerStart.Offset); + } + } + + var branchTargetOffsets = branchTargetOffsetsBuilder.ToImmutable(); + + // ending the method is also important + var endOfMethodOffset = instrs[instrs.Count - 1].Offset; + + var blocks = ImmutableArray.Empty; + int? blockStartedAt = null; + Instruction unreachableAfter = null; + foreach (var i in instrs) + { + var offset = i.Offset; + var branchesAtLoc = branchInstrLocs[offset]; + + if (blockStartedAt == null) + { + blockStartedAt = offset; + unreachableAfter = null; + } + + var isBranch = branchInstrOffsets.Contains(offset); + var isFollowedByBranchTarget = i.Next != null && branchTargetOffsets.Contains(i.Next.Offset); + var isEndOfMtd = endOfMethodOffset == offset; + + if (unreachableAfter == null && DoesNotReturn(i)) + { + unreachableAfter = i; + } + + var blockEnds = isBranch || isFollowedByBranchTarget || isEndOfMtd; + if (blockEnds) + { + var nextInstr = i.Next; + + // figure out all the different places the basic block could lead to + ImmutableArray goesTo; + if (branchesAtLoc.Any()) + { + // it ends in a branch, where all does it branch? + goesTo = ImmutableArray.Empty; + foreach (var branch in branchesAtLoc) + { + if (branch.HasMultiTargets) + { + goesTo = goesTo.AddRange(branch.TargetOffsets); + } + else + { + goesTo = goesTo.Add(branch.TargetOffset); + } + } + } + else if (nextInstr != null) + { + // it falls throw to another instruction + goesTo = ImmutableArray.Create(nextInstr.Offset); + } + else + { + // it ends the method + goesTo = ImmutableArray.Empty; + } + + var exceptionSwitchesTo = ImmutableArray.Empty; + + // if the block is covered by any exception handlers then + // it is possible that it will branch to its handler block + foreach (var handler in exceptionHandlers) + { + var tryStart = handler.TryStart.Offset; + var tryEnd = handler.TryEnd.Offset; + + var containsStartOfTry = + tryStart >= blockStartedAt.Value && + tryStart <= i.Offset; + + var containsEndOfTry = + tryEnd >= blockStartedAt.Value && + tryEnd <= i.Offset; + + var blockInsideTry = blockStartedAt.Value >= tryStart && i.Offset <= tryEnd; + + // blocks do not necessarily align to the TRY part of exception handlers, so we need to handle three cases: + // - the try _starts_ in the block + // - the try _ends_ in the block + // - the try complete covers the block, but starts and ends before and after it (respectively) + var tryOverlapsBlock = containsStartOfTry || containsEndOfTry || blockInsideTry; + + if (!tryOverlapsBlock) + { + continue; + } + + // if there's a filter, that runs first + if (handler.FilterStart != null) + { + exceptionSwitchesTo = exceptionSwitchesTo.Add(handler.FilterStart.Offset); + } + else + { + // otherwise, go straight to the handler + exceptionSwitchesTo = exceptionSwitchesTo.Add(handler.HandlerStart.Offset); + } + } + + blocks = blocks.Add(new BasicBlock(blockStartedAt.Value, unreachableAfter, goesTo, exceptionSwitchesTo)); + + blockStartedAt = null; + unreachableAfter = null; + } + } + + return blocks; + } + + /// + /// Returns true if the given instruction will never return, + /// and thus subsequent instructions will never be run. + /// + private bool DoesNotReturn(Instruction instr) + { + if (!IsCall(instr, out var mtd)) + { + return false; + } + + return DoesNotReturnMethods.Contains(mtd.MetadataToken); + } + + /// + /// Returns true if the given instruction is a Call or Callvirt. + /// + /// If it is a call, extracts the MethodReference that is being called. + /// + private static bool IsCall(Instruction instr, out MethodReference mtd) + { + var opcode = instr.OpCode; + if (opcode != OpCodes.Call && opcode != OpCodes.Callvirt) + { + mtd = null; + return false; + } + + mtd = (MethodReference)instr.Operand; + + return true; + } + } +} diff --git a/src/coverlet.msbuild.tasks/InstrumentationTask.cs b/src/coverlet.msbuild.tasks/InstrumentationTask.cs index cb2a34784..11bfbc6db 100644 --- a/src/coverlet.msbuild.tasks/InstrumentationTask.cs +++ b/src/coverlet.msbuild.tasks/InstrumentationTask.cs @@ -40,6 +40,8 @@ public class InstrumentationTask : BaseTask public bool SkipAutoProps { get; set; } + public string DoesNotReturnAttribute { get; set; } + [Output] public ITaskItem InstrumenterState { get; set; } @@ -98,7 +100,8 @@ public override bool Execute() SingleHit = SingleHit, MergeWith = MergeWith, UseSourceLink = UseSourceLink, - SkipAutoProps = SkipAutoProps + SkipAutoProps = SkipAutoProps, + DoesNotReturnAttributes = DoesNotReturnAttribute?.Split(',') }; Coverage coverage = new Coverage(Path, diff --git a/src/coverlet.msbuild.tasks/coverlet.msbuild.targets b/src/coverlet.msbuild.tasks/coverlet.msbuild.targets index fefd1cbdb..7d6c3906a 100644 --- a/src/coverlet.msbuild.tasks/coverlet.msbuild.targets +++ b/src/coverlet.msbuild.tasks/coverlet.msbuild.targets @@ -39,7 +39,8 @@ SingleHit="$(SingleHit)" MergeWith="$(MergeWith)" UseSourceLink="$(UseSourceLink)" - SkipAutoProps="$(SkipAutoProps)" > + SkipAutoProps="$(SkipAutoProps)" + DoesNotReturnAttribute="$(DoesNotReturnAttribute)"> diff --git a/test/coverlet.collector.tests/CoverletSettingsParserTests.cs b/test/coverlet.collector.tests/CoverletSettingsParserTests.cs index dd56a6572..9de9993d5 100644 --- a/test/coverlet.collector.tests/CoverletSettingsParserTests.cs +++ b/test/coverlet.collector.tests/CoverletSettingsParserTests.cs @@ -43,23 +43,24 @@ public void ParseShouldSelectFirstTestModuleFromTestModulesList() } [Theory] - [InlineData("[*]*,[coverlet]*", "[coverlet.*.tests?]*,[coverlet.*.tests.*]*", @"E:\temp,/var/tmp", "module1.cs,module2.cs", "Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute")] - [InlineData("[*]*,,[coverlet]*", "[coverlet.*.tests?]*,,[coverlet.*.tests.*]*", @"E:\temp,,/var/tmp", "module1.cs,,module2.cs", "Obsolete,,GeneratedCodeAttribute,,CompilerGeneratedAttribute")] - [InlineData("[*]*, ,[coverlet]*", "[coverlet.*.tests?]*, ,[coverlet.*.tests.*]*", @"E:\temp, ,/var/tmp", "module1.cs, ,module2.cs", "Obsolete, ,GeneratedCodeAttribute, ,CompilerGeneratedAttribute")] - [InlineData("[*]*,\t,[coverlet]*", "[coverlet.*.tests?]*,\t,[coverlet.*.tests.*]*", "E:\\temp,\t,/var/tmp", "module1.cs,\t,module2.cs", "Obsolete,\t,GeneratedCodeAttribute,\t,CompilerGeneratedAttribute")] - [InlineData("[*]*, [coverlet]*", "[coverlet.*.tests?]*, [coverlet.*.tests.*]*", @"E:\temp, /var/tmp", "module1.cs, module2.cs", "Obsolete, GeneratedCodeAttribute, CompilerGeneratedAttribute")] - [InlineData("[*]*,\t[coverlet]*", "[coverlet.*.tests?]*,\t[coverlet.*.tests.*]*", "E:\\temp,\t/var/tmp", "module1.cs,\tmodule2.cs", "Obsolete,\tGeneratedCodeAttribute,\tCompilerGeneratedAttribute")] - [InlineData("[*]*, \t[coverlet]*", "[coverlet.*.tests?]*, \t[coverlet.*.tests.*]*", "E:\\temp, \t/var/tmp", "module1.cs, \tmodule2.cs", "Obsolete, \tGeneratedCodeAttribute, \tCompilerGeneratedAttribute")] - [InlineData("[*]*,\r\n[coverlet]*", "[coverlet.*.tests?]*,\r\n[coverlet.*.tests.*]*", "E:\\temp,\r\n/var/tmp", "module1.cs,\r\nmodule2.cs", "Obsolete,\r\nGeneratedCodeAttribute,\r\nCompilerGeneratedAttribute")] - [InlineData("[*]*, \r\n [coverlet]*", "[coverlet.*.tests?]*, \r\n [coverlet.*.tests.*]*", "E:\\temp, \r\n /var/tmp", "module1.cs, \r\n module2.cs", "Obsolete, \r\n GeneratedCodeAttribute, \r\n CompilerGeneratedAttribute")] - [InlineData("[*]*,\t\r\n\t[coverlet]*", "[coverlet.*.tests?]*,\t\r\n\t[coverlet.*.tests.*]*", "E:\\temp,\t\r\n\t/var/tmp", "module1.cs,\t\r\n\tmodule2.cs", "Obsolete,\t\r\n\tGeneratedCodeAttribute,\t\r\n\tCompilerGeneratedAttribute")] - [InlineData("[*]*, \t \r\n \t [coverlet]*", "[coverlet.*.tests?]*, \t \r\n \t [coverlet.*.tests.*]*", "E:\\temp, \t \r\n \t /var/tmp", "module1.cs, \t \r\n \t module2.cs", "Obsolete, \t \r\n \t GeneratedCodeAttribute, \t \r\n \t CompilerGeneratedAttribute")] - [InlineData(" [*]* , [coverlet]* ", " [coverlet.*.tests?]* , [coverlet.*.tests.*]* ", " E:\\temp , /var/tmp ", " module1.cs , module2.cs ", " Obsolete , GeneratedCodeAttribute , CompilerGeneratedAttribute ")] + [InlineData("[*]*,[coverlet]*", "[coverlet.*.tests?]*,[coverlet.*.tests.*]*", @"E:\temp,/var/tmp", "module1.cs,module2.cs", "Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute", "DoesNotReturnAttribute,ThrowsAttribute")] + [InlineData("[*]*,,[coverlet]*", "[coverlet.*.tests?]*,,[coverlet.*.tests.*]*", @"E:\temp,,/var/tmp", "module1.cs,,module2.cs", "Obsolete,,GeneratedCodeAttribute,,CompilerGeneratedAttribute", "DoesNotReturnAttribute,,ThrowsAttribute")] + [InlineData("[*]*, ,[coverlet]*", "[coverlet.*.tests?]*, ,[coverlet.*.tests.*]*", @"E:\temp, ,/var/tmp", "module1.cs, ,module2.cs", "Obsolete, ,GeneratedCodeAttribute, ,CompilerGeneratedAttribute", "DoesNotReturnAttribute, ,ThrowsAttribute")] + [InlineData("[*]*,\t,[coverlet]*", "[coverlet.*.tests?]*,\t,[coverlet.*.tests.*]*", "E:\\temp,\t,/var/tmp", "module1.cs,\t,module2.cs", "Obsolete,\t,GeneratedCodeAttribute,\t,CompilerGeneratedAttribute", "DoesNotReturnAttribute,\t,ThrowsAttribute")] + [InlineData("[*]*, [coverlet]*", "[coverlet.*.tests?]*, [coverlet.*.tests.*]*", @"E:\temp, /var/tmp", "module1.cs, module2.cs", "Obsolete, GeneratedCodeAttribute, CompilerGeneratedAttribute", "DoesNotReturnAttribute, ThrowsAttribute")] + [InlineData("[*]*,\t[coverlet]*", "[coverlet.*.tests?]*,\t[coverlet.*.tests.*]*", "E:\\temp,\t/var/tmp", "module1.cs,\tmodule2.cs", "Obsolete,\tGeneratedCodeAttribute,\tCompilerGeneratedAttribute", "DoesNotReturnAttribute,\tThrowsAttribute")] + [InlineData("[*]*, \t[coverlet]*", "[coverlet.*.tests?]*, \t[coverlet.*.tests.*]*", "E:\\temp, \t/var/tmp", "module1.cs, \tmodule2.cs", "Obsolete, \tGeneratedCodeAttribute, \tCompilerGeneratedAttribute", "DoesNotReturnAttribute, \tThrowsAttribute")] + [InlineData("[*]*,\r\n[coverlet]*", "[coverlet.*.tests?]*,\r\n[coverlet.*.tests.*]*", "E:\\temp,\r\n/var/tmp", "module1.cs,\r\nmodule2.cs", "Obsolete,\r\nGeneratedCodeAttribute,\r\nCompilerGeneratedAttribute", "DoesNotReturnAttribute,\r\nThrowsAttribute")] + [InlineData("[*]*, \r\n [coverlet]*", "[coverlet.*.tests?]*, \r\n [coverlet.*.tests.*]*", "E:\\temp, \r\n /var/tmp", "module1.cs, \r\n module2.cs", "Obsolete, \r\n GeneratedCodeAttribute, \r\n CompilerGeneratedAttribute", "DoesNotReturnAttribute, \r\n ThrowsAttribute")] + [InlineData("[*]*,\t\r\n\t[coverlet]*", "[coverlet.*.tests?]*,\t\r\n\t[coverlet.*.tests.*]*", "E:\\temp,\t\r\n\t/var/tmp", "module1.cs,\t\r\n\tmodule2.cs", "Obsolete,\t\r\n\tGeneratedCodeAttribute,\t\r\n\tCompilerGeneratedAttribute", "DoesNotReturnAttribute,\t\r\n\tThrowsAttribute")] + [InlineData("[*]*, \t \r\n \t [coverlet]*", "[coverlet.*.tests?]*, \t \r\n \t [coverlet.*.tests.*]*", "E:\\temp, \t \r\n \t /var/tmp", "module1.cs, \t \r\n \t module2.cs", "Obsolete, \t \r\n \t GeneratedCodeAttribute, \t \r\n \t CompilerGeneratedAttribute", "DoesNotReturnAttribute, \t \r\n \t ThrowsAttribute")] + [InlineData(" [*]* , [coverlet]* ", " [coverlet.*.tests?]* , [coverlet.*.tests.*]* ", " E:\\temp , /var/tmp ", " module1.cs , module2.cs ", " Obsolete , GeneratedCodeAttribute , CompilerGeneratedAttribute ", "DoesNotReturnAttribute , ThrowsAttribute")] public void ParseShouldCorrectlyParseConfigurationElement(string includeFilters, string excludeFilters, string includeDirectories, string excludeSourceFiles, - string excludeAttributes) + string excludeAttributes, + string doesNotReturnAttributes) { var testModules = new List { "abc.dll" }; var doc = new XmlDocument(); @@ -74,6 +75,7 @@ public void ParseShouldCorrectlyParseConfigurationElement(string includeFilters, this.CreateCoverletNodes(doc, configElement, CoverletConstants.SingleHitElementName, "true"); this.CreateCoverletNodes(doc, configElement, CoverletConstants.IncludeTestAssemblyElementName, "true"); this.CreateCoverletNodes(doc, configElement, CoverletConstants.SkipAutoProps, "true"); + this.CreateCoverletNodes(doc, configElement, CoverletConstants.DoesNotReturnAttributesElementName, doesNotReturnAttributes); CoverletSettings coverletSettings = _coverletSettingsParser.Parse(configElement, testModules); @@ -91,6 +93,8 @@ public void ParseShouldCorrectlyParseConfigurationElement(string includeFilters, Assert.Equal("[coverlet.*]*", coverletSettings.ExcludeFilters[0]); Assert.Equal("[coverlet.*.tests?]*", coverletSettings.ExcludeFilters[1]); Assert.Equal("[coverlet.*.tests.*]*", coverletSettings.ExcludeFilters[2]); + Assert.Equal("DoesNotReturnAttribute", coverletSettings.DoesNotReturnAttributes[0]); + Assert.Equal("ThrowsAttribute", coverletSettings.DoesNotReturnAttributes[1]); Assert.False(coverletSettings.UseSourceLink); Assert.True(coverletSettings.SingleHit); diff --git a/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs b/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs index 3188d6e91..e05c782b1 100644 --- a/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs +++ b/test/coverlet.core.tests/Coverage/InstrumenterHelper.Assertions.cs @@ -336,9 +336,54 @@ public static Document AssertNonInstrumentedLines(this Document document, BuildC int[] lineRange = Enumerable.Range(from, to - from + 1).ToArray(); - if (document.Lines.Select(l => l.Value.Number).Intersect(lineRange).Count() > 0) + return AssertNonInstrumentedLines(document, configuration, lineRange); + } + + public static Document AssertNonInstrumentedLines(this Document document, BuildConfiguration configuration, params int[] lines) + { + if (document is null) + { + throw new ArgumentNullException(nameof(document)); + } + + BuildConfiguration buildConfiguration = GetAssemblyBuildConfiguration(); + + if ((buildConfiguration & configuration) != buildConfiguration) + { + return document; + } + + var unexpectedlyInstrumented = document.Lines.Select(l => l.Value.Number).Intersect(lines); + + if (unexpectedlyInstrumented.Any()) + { + throw new XunitException($"Unexpected instrumented lines, '{string.Join(',', unexpectedlyInstrumented)}'"); + } + + return document; + } + + public static Document AssertInstrumentLines(this Document document, BuildConfiguration configuration, params int[] lines) + { + if (document is null) + { + throw new ArgumentNullException(nameof(document)); + } + + BuildConfiguration buildConfiguration = GetAssemblyBuildConfiguration(); + + if ((buildConfiguration & configuration) != buildConfiguration) + { + return document; + } + + var instrumentedLines = document.Lines.Select(l => l.Value.Number).ToHashSet(); + + var missing = lines.Where(l => !instrumentedLines.Contains(l)); + + if (missing.Any()) { - throw new XunitException($"Unexpected instrumented lines, '{string.Join(',', lineRange)}'"); + throw new XunitException($"Expected lines to be instrumented, '{string.Join(',', missing)}'"); } return document; diff --git a/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs b/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs index 71ef28073..cae916116 100644 --- a/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs +++ b/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs @@ -18,6 +18,7 @@ using Xunit; using Microsoft.Extensions.DependencyModel; using Microsoft.VisualStudio.TestPlatform; +using Coverlet.Core.Tests; namespace Coverlet.Core.Instrumentation.Tests { @@ -77,7 +78,7 @@ public void TestCoreLibInstrumentation() InstrumentationHelper instrumentationHelper = new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), partialMockFileSystem.Object, _mockLogger.Object, sourceRootTranslator); Instrumenter instrumenter = new Instrumenter(Path.Combine(directory.FullName, files[0]), "_coverlet_instrumented", Array.Empty(), Array.Empty(), Array.Empty(), - Array.Empty(), false, false, _mockLogger.Object, instrumentationHelper, partialMockFileSystem.Object, sourceRootTranslator, new CecilSymbolHelper()); + Array.Empty(), Array.Empty(), false, false, _mockLogger.Object, instrumentationHelper, partialMockFileSystem.Object, sourceRootTranslator, new CecilSymbolHelper()); Assert.True(instrumenter.CanInstrument()); InstrumenterResult result = instrumenter.Instrument(); @@ -242,7 +243,7 @@ private InstrumenterTest CreateInstrumentor(bool fakeCoreLibModule = false, stri new InstrumentationHelper(new ProcessExitHandler(), new RetryHelper(), new FileSystem(), new Mock().Object, new SourceRootTranslator(new Mock().Object, new FileSystem())); module = Path.Combine(directory.FullName, destModule); - Instrumenter instrumenter = new Instrumenter(module, identifier, Array.Empty(), Array.Empty(), Array.Empty(), attributesToIgnore, false, false, + Instrumenter instrumenter = new Instrumenter(module, identifier, Array.Empty(), Array.Empty(), Array.Empty(), attributesToIgnore, Array.Empty(), false, false, _mockLogger.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(_mockLogger.Object, new FileSystem()), new CecilSymbolHelper()); return new InstrumenterTest { @@ -420,7 +421,7 @@ public void SkipEmbeddedPpdbWithoutLocalSource() new SourceRootTranslator(xunitDll, new Mock().Object, new FileSystem())); Instrumenter instrumenter = new Instrumenter(xunitDll, "_xunit_instrumented", Array.Empty(), Array.Empty(), Array.Empty(), - Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(xunitDll, loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Array.Empty(), Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(xunitDll, loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); Assert.True(instrumentationHelper.HasPdb(xunitDll, out bool embedded)); Assert.True(embedded); Assert.False(instrumenter.CanInstrument()); @@ -433,7 +434,7 @@ public void SkipEmbeddedPpdbWithoutLocalSource() new SourceRootTranslator(sample, new Mock().Object, new FileSystem())); instrumenter = new Instrumenter(sample, "_coverlet_tests_projectsample_empty", Array.Empty(), Array.Empty(), Array.Empty(), - Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(sample, loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Array.Empty(), Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(sample, loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); Assert.True(instrumentationHelper.HasPdb(sample, out embedded)); Assert.False(embedded); @@ -479,7 +480,8 @@ public void SkipPpdbWithoutLocalSource() string sample = Directory.GetFiles(Path.Combine(Directory.GetCurrentDirectory(), "TestAssets"), dllFileName).First(); var loggerMock = new Mock(); Instrumenter instrumenter = new Instrumenter(sample, "_75d9f96508d74def860a568f426ea4a4_instrumented", Array.Empty(), Array.Empty(), Array.Empty(), - Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, partialMockFileSystem.Object, new SourceRootTranslator(loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Array.Empty(), Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, partialMockFileSystem.Object, new SourceRootTranslator(loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Assert.True(instrumentationHelper.HasPdb(sample, out bool embedded)); Assert.False(embedded); Assert.False(instrumenter.CanInstrument()); @@ -496,7 +498,8 @@ public void TestInstrument_MissingModule() new SourceRootTranslator(new Mock().Object, new FileSystem())); var instrumenter = new Instrumenter("test", "_test_instrumented", Array.Empty(), Array.Empty(), Array.Empty(), - Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Array.Empty(), Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, new FileSystem(), new SourceRootTranslator(loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Assert.False(instrumenter.CanInstrument()); loggerMock.Verify(l => l.LogWarning(It.IsAny())); } @@ -519,7 +522,8 @@ public void TestInstrument_AssemblyMarkedAsExcludeFromCodeCoverage() new SourceRootTranslator(new Mock().Object, new FileSystem())); Instrumenter instrumenter = new Instrumenter(excludedbyattributeDll, "_xunit_excludedbyattribute", Array.Empty(), Array.Empty(), Array.Empty(), - Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, partialMockFileSystem.Object, new SourceRootTranslator(loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + Array.Empty(), Array.Empty(), false, false, loggerMock.Object, instrumentationHelper, partialMockFileSystem.Object, new SourceRootTranslator(loggerMock.Object, new FileSystem()), new CecilSymbolHelper()); + InstrumenterResult result = instrumenter.Instrument(); Assert.Empty(result.Documents); loggerMock.Verify(l => l.LogVerbose(It.IsAny())); @@ -638,5 +642,73 @@ public void TestInstrument_AsyncAwaitInsideMethodWithExcludeAttributeAreExcluded instrumenterTest.Directory.Delete(true); } + + [Fact] + public void TestReachabilityHelper() + { + var allInstrumentableLines = + new[] + { + // Throws + 7, 8, + // NoBranches + 12, 13, 14, 15, 16, + // If + 19, 20, 22, 23, 24, 25, 26, 27, 29, 30, + // Switch + 33, 34, 36, 39, 40, 41, 42, 44, 45, 49, 50, 52, 53, 55, 56, 58, 59, 61, 62, 64, 65, 68, 69, + // Subtle + 72, 73, 75, 78, 79, 80, 82, 83, 86, 87, 88, 91, 92, 95, 96, 98, 99, 101, 102, 103, + // UnreachableBranch + 106, 107, 108, 110, 111, 112, 113, 114, + // ThrowsGeneric + 118, 119, + // CallsGenericMethodDoesNotReturn + 124, 125, 126, 127, 128, + // AlsoThrows + 134, 135, + // CallsGenericClassDoesNotReturn + 140, 141, 142, 143, 144, + // WithLeave + 147, 149, 150, 151, 152, 153, 154, 155, 156, 159, 161, 163, 166, 167, 168, + // FiltersAndFinallies + 171, 173, 174, 175, 176, 177, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 192, 193, 194, 195, 196, 197 + }; + var notReachableLines = + new[] + { + // NoBranches + 15, 16, + // If + 26, 27, + // Switch + 41, 42, + // Subtle + 79, 80, 88, 96, 98, 99, + // UnreachableBranch + 110, 111, 112, 113, 114, + // CallsGenericMethodDoesNotReturn + 127, 128, + // CallsGenericClassDoesNotReturn + 143, 144, + // WithLeave + 163, 164, + // FiltersAndFinallies + 176, 177, 183, 184, 189, 190, 195, 196, 197 + }; + + var expectedToBeInstrumented = allInstrumentableLines.Except(notReachableLines).ToArray(); + + var instrumenterTest = CreateInstrumentor(); + var result = instrumenterTest.Instrumenter.Instrument(); + + var doc = result.Documents.Values.FirstOrDefault(d => Path.GetFileName(d.Path) == "Instrumentation.DoesNotReturn.cs"); + + // check for instrumented lines + doc.AssertNonInstrumentedLines(BuildConfiguration.Debug, notReachableLines); + doc.AssertInstrumentLines(BuildConfiguration.Debug, expectedToBeInstrumented); + + instrumenterTest.Directory.Delete(true); + } } } diff --git a/test/coverlet.core.tests/Samples/Instrumentation.DoesNotReturn.cs b/test/coverlet.core.tests/Samples/Instrumentation.DoesNotReturn.cs new file mode 100644 index 000000000..75512c304 --- /dev/null +++ b/test/coverlet.core.tests/Samples/Instrumentation.DoesNotReturn.cs @@ -0,0 +1,199 @@ +namespace Coverlet.Core.Samples.Tests +{ + public class DoesNotReturn + { + [System.Diagnostics.CodeAnalysis.DoesNotReturn] + public int Throws() + { + throw new System.Exception(); + } + + public void NoBranches() + { + System.Console.WriteLine("Before"); + Throws(); + System.Console.WriteLine("After"); // unreachable + } // unreachable + + public void If() + { + System.Console.WriteLine("In-After"); + + if (System.Console.ReadKey().KeyChar == 'Y') + { + System.Console.WriteLine("In-Before"); + Throws(); + System.Console.WriteLine("In-After"); // unreachable + } // unreachable + + System.Console.WriteLine("Out-After"); + } + + public void Switch() + { + System.Console.WriteLine("Out-Before"); + + switch (System.Console.ReadKey().KeyChar) + { + case 'A': + System.Console.WriteLine("In-Before"); + Throws(); + System.Console.WriteLine("In-After"); // should be unreachable + break; // should be unreachable + case 'B': + System.Console.WriteLine("In-Constant-1"); + break; + + // need a number of additional, in order, branches to get a Switch generated + case 'C': + System.Console.WriteLine("In-Constant-2"); + break; + case 'D': + System.Console.WriteLine("In-Constant-3"); + break; + case 'E': + System.Console.WriteLine("In-Constant-4"); + break; + case 'F': + System.Console.WriteLine("In-Constant-5"); + break; + case 'G': + System.Console.WriteLine("In-Constant-6"); + break; + case 'H': + System.Console.WriteLine("In-Constant-7"); + break; + } + + System.Console.WriteLine("Out-After"); + } + + public void Subtle() + { + var key = System.Console.ReadKey(); + + switch (key.KeyChar) + { + case 'A': + Throws(); + System.Console.WriteLine("In-Constant-1"); // unreachable + goto case 'B'; // unreachable + case 'B': + System.Console.WriteLine("In-Constant-2"); + break; + + case 'C': + System.Console.WriteLine("In-Constant-3"); + Throws(); + goto alwayUnreachable; // unreachable + + case 'D': + System.Console.WriteLine("In-Constant-4"); + goto subtlyReachable; + } + + Throws(); + System.Console.WriteLine("Out-Constant-1"); // unreachable + + alwayUnreachable: // unreachable + System.Console.WriteLine("Out-Constant-2"); // unreachable + + subtlyReachable: + System.Console.WriteLine("Out-Constant-3"); + } + + public void UnreachableBranch() + { + var key = System.Console.ReadKey(); + Throws(); + + if (key.KeyChar == 'A') // unreachable + { // unreachable + System.Console.WriteLine("Constant-1"); // unreachable + } // unreachable + } // unreachable + + [System.Diagnostics.CodeAnalysis.DoesNotReturn] + public void ThrowsGeneric() + { + throw new System.Exception(typeof(T).Name); + } + + + public void CallsGenericMethodDoesNotReturn() + { + System.Console.WriteLine("Constant-1"); + ThrowsGeneric(); + System.Console.WriteLine("Constant-2"); // unreachable + } + + private class GenericClass + { + [System.Diagnostics.CodeAnalysis.DoesNotReturn] + public static void AlsoThrows() + { + throw new System.Exception(typeof(T).Name); + } + } + + public void CallsGenericClassDoesNotReturn() + { + System.Console.WriteLine("Constant-1"); + GenericClass.AlsoThrows(); + System.Console.WriteLine("Constant-2"); // unreachable + } + + public void WithLeave() + { + try + { + System.Console.WriteLine("Constant-1"); + } + catch (System.Exception e) + { + if (e is System.InvalidOperationException) + { + goto endOfMethod; + } + + System.Console.WriteLine("InCatch-1"); + + Throws(); + + System.Console.WriteLine("InCatch-2"); // unreachable + } // unreachable + + endOfMethod: + System.Console.WriteLine("Constant-2"); + } + + public void FiltersAndFinallies() + { + try + { + System.Console.WriteLine("Constant-1"); + Throws(); + System.Console.WriteLine("Constant-2"); //unreachable + } //unreachable + catch (System.InvalidOperationException e) + when (e.Message != null) + { + System.Console.WriteLine("InCatch-1"); + Throws(); + System.Console.WriteLine("InCatch-2"); //unreachable + } //unreachable + catch (System.InvalidOperationException) + { + System.Console.WriteLine("InCatch-3"); + Throws(); + System.Console.WriteLine("InCatch-4"); //unreachable + } //unreachable + finally + { + System.Console.WriteLine("InFinally-1"); + Throws(); + System.Console.WriteLine("InFinally-2"); //unreachable + } //unreachable + } //unreachable + } +}