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

Added DiagnosticsSuppressor for NullReference #325

Merged
merged 5 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,7 @@ From Visual Studio one can debug the analyzers by setting the **nunit.analyzers.
### Building using Cake

The command `.\build.ps1` will restore the packages necessary to build the solution, build the projects, and then run the tests. The script can also build the projects in **Release** mode using the option `-Configuration Release` or create NuGet Packages using the option `-Target Pack`. This will create a NuGet package under `package\Debug\` (for a `Debug` build) and the file will be named `NUnit.Analyzers.***.nupkg` where `***` depends upon the build type (`Debug` vs. `Release`) and the version. The NuGet package can then be referenced from another project.
You need to use the `-TargetFramework netstandard2.0` option to include the new DiagnosticSuppressor rules (NUnit3001-) which are not available in `netstandard1.6`.

See [NuGet package vs. extension](https://docs.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview#nuget-package-vs-extension) for more information about the difference between installing a Roslyn analyzer as a NuGet package or as a Visual Studio extension.

See [NuGet package vs. extension](https://docs.microsoft.com/en-us/visualstudio/code-quality/roslyn-analyzers-overview#nuget-package-vs-extension) for more information about the difference between installing a Roslyn analyzer as a NuGet package or as a Visual Studio extension.
4 changes: 3 additions & 1 deletion build.cake
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

var target = Argument("target", "Default");
var configuration = Argument("configuration", "Debug");
var targetFramework = Argument("targetFramework", "netstandard1.6");

//////////////////////////////////////////////////////////////////////
// SET PACKAGE VERSION
Expand Down Expand Up @@ -183,7 +184,8 @@ Task("Pack")
OutputDirectory = PACKAGE_DIR,
Properties = new Dictionary<string, string>()
{
{"Configuration", configuration}
{"Configuration", configuration},
{"TargetFramework", targetFramework }
}
});
});
Expand Down
4 changes: 4 additions & 0 deletions build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and execute your Cake build script with the parameters you provide.
The build script to execute.
.PARAMETER Target
The build script target to run.
.PARAMETER TargetFramework
The build script target framework
.PARAMETER Configuration
The build configuration to use.
.PARAMETER Verbosity
Expand Down Expand Up @@ -44,6 +46,7 @@ Param(
[string]$Script = "build.cake",
[string]$Target,
[string]$Configuration,
[string]$TargetFramework,
[ValidateSet("Quiet", "Minimal", "Normal", "Verbose", "Diagnostic")]
[string]$Verbosity,
[switch]$ShowDescription,
Expand Down Expand Up @@ -221,6 +224,7 @@ if (!(Test-Path $CAKE_EXE)) {
# Build Cake arguments
$cakeArguments = @("$Script");
if ($Target) { $cakeArguments += "-target=$Target" }
if ($TargetFramework) { $cakeArguments += "-targetFramework=$TargetFramework" }
if ($Configuration) { $cakeArguments += "-configuration=$Configuration" }
if ($Verbosity) { $cakeArguments += "-verbosity=$Verbosity" }
if ($ShowDescription) { $cakeArguments += "-showdescription" }
Expand Down
114 changes: 114 additions & 0 deletions documentation/NUnit3001.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# NUnit3001

## Expression was checked in an Assert.NotNull, Assert.IsNotNull or Assert.That call

| Topic | Value
| :-- | :--
| Id | NUnit3001
| Severity | Info
| Enabled | True
| Category | Suppressor
| Code | [DereferencePossiblyNullReferenceSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/DereferencePossiblyNullReferenceSuppressor.cs)

## Description

This rule check diagnostics reported by the CS8601-CS8607 and CS8629 compiler errors:

`CS8602: Dereference of a possibly null reference.`

It then checks the previous statements for one of:
* `Assert.NotNull(...)`
* `Assert.IsNotNull(...)`
* `Assert.That(..., Is.Not.Null)`

For the same expression as the one that raised the original compiler error.
If found, the compiler error is suppressed.

The exception is that if the statement is part of an `Assert.Multiple`
it is not suppressed, as in this case the statement containing the compiler error will be executed.

## Motivation

```csharp
[TestFixture]
internal sealed class SomeClassFixture
{
private SomeClass instance;

[SetUp]
public void Setup()
{
instance = new SomeClass();
}

[Test]
public void Test()
{
string? result = instance.MethodUnderTest();
Assert.That(result, Is.Not.Null);
Assert.That(result.Length, Is.GreaterThan(0));
}

[Test]
public void TestMultiple()
{
string? result = instance.MethodUnderTest();
Assert.Multiple(() =>
{
Assert.That(result, Is.Not.Null);
Assert.That(result.Length, Is.GreaterThan(0));
});
}
}
```

In the above fixture the compiler would give a warnings because `result` can be `null` and the compiler knows nothing
about the `Assert.That(result, Is.Not.Null);` statement.

The first occurrence - in the `Test` method - will be suppressed, the second - in the `TestMultiple` - will not.

## How to fix violations

Ensure that the reference is not `null` before dereferencing it.
This can be done using regular C# language constructs or NUnit assertions.

<!-- start generated config severity -->
## Configure severity

The rule has no severity, but can be disabled.

### Via ruleset file

To disable the rule for a project, you need to add a
[ruleset file](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/NUnit.Analyzers.Suppressions.ruleset)

```xml
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="NUnit.Analyzer Suppressions" Description="DiagnosticSuppression Rules" ToolsVersion="12.0">
<Rules AnalyzerId="DiagnosticSuppressors" RuleNamespace="NUnit.NUnitAnalyzers">
<Rule Id="NUnit3001" Action="Info" /> <!-- Possible Null Reference -->
<Rule Id="NUnit3002" Action="Info" /> <!-- NonNullableField is Uninitialized -->
</Rules>
</RuleSet>
```

and add it to the project like:

```xml
<PropertyGroup>
<CodeAnalysisRuleSet>NUnit.Analyzers.Suppressions.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>
```

For more info about rulesets see [MSDN](https://msdn.microsoft.com/en-us/library/dd264949.aspx).

### Via .editorconfig file

This is currently not working. Waiting for [Roslyn](https://github.com/dotnet/roslyn/issues/49727)

```ini
# NUnit3001: Expression was checked in an Assert.NotNull, Assert.IsNotNull or Assert.That call
dotnet_diagnostic.NUnit3001.severity = none
```
<!-- end generated config severity -->

91 changes: 91 additions & 0 deletions documentation/NUnit3002.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# NUnit3002

## Field is initialized in SetUp or OneTimeSetUp method

| Topic | Value
| :-- | :--
| Id | NUnit3002
| Severity | Info
| Enabled | True
| Category | Suppressor
| Code | [NonNullableFieldIsUninitializedSuppressor](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/NonNullableFieldIsUninitializedSuppressor.cs)

## Description

This rule check diagnostics reported by the CS8618 compiler error:

`CS8618: Non-nullable field '_name_' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.`

If the violating field is set in the `SetUp` or `OneTimeSetUp` method. The rule suppresses the error.
This allows for non-nullable fields to be used in a `TestFixture`.

## Motivation

```csharp
[TestFixture]
internal sealed class SomeClassFixture
{
private SomeClass instance;

[SetUp]
public void Setup()
{
instance = new SomeClass();
}

[Test]
public void Test()
{
Assert.That(instance.MethodUnderTest(), Is.True)
}
}
```

In the above fixture the compiler would give a warning because `instance` is not set in the constructor.
The suggestion to mark `instance` as nullable would mean that we have to test for null in all `Test` methods
or use the null suppression operator (`!`) everywhere.

## How to fix violations

Initialize the field in the `SetUp` or `OneTimeSetUp` methods.

<!-- start generated config severity -->
## Configure severity

The rule has no severity, but can be disabled.

### Via ruleset file

To disable the rule for a project, you need to add a
[ruleset file](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/DiagnosticSuppressors/NUnit.Analyzers.Suppressions.ruleset)

```xml
<?xml version="1.0" encoding="utf-8"?>
<RuleSet Name="NUnit.Analyzer Suppressions" Description="DiagnosticSuppression Rules" ToolsVersion="12.0">
<Rules AnalyzerId="DiagnosticSuppressors" RuleNamespace="NUnit.NUnitAnalyzers">
<Rule Id="NUnit3001" Action="Info" /> <!-- Possible Null Reference -->
<Rule Id="NUnit3002" Action="Info" /> <!-- NonNullableField is Uninitialized -->
</Rules>
</RuleSet>
```

and add it to the project like:

```xml
<PropertyGroup>
<CodeAnalysisRuleSet>NUnit.Analyzers.Suppressions.ruleset</CodeAnalysisRuleSet>
</PropertyGroup>
```

For more info about rulesets see [MSDN](https://msdn.microsoft.com/en-us/library/dd264949.aspx).

### Via .editorconfig file

This is currently not working. Waiting for [Roslyn](https://github.com/dotnet/roslyn/issues/49727)

```ini
# NUnit3002: Field is initialized in SetUp or OneTimeSetUp method
dotnet_diagnostic.NUnit3002.severity = none
```
<!-- end generated config severity -->

9 changes: 9 additions & 0 deletions documentation/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,12 @@ Rules which improve assertions in the test code.
| [NUnit2040](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2040.md) | Non-reference types for SameAs constraint | :white_check_mark: | :exclamation: | :white_check_mark: |
| [NUnit2041](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2041.md) | Incompatible types for comparison constraint | :white_check_mark: | :exclamation: | :x: |
| [NUnit2042](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2042.md) | Comparison constraint on object | :white_check_mark: | :information_source: | :x: |

### Suppressor Rules (NUnit3001 - )

Rules which suppress compiler errors based on context.

| Id | Title | :mag: | :memo: | :bulb: |
| :-- | :-- | :--: | :--: | :--: |
| [NUnit3001](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit3001.md) | Expression was checked in an Assert.NotNull, Assert.IsNotNull or Assert.That call | :white_check_mark: | :information_source: | :x: |
| [NUnit3002](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit3002.md) | Field is initialized in SetUp or OneTimeSetUp method | :white_check_mark: | :information_source: | :x: |
2 changes: 2 additions & 0 deletions src/nunit.analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{2F501E14-3
..\documentation\NUnit2039.md = ..\documentation\NUnit2039.md
..\documentation\NUnit2040.md = ..\documentation\NUnit2040.md
..\documentation\NUnit2041.md = ..\documentation\NUnit2041.md
..\documentation\NUnit3001.md = ..\documentation\NUnit3001.md
..\documentation\NUnit3002.md = ..\documentation\NUnit3002.md
..\README.md = ..\README.md
EndProjectSection
EndProject
Expand Down
Loading