-
Notifications
You must be signed in to change notification settings - Fork 392
feat: Add ExcludeByAttribute option (#232) #233
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
Changes from 5 commits
63ba563
b0a6f4f
fa03cb3
c4ba815
4f16aa0
c04da1e
643e1a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.IO; | ||
|
|
@@ -22,20 +23,22 @@ internal class Instrumenter | |
| private readonly string[] _excludeFilters; | ||
| private readonly string[] _includeFilters; | ||
| private readonly string[] _excludedFiles; | ||
| private readonly string[] _excludedAttributes; | ||
| private InstrumenterResult _result; | ||
| private FieldDefinition _customTrackerHitsArray; | ||
| private FieldDefinition _customTrackerHitsFilePath; | ||
| private ILProcessor _customTrackerClassConstructorIl; | ||
| private TypeDefinition _customTrackerTypeDef; | ||
| private MethodReference _customTrackerRecordHitMethod; | ||
|
|
||
| public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles) | ||
| public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles, string[] excludedAttributes) | ||
| { | ||
| _module = module; | ||
| _identifier = identifier; | ||
| _excludeFilters = excludeFilters; | ||
| _includeFilters = includeFilters; | ||
| _excludedFiles = excludedFiles ?? Array.Empty<string>(); | ||
| _excludedAttributes = excludedAttributes; | ||
| } | ||
|
|
||
| public bool CanInstrument() => InstrumentationHelper.HasPdb(_module); | ||
|
|
@@ -179,7 +182,7 @@ private void AddCustomModuleTrackerToModule(ModuleDefinition module) | |
| { | ||
| handler.CatchType = module.ImportReference(handler.CatchType); | ||
| } | ||
|
|
||
| methodOnCustomType.Body.ExceptionHandlers.Add(handler); | ||
| } | ||
|
|
||
|
|
@@ -392,19 +395,24 @@ private static void ReplaceExceptionHandlerBoundary(ExceptionHandler handler, In | |
| handler.TryStart = newTarget; | ||
| } | ||
|
|
||
| private static bool IsExcludeAttribute(CustomAttribute customAttribute) | ||
| private bool IsExcludeAttribute(CustomAttribute customAttribute) | ||
| { | ||
| var excludeAttributeNames = new[] | ||
| // The default custom attributes used to exclude from coverage. | ||
| IEnumerable<string> excludeAttributeNames = new List<string>() | ||
| { | ||
| nameof(ExcludeFromCoverageAttribute), | ||
| "ExcludeFromCoverage", | ||
| nameof(ExcludeFromCodeCoverageAttribute), | ||
| "ExcludeFromCodeCoverage" | ||
| nameof(ExcludeFromCodeCoverageAttribute) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a backward incompatible change. We should make specifying the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this actually incompatible? From what I saw of the usage even if you use the short name attribute like
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more of a user experience thing. Since the compiler doesn't enforce the |
||
| }; | ||
|
|
||
| var attributeName = customAttribute.AttributeType.Name; | ||
| return excludeAttributeNames.Any(a => a.Equals(attributeName)); | ||
| } | ||
| // Include the other attributes to exclude based on incoming parameters. | ||
| if (_excludedAttributes != null) | ||
| { | ||
| excludeAttributeNames = _excludedAttributes.Union(excludeAttributeNames); | ||
| } | ||
|
|
||
| return excludeAttributeNames.Any(a => | ||
| customAttribute.AttributeType.Name.Equals(a.EndsWith("Attribute")? a : $"{a}Attribute")); | ||
| } | ||
|
|
||
| private static Mono.Cecil.Cil.MethodBody GetMethodBody(MethodDefinition method) | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should mention that the
Attributesuffix is optional. See my other commentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a sight fear since it wasn't using the fully qualified name that it could cause issues, but I can make the it try to do the short name resolution like the compiler as well.