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

Other ways to support exclude from code analysis? #6

Closed
ChrisTorng opened this issue Dec 20, 2019 · 3 comments
Closed

Other ways to support exclude from code analysis? #6

ChrisTorng opened this issue Dec 20, 2019 · 3 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@ChrisTorng
Copy link

ChrisTorng commented Dec 20, 2019

I think asking most people to define NULLABLE_ATTRIBUTES_EXCLUDE_FROM_CODE_COVERAGE to exclude from code analysis for all projects is troublesome. I found Exclude code from test coverage and code analysis declaring that [DebuggerNonUserCode] and [DebuggerHidden] can be used to exclude from code coverage. They all support .NET Standard 1.0.

@manuelroemer manuelroemer added enhancement New feature or request good first issue Good for newcomers labels Dec 20, 2019
@manuelroemer
Copy link
Owner

Thank you for the feedback! You are right, having to define this attribute should not be a requirement by default. This is a very good proposal for solving this problem. I will have a look into this and try to make these changes in a way that requires the least effort from end users. 👍

One open question is what we should do with the ExcludeFromCodeCoverage attribute. Unfortunately, there is no way (that I am aware of) to check for the current target framework in .cs files. This means that it's not possible to conditionally apply the ExcludeFromCodeCoverage attribute without extra steps, because we have no information if it's available in the currently targeted .NET FW.

Right now I can think of three ways to handle this:

  1. Entirely remove ExcludeFromCodeCoverage and replace it with DebuggerNonUserCode.
  2. Always add DebuggerNonUserCode, but keep supporting the NULLABLE_ATTRIBUTES_EXCLUDE_FROM_CODE_COVERAGE compiler constant to allow users to manually apply the ExcludeFromCodeCoverage attribute.
  3. By abusing the .nuspec: Depending on the target framework of the project into which the package is installed, choose between two different Nullable.cs files to be added to the project. If the target framework supports ExcludeFromCodeCoverage, add a file which uses this attribute to the project. If not, add a file which does not use this attribute to the project.

3 seems to be the most convenient option for end users, but it will also be harder to maintain, because this requires to make changes in two separate code files with only marginal differences. I am still leaning towards this option though, especially considering that this package will not require frequent updates.

Thanks again for raising this issue! Feel free to share your thoughts on the open question above.

@ChrisTorng
Copy link
Author

For my opinion, I think almost nobody needs nullable support for .NET Standard 1.x. I may just use ExcludeFromCodeCoverage and only support .NET Standard 2.0. For those multi-targeting libraries who do need 1.x, use NULLABLE_ATTRIBUTES_NO_EXCLUDE_FROM_CODE_COVERAGE to exclude ExcludeFromCodeCoverage attributes. Or NULLABLE_ATTRIBUTES_NET_STANDARD_1_0 to support .NET Standard 1.0. That means opt-out for a few projects, not opt-in for most projects.

@manuelroemer
Copy link
Owner

The new version 1.2.0 now excludes the attributes from code coverage by default.
I decided to go with option 3 from the comment above and conditionally select a file with/without the ExcludeFromCodeCoverage attribute. While this increases the maintenance effort, it ensures out-of-the-box compatibility with every target framework and therefore introduces no breaking change.

Feel free open new issues if the current code coverage solution does not work in certain cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants