Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude code that follows [DoesNotReturn] from code coverage (per #898) #904

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e915869
Implementing reachability analysis, which will allow instructions tha…
kevin-montrose Jul 15, 2020
8314d66
Do a read-only pass through modules to find "does not return" methods…
kevin-montrose Jul 21, 2020
575edcb
.Resolve() can still fail, just for non-self-inflicted reasons; handl…
kevin-montrose Jul 25, 2020
8524e72
add test sample
MarcoRossignoli Aug 19, 2020
ef9eb95
move Reachability related objects to new file and namespace per https…
kevin-montrose Aug 20, 2020
1ba63a1
Move everything in ReachabilityHelper over to immutable collections (…
kevin-montrose Aug 21, 2020
b6692d9
Implements support for tracing throw exception handlers. This is nec…
kevin-montrose Aug 21, 2020
9660b92
Remove debugging code.
kevin-montrose Aug 21, 2020
8d01c06
While this wouldn't produce an error, it is incorrect to consider thi…
kevin-montrose Aug 21, 2020
5eb5bac
don't do control flow analysis in reachability if there are no [DoesN…
kevin-montrose Aug 21, 2020
eb6cced
formatting
kevin-montrose Aug 21, 2020
dd47112
merge master
kevin-montrose Aug 21, 2020
c98b456
Merge branch 'exclude-unreachable-code' of https://github.com/kevin-m…
kevin-montrose Aug 21, 2020
d953b56
remove some now dead usings, and fix a comment
kevin-montrose Aug 21, 2020
f0f7155
doh, did not notice this example... removing since I've already got t…
kevin-montrose Aug 21, 2020
8e907f2
Put DoesNotReturnAttributes attributes into the appropriate places fo…
kevin-montrose Aug 21, 2020
5a16f1b
document new settings for attributes
kevin-montrose Aug 21, 2020
b636ab4
Update src/coverlet.core/Attributes/DoesNotReturnAttribute.cs
kevin-montrose Aug 22, 2020
5aac031
Update src/coverlet.core/Instrumentation/Instrumenter.cs
kevin-montrose Aug 22, 2020
4fc5202
Add a blank line per https://github.com/coverlet-coverage/coverlet/pu…
kevin-montrose Aug 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions Documentation/GlobalTool.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,27 @@ Arguments:
<ASSEMBLY> 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.
Expand Down
7 changes: 7 additions & 0 deletions Documentation/MSBuildIntegration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ```,```.

Expand Down
1 change: 1 addition & 0 deletions Documentation/VSTestIntegration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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?
```
Expand Down
3 changes: 2 additions & 1 deletion src/coverlet.collector/DataCollection/CoverageWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions src/coverlet.collector/DataCollection/CoverletSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ internal class CoverletSettings
/// </summary>
public bool SkipAutoProps { get; set; }

/// <summary>
/// Attributes that mark methods that never return.
/// </summary>
public string[] DoesNotReturnAttributes { get; set; }

public override string ToString()
{
var builder = new StringBuilder();
Expand All @@ -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<string>()));

return builder.ToString();
}
Expand Down
12 changes: 12 additions & 0 deletions src/coverlet.collector/DataCollection/CoverletSettingsParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public CoverletSettings Parse(XmlElement configurationElement, IEnumerable<strin
coverletSettings.SingleHit = ParseSingleHit(configurationElement);
coverletSettings.IncludeTestAssembly = ParseIncludeTestAssembly(configurationElement);
coverletSettings.SkipAutoProps = ParseSkipAutoProps(configurationElement);
coverletSettings.DoesNotReturnAttributes = ParseDoesNotReturnAttributes(configurationElement);
}

coverletSettings.ReportFormats = ParseReportFormats(configurationElement);
Expand Down Expand Up @@ -218,6 +219,17 @@ private bool ParseSkipAutoProps(XmlElement configurationElement)
return skipAutoProps;
}

/// <summary>
/// Parse attributes that mark methods that do not return.
/// </summary>
/// <param name="configurationElement">Configuration element</param>
/// <returns>DoesNotReturn attributes</returns>
private string[] ParseDoesNotReturnAttributes(XmlElement configurationElement)
{
XmlElement doesNotReturnAttributesElement = configurationElement[CoverletConstants.DoesNotReturnAttributesElementName];
return this.SplitElement(doesNotReturnAttributesElement);
}

/// <summary>
/// Splits a comma separated elements into an array
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/coverlet.collector/Utilities/CoverletConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
}
4 changes: 3 additions & 1 deletion src/coverlet.console/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(() =>
{
Expand All @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions src/coverlet.core/Attributes/DoesNotReturnAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
using System;

namespace Coverlet.Core.Attributes
{
[AttributeUsage(AttributeTargets.Property | AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class)]
internal class DoesNotReturnAttribute : Attribute { }
}
4 changes: 4 additions & 0 deletions src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -124,6 +127,7 @@ public CoveragePrepareResult PrepareModules()
_includeFilters,
_excludedSourceFiles,
_excludeAttributes,
_doesNotReturnAttributes,
_singleHit,
_skipAutoProps,
_logger,
Expand Down
74 changes: 61 additions & 13 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Runtime.CompilerServices;

using Coverlet.Core.Instrumentation.Reachability;
kevin-montrose marked this conversation as resolved.
Show resolved Hide resolved
using Coverlet.Core.Abstractions;
using Coverlet.Core.Attributes;
using Coverlet.Core.Symbols;
Expand Down Expand Up @@ -45,6 +46,8 @@ internal class Instrumenter
private List<string> _branchesInCompiledGeneratedClass;
private List<(MethodDefinition, int)> _excludedMethods;
private List<string> _excludedCompilerGeneratedTypes;
private readonly string[] _doesNotReturnAttributes;
private ReachabilityHelper _reachabilityHelper;

public bool SkipModule { get; set; } = false;

Expand All @@ -55,6 +58,7 @@ public Instrumenter(
string[] includeFilters,
string[] excludedFiles,
string[] excludedAttributes,
string[] doesNotReturnAttributes,
bool singleHit,
bool skipAutoProps,
ILogger logger,
Expand All @@ -68,27 +72,30 @@ public Instrumenter(
_excludeFilters = excludeFilters;
_includeFilters = includeFilters;
_excludedFilesHelper = new ExcludedFilesHelper(excludedFiles, logger);
_excludedAttributes = (excludedAttributes ?? Array.Empty<string>())
// 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<string>()
{
nameof(ExcludeFromCoverageAttribute),
nameof(ExcludeFromCodeCoverageAttribute)
})
.ToArray();
_excludedAttributes = PrepareAttributes(excludedAttributes, nameof(ExcludeFromCoverageAttribute), nameof(ExcludeFromCodeCoverageAttribute));
_singleHit = singleHit;
_isCoreLibrary = Path.GetFileNameWithoutExtension(_module) == "System.Private.CoreLib";
_logger = logger;
_instrumentationHelper = instrumentationHelper;
_fileSystem = fileSystem;
_sourceRootTranslator = sourceRootTranslator;
_cecilSymbolHelper = cecilSymbolHelper;
_doesNotReturnAttributes = PrepareAttributes(doesNotReturnAttributes, nameof(DoesNotReturnAttribute));
_skipAutoProps = skipAutoProps;
}

private static string[] PrepareAttributes(IEnumerable<string> providedAttrs, params string[] defaultAttrs)
{
return
(providedAttrs ?? Array.Empty<string>())
// 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
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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;
Expand Down
Loading