Skip to content

Change CecilSymbolHelper to instance class #846

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a6e0687
Merge pull request #1 from tonerdo/master
daveMueller Aug 17, 2019
84c0aff
Merge pull request #2 from tonerdo/master
daveMueller Sep 5, 2019
5b4e316
Merge pull request #3 from tonerdo/master
daveMueller Sep 11, 2019
45075e1
Merge pull request #4 from tonerdo/master
daveMueller Sep 14, 2019
ef7d0b0
Merge pull request #5 from tonerdo/master
daveMueller Sep 17, 2019
a50f143
Merge pull request #6 from tonerdo/master
daveMueller Sep 24, 2019
28e038c
Merge pull request #7 from tonerdo/master
daveMueller Oct 9, 2019
a8198b6
Merge pull request #8 from tonerdo/master
daveMueller Oct 20, 2019
fce0e2d
Merge pull request #9 from tonerdo/master
daveMueller Dec 18, 2019
8b8505b
Merge pull request #10 from tonerdo/master
daveMueller Jan 18, 2020
566262b
Merge pull request #11 from tonerdo/master
daveMueller Jan 21, 2020
eb5969d
Merge pull request #12 from tonerdo/master
daveMueller Jan 26, 2020
a3f4a2f
Merge pull request #13 from tonerdo/master
daveMueller Jan 30, 2020
c6fb0fc
Merge pull request #14 from tonerdo/master
daveMueller Feb 19, 2020
903ccdf
Merge pull request #15 from tonerdo/master
daveMueller Mar 15, 2020
3a4a37b
Merge pull request #16 from tonerdo/master
daveMueller Apr 1, 2020
438132c
Merge pull request #17 from tonerdo/master
daveMueller Apr 18, 2020
54b7e29
Merge pull request #18 from coverlet-coverage/master
daveMueller May 3, 2020
01946ee
Merge pull request #19 from coverlet-coverage/master
daveMueller May 11, 2020
2fefa04
changed CecilSymbolHelper to instance class; inject via DI container
daveMueller May 13, 2020
4ff29e3
code review
daveMueller May 16, 2020
970280a
namespace fixes
daveMueller May 16, 2020
befd87e
code review
daveMueller May 27, 2020
777351a
code review
daveMueller May 27, 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
8 changes: 4 additions & 4 deletions src/coverlet.collector/DataCollection/CoverageManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal class CoverageManager
public IReporter[] Reporters { get; }

public CoverageManager(CoverletSettings settings, TestPlatformEqtTrace eqtTrace, TestPlatformLogger logger, ICoverageWrapper coverageWrapper,
IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator)
IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator, ICecilSymbolHelper cecilSymbolHelper)
: this(settings,
settings.ReportFormats.Select(format =>
{
Expand All @@ -39,19 +39,19 @@ public CoverageManager(CoverletSettings settings, TestPlatformEqtTrace eqtTrace,
}
}).Where(r => r != null).ToArray(),
new CoverletLogger(eqtTrace, logger),
coverageWrapper, instrumentationHelper, fileSystem, sourceRootTranslator)
coverageWrapper, instrumentationHelper, fileSystem, sourceRootTranslator, cecilSymbolHelper)
{
}

public CoverageManager(CoverletSettings settings, IReporter[] reporters, ILogger logger, ICoverageWrapper coverageWrapper,
IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator)
IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator, ICecilSymbolHelper cecilSymbolHelper)
{
// Store input vars
Reporters = reporters;
_coverageWrapper = coverageWrapper;

// Coverage object
_coverage = _coverageWrapper.CreateCoverage(settings, logger, instrumentationHelper, fileSystem, sourceRootTranslator);
_coverage = _coverageWrapper.CreateCoverage(settings, logger, instrumentationHelper, fileSystem, sourceRootTranslator, cecilSymbolHelper);
}

