search

manuelroemer / Nullable

A source code only package which allows you to use .NET's nullable attributes in older target frameworks like .NET Standard 2.0 or the "old" .NET Framework.
https://www.nuget.org/packages/Nullable
MIT License
186 stars 8 forks source link

Other ways to support exclude from code analysis? #6

Closed ChrisTorng closed 4 years ago

ChrisTorng commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.