/// <summary>
Expand Down
5 changes: 3 additions & 2 deletions src/coverlet.collector/DataCollection/CoverageWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal class CoverageWrapper : ICoverageWrapper
/// <param name="settings">Coverlet settings</param>
/// <param name="coverletLogger">Coverlet logger</param>
/// <returns>Coverage object</returns>
public Coverage CreateCoverage(CoverletSettings settings, ILogger coverletLogger, IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator)
public Coverage CreateCoverage(CoverletSettings settings, ILogger coverletLogger, IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator, ICecilSymbolHelper cecilSymbolHelper)
{
return new Coverage(
settings.TestModule,
Expand All @@ -31,7 +31,8 @@ public Coverage CreateCoverage(CoverletSettings settings, ILogger coverletLogger
coverletLogger,
instrumentationHelper,
fileSystem,
sourceRootTranslator);
sourceRootTranslator,
cecilSymbolHelper);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Coverlet.Collector.Utilities.Interfaces;
using Coverlet.Core.Abstractions;
using Coverlet.Core.Helpers;
using Coverlet.Core.Symbols;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection;

Expand Down Expand Up @@ -133,7 +134,7 @@ private void OnSessionStart(object sender, SessionStartEventArgs sessionStartEve
// Get coverage and attachment managers
_coverageManager = new CoverageManager(coverletSettings, _eqtTrace, _logger, _coverageWrapper,
_serviceProvider.GetRequiredService<IInstrumentationHelper>(), _serviceProvider.GetRequiredService<IFileSystem>(),
_serviceProvider.GetRequiredService<ISourceRootTranslator>());
_serviceProvider.GetRequiredService<ISourceRootTranslator>(), _serviceProvider.GetRequiredService<ICecilSymbolHelper>());

// Instrument modules
_coverageManager.InstrumentModules();
Expand Down Expand Up @@ -227,6 +228,7 @@ private static IServiceCollection GetDefaultServiceCollection(TestPlatformEqtTra
serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();
// We cache resolutions
serviceCollection.AddSingleton<ISourceRootTranslator, SourceRootTranslator>(serviceProvider => new SourceRootTranslator(testModule, serviceProvider.GetRequiredService<ILogger>(), serviceProvider.GetRequiredService<IFileSystem>()));
serviceCollection.AddSingleton<ICecilSymbolHelper, CecilSymbolHelper>();
return serviceCollection;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ internal interface ICoverageWrapper
/// Creates a coverage object from given coverlet settings
/// </summary>
/// <param name="settings">Coverlet settings</param>
/// <param name="coverletLogger">Coverlet logger</param>
/// <param name="logger">Coverlet logger</param>
/// <param name="instrumentationHelper">Coverlet instrumentationHelper</param>
/// <param name="fileSystem">Coverlet fileSystem</param>
/// <param name="sourceRootTranslator">Coverlet sourceRootTranslator</param>
/// <param name="cecilSymbolHelper"></param>
/// <returns>Coverage object</returns>
Coverage CreateCoverage(CoverletSettings settings, ILogger logger, IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator);
Coverage CreateCoverage(CoverletSettings settings, ILogger logger, IInstrumentationHelper instrumentationHelper, IFileSystem fileSystem, ISourceRootTranslator sourceRootTranslator, ICecilSymbolHelper cecilSymbolHelper);

/// <summary>
/// Gets the coverage result from provided coverage object
Expand Down
8 changes: 5 additions & 3 deletions src/coverlet.console/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Coverlet.Core.Enums;
using Coverlet.Core.Helpers;
using Coverlet.Core.Reporters;
using Coverlet.Core.Symbols;
using McMaster.Extensions.CommandLineUtils;
using Microsoft.Extensions.DependencyInjection;

Expand All @@ -29,12 +30,12 @@ static int Main(string[] args)
// We need to keep singleton/static semantics
serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();
serviceCollection.AddSingleton<ISourceRootTranslator, SourceRootTranslator>(provider => new SourceRootTranslator(provider.GetRequiredService<ILogger>(), provider.GetRequiredService<IFileSystem>()));
serviceCollection.AddSingleton<ICecilSymbolHelper, CecilSymbolHelper>();

ServiceProvider serviceProvider = serviceCollection.BuildServiceProvider();

var logger = (ConsoleLogger) serviceProvider.GetService<ILogger>();
var fileSystem = serviceProvider.GetService<IFileSystem>();
var sourceTranslator = serviceProvider.GetService<ISourceRootTranslator>();

var app = new CommandLineApplication();
app.Name = "coverlet";
Expand Down Expand Up @@ -86,9 +87,10 @@ static int Main(string[] args)
mergeWith.Value(),
useSourceLink.HasValue(),
logger,
serviceProvider.GetService<IInstrumentationHelper>(),
serviceProvider.GetRequiredService<IInstrumentationHelper>(),
fileSystem,
sourceTranslator);
serviceProvider.GetRequiredService<ISourceRootTranslator>(),
serviceProvider.GetRequiredService<ICecilSymbolHelper>());
coverage.PrepareModules();

Process process = new Process();
Expand Down
13 changes: 13 additions & 0 deletions src/coverlet.core/Abstractions/ICecilSymbolHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System.Collections.Generic;
using Coverlet.Core.Symbols;
using Mono.Cecil;
using Mono.Cecil.Cil;

namespace Coverlet.Core.Abstractions
{
internal interface ICecilSymbolHelper
{
IReadOnlyList<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition);
bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction);
}
}
7 changes: 5 additions & 2 deletions src/coverlet.core/Coverage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ internal class Coverage
private IInstrumentationHelper _instrumentationHelper;
private IFileSystem _fileSystem;
private ISourceRootTranslator _sourceRootTranslator;
private ICecilSymbolHelper _cecilSymbolHelper;
private List<InstrumenterResult> _results;

public string Identifier
Expand All @@ -47,7 +48,8 @@ public Coverage(string module,
ILogger logger,
IInstrumentationHelper instrumentationHelper,
IFileSystem fileSystem,
ISourceRootTranslator sourceRootTranslator)
ISourceRootTranslator sourceRootTranslator,
ICecilSymbolHelper cecilSymbolHelper)
{
_module = module;
_includeFilters = includeFilters;
Expand All @@ -63,6 +65,7 @@ public Coverage(string module,
_instrumentationHelper = instrumentationHelper;
_fileSystem = fileSystem;
_sourceRootTranslator = sourceRootTranslator;
_cecilSymbolHelper = cecilSymbolHelper;

_identifier = Guid.NewGuid().ToString();
_results = new List<InstrumenterResult>();
Expand Down Expand Up @@ -100,7 +103,7 @@ public CoveragePrepareResult PrepareModules()
continue;
}

var instrumenter = new Instrumenter(module, _identifier, _excludeFilters, _includeFilters, _excludedSourceFiles, _excludeAttributes, _singleHit, _logger, _instrumentationHelper, _fileSystem, _sourceRootTranslator);
var instrumenter = new Instrumenter(module, _identifier, _excludeFilters, _includeFilters, _excludedSourceFiles, _excludeAttributes, _singleHit, _logger, _instrumentationHelper, _fileSystem, _sourceRootTranslator, _cecilSymbolHelper);

if (instrumenter.CanInstrument())
{
Expand Down
9 changes: 6 additions & 3 deletions src/coverlet.core/Instrumentation/Instrumenter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ internal class Instrumenter
private readonly IInstrumentationHelper _instrumentationHelper;
private readonly IFileSystem _fileSystem;
private readonly ISourceRootTranslator _sourceRootTranslator;
private readonly ICecilSymbolHelper _cecilSymbolHelper;
private InstrumenterResult _result;
private FieldDefinition _customTrackerHitsArray;
private FieldDefinition _customTrackerHitsFilePath;
Expand Down Expand Up @@ -57,7 +58,8 @@ public Instrumenter(
ILogger logger,
IInstrumentationHelper instrumentationHelper,
IFileSystem fileSystem,
ISourceRootTranslator sourceRootTranslator)
ISourceRootTranslator sourceRootTranslator,
ICecilSymbolHelper cecilSymbolHelper)
{
_module = module;
_identifier = identifier;
Expand All @@ -71,6 +73,7 @@ public Instrumenter(
_instrumentationHelper = instrumentationHelper;
_fileSystem = fileSystem;
_sourceRootTranslator = sourceRootTranslator;
_cecilSymbolHelper = cecilSymbolHelper;
}

public bool CanInstrument()
Expand Down Expand Up @@ -447,7 +450,7 @@ private void InstrumentIL(MethodDefinition method)
var index = 0;
var count = processor.Body.Instructions.Count;

var branchPoints = CecilSymbolHelper.GetBranchPoints(method);
var branchPoints = _cecilSymbolHelper.GetBranchPoints(method);

for (int n = 0; n < count; n++)
{
Expand All @@ -456,7 +459,7 @@ private void InstrumentIL(MethodDefinition method)
var targetedBranchPoints = branchPoints.Where(p => p.EndOffset == instruction.Offset);

// Check if the instruction is coverable
if (CecilSymbolHelper.SkipNotCoverableInstruction(method, instruction))
if (_cecilSymbolHelper.SkipNotCoverableInstruction(method, instruction))
{
index++;
continue;
Expand Down
25 changes: 10 additions & 15 deletions src/coverlet.core/Symbols/CecilSymbolHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;

using Coverlet.Core.Abstractions;
using Coverlet.Core.Extensions;

using Mono.Cecil;
Expand All @@ -16,16 +16,11 @@

namespace Coverlet.Core.Symbols
{
internal static class CecilSymbolHelper
internal class CecilSymbolHelper : ICecilSymbolHelper
{
private const int StepOverLineCode = 0xFEEFEE;
private static ConcurrentDictionary<string, int[]> CompilerGeneratedBranchesToExclude = null;

static CecilSymbolHelper()
{
// Create single instance, we cannot collide because we use full method name as key
CompilerGeneratedBranchesToExclude = new ConcurrentDictionary<string, int[]>();
}
// Create single instance, we cannot collide because we use full method name as key
private readonly ConcurrentDictionary<string, int[]> _compilerGeneratedBranchesToExclude = new ConcurrentDictionary<string, int[]>();

// In case of nested compiler generated classes, only the root one presents the CompilerGenerated attribute.
// So let's search up to the outermost declaring type to find the attribute
Expand Down Expand Up @@ -265,9 +260,9 @@ private static bool SkipGeneratedBranchForExceptionRethrown(List<Instruction> in
instructions[branchIndex + 3].OpCode == OpCodes.Throw;
}

private static bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDefinition, Instruction instruction, List<Instruction> bodyInstructions)
private bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDefinition, Instruction instruction, List<Instruction> bodyInstructions)
{
if (!CompilerGeneratedBranchesToExclude.ContainsKey(methodDefinition.FullName))
if (!_compilerGeneratedBranchesToExclude.ContainsKey(methodDefinition.FullName))
{
/*
This method is used to parse compiler generated code inside async state machine and find branches generated for exception catch blocks
Expand Down Expand Up @@ -393,13 +388,13 @@ private static bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition m
}
}

CompilerGeneratedBranchesToExclude.TryAdd(methodDefinition.FullName, detectedBranches.ToArray());
_compilerGeneratedBranchesToExclude.TryAdd(methodDefinition.FullName, detectedBranches.ToArray());
}

return CompilerGeneratedBranchesToExclude[methodDefinition.FullName].Contains(instruction.Offset);
return _compilerGeneratedBranchesToExclude[methodDefinition.FullName].Contains(instruction.Offset);
}

public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
public IReadOnlyList<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
{
var list = new List<BranchPoint>();
if (methodDefinition is null)
Expand Down Expand Up @@ -664,7 +659,7 @@ await ...
IL_00eb: br.s IL_00ed
...
*/
internal static bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction)
public bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction)
{
if (!IsMoveNextInsideAsyncStateMachine(methodDefinition))
{
Expand Down
5 changes: 4 additions & 1 deletion src/coverlet.msbuild.tasks/InstrumentationTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Coverlet.Core;
using Coverlet.Core.Abstractions;
using Coverlet.Core.Helpers;
using Coverlet.Core.Symbols;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -131,6 +132,7 @@ public override bool Execute()
serviceCollection.AddSingleton<ISourceRootTranslator, SourceRootTranslator>(serviceProvider => new SourceRootTranslator(_path, serviceProvider.GetRequiredService<ILogger>(), serviceProvider.GetRequiredService<IFileSystem>()));
// We need to keep singleton/static semantics
serviceCollection.AddSingleton<IInstrumentationHelper, InstrumentationHelper>();
serviceCollection.AddSingleton<ICecilSymbolHelper, CecilSymbolHelper>();

ServiceProvider = serviceCollection.BuildServiceProvider();

Expand All @@ -156,7 +158,8 @@ public override bool Execute()
_logger,
ServiceProvider.GetService<IInstrumentationHelper>(),
fileSystem,
ServiceProvider.GetService<ISourceRootTranslator>());
ServiceProvider.GetService<ISourceRootTranslator>(),
ServiceProvider.GetService<ICecilSymbolHelper>());

CoveragePrepareResult prepareResult = coverage.PrepareModules();
InstrumenterState = new TaskItem(System.IO.Path.GetTempFileName());
Expand Down
Loading