Skip to content

Commit

Permalink
Added a suppressor when CA1812 fires on NUnit Test classes.
Browse files Browse the repository at this point in the history
  • Loading branch information
manfred-brands committed Aug 12, 2023
1 parent 35fe3df commit b0adf70
Show file tree
Hide file tree
Showing 18 changed files with 340 additions and 20 deletions.
1 change: 1 addition & 0 deletions documentation/NUnit3001.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ To disable the rule for a project, you need to add a
<Rules AnalyzerId="DiagnosticSuppressors" RuleNamespace="NUnit.NUnitAnalyzers">
<Rule Id="NUnit3001" Action="Info" /> <!-- Possible Null Reference -->
<Rule Id="NUnit3002" Action="Info" /> <!-- NonNullableField/Property is Uninitialized -->
<Rule Id="NUnit3003" Action="Info" /> <!-- Avoid Uninstantiated Internal Classes -->
</Rules>
</RuleSet>
```
Expand Down
1 change: 1 addition & 0 deletions documentation/NUnit3002.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ To disable the rule for a project, you need to add a
<Rules AnalyzerId="DiagnosticSuppressors" RuleNamespace="NUnit.NUnitAnalyzers">
<Rule Id="NUnit3001" Action="Info" /> <!-- Possible Null Reference -->
<Rule Id="NUnit3002" Action="Info" /> <!-- NonNullableField/Property is Uninitialized -->
<Rule Id="NUnit3003" Action="Info" /> <!-- Avoid Uninstantiated Internal Classes -->
</Rules>
</RuleSet>
```
Expand Down
63 changes: 63 additions & 0 deletions documentation/NUnit3003.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# NUnit3003

## Class is a NUnit TestFixture and called by reflection

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

## Description

Class is a NUnit TestFixture and called by reflection

## Motivation

The default roslyn analyzer has rule [CA1812](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1812)
which warns about internal classes not being used.
That analyzer doesn't know about NUnit test classes.
This suppressor catches the error, verifies the class is an NUNit TestFixture and if so suppresses the error.

<!-- 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/Property is Uninitialized -->
<Rule Id="NUnit3003" Action="Info" /> <!-- Avoid Uninstantiated Internal Classes -->
</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://learn.microsoft.com/en-us/visualstudio/code-quality/using-rule-sets-to-group-code-analysis-rules?view=vs-2022).

### Via .editorconfig file

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

```ini
# NUnit3003: Class is a NUnit TestFixture and called by reflection
dotnet_diagnostic.NUnit3003.severity = none
```
<!-- end generated config severity -->
1 change: 1 addition & 0 deletions documentation/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,4 @@ Rules which suppress compiler errors based on context. Note that these rules are
| :-- | :-- | :--: | :--: | :--: |
| [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/Property is initialized in SetUp or OneTimeSetUp method | :white_check_mark: | :information_source: | :x: |
| [NUnit3003](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit3003.md) | Class is a NUnit TestFixture and called by reflection | :white_check_mark: | :information_source: | :x: |
1 change: 1 addition & 0 deletions src/nunit.analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "documentation", "documentat
..\documentation\NUnit2047.md = ..\documentation\NUnit2047.md
..\documentation\NUnit3001.md = ..\documentation\NUnit3001.md
..\documentation\NUnit3002.md = ..\documentation\NUnit3002.md
..\documentation\NUnit3003.md = ..\documentation\NUnit3003.md
..\README.md = ..\README.md
EndProjectSection
EndProject
Expand Down
49 changes: 49 additions & 0 deletions src/nunit.analyzers.tests/DefaultEnabledAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

namespace NUnit.Analyzers.Tests
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
#pragma warning disable RS1036 // Specify analyzer banned API enforcement setting
internal sealed class DefaultEnabledAnalyzer : DiagnosticAnalyzer
#pragma warning restore RS1036 // Specify analyzer banned API enforcement setting
{
private readonly DiagnosticAnalyzer inner;

internal DefaultEnabledAnalyzer(DiagnosticAnalyzer inner)
{
this.inner = inner;
this.SupportedDiagnostics = EnabledDiagnostics(inner.SupportedDiagnostics);

static ImmutableArray<DiagnosticDescriptor> EnabledDiagnostics(ImmutableArray<DiagnosticDescriptor> source)
{
var builder = ImmutableArray.CreateBuilder<DiagnosticDescriptor>(source.Length);
foreach (var diagnostic in source)
{
builder.Add(
new DiagnosticDescriptor(
diagnostic.Id,
diagnostic.Title,
diagnostic.MessageFormat,
diagnostic.Category,
diagnostic.DefaultSeverity,
isEnabledByDefault: true,
diagnostic.Description,
diagnostic.HelpLinkUri,
diagnostic.CustomTags?.ToArray() ?? Array.Empty<string>()));
}

return builder.MoveToImmutable();
}
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }

#pragma warning disable RS1025, RS1026
public override void Initialize(AnalysisContext context) => this.inner.Initialize(context);
#pragma warning restore RS1025, RS1026
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using System;
using System.IO;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Analyzers.DiagnosticSuppressors;
using NUnit.Framework;

namespace NUnit.Analyzers.Tests.DiagnosticSuppressors
{
internal class AvoidUninstantiatedInternalClassesSuppressorTests
{
private const string CalculatorClass = @"
internal sealed class Calculator
{
private readonly Calculation _calculation;
public Calculator(Calculation calculation) => _calculation = calculation;
public int Calculate(int op1, int op2)
{
return _calculation == Calculation.Add ? op1 + op2 : op1 - op2;
}
public enum Calculation
{
Add,
Subtract,
}
}
";

private const string CalculatorTest = @"
internal sealed class CalculatorTests
{
[Test]
public void TestAdd()
{
var instance = new Calculator(Calculator.Calculation.Add);
var result = instance.Calculate(3, 4);
Assert.That(result, Is.EqualTo(7));
}
[Test]
public void TestSubtract()
{
var instance = new Calculator(Calculator.Calculation.Subtract);
var result = instance.Calculate(3, 4);
Assert.That(result, Is.EqualTo(-1));
}
}
";

private static readonly DiagnosticSuppressor suppressor = new AvoidUninstantiatedInternalClassesSuppressor();
private DiagnosticAnalyzer analyzer;

[OneTimeSetUp]
public void OneTimeSetUp()
{
// Find the NetAnalyzers assembly (note version should match the one referenced)
string netAnalyzersPath = Path.Combine(PathHelper.GetNuGetPackageDirectory(),
"microsoft.codeanalysis.netanalyzers/7.0.3/analyzers/dotnet/cs/Microsoft.CodeAnalysis.CSharp.NetAnalyzers.dll");
Assembly netAnalyzerAssembly = Assembly.LoadFrom(netAnalyzersPath);
Type analyzerType = netAnalyzerAssembly.GetType("Microsoft.CodeQuality.CSharp.Analyzers.Maintainability.CSharpAvoidUninstantiatedInternalClasses", true)!;
this.analyzer = (DiagnosticAnalyzer)Activator.CreateInstance(analyzerType!)!;

this.analyzer = new DefaultEnabledAnalyzer(this.analyzer);
}

[Test]
public async Task NonTestClass()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(CalculatorClass);

await TestHelpers.NotSuppressed(this.analyzer, suppressor, testCode).ConfigureAwait(true);
}

[Test]
public async Task TestClass()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing($@"
{CalculatorClass}
{CalculatorTest}
");

await TestHelpers.Suppressed(this.analyzer, suppressor, testCode).ConfigureAwait(true);
}
}
}
1 change: 1 addition & 0 deletions src/nunit.analyzers.tests/DocumentationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ private static string CreateStub(DiagnosticSuppressor suppressor, SuppressionDes
<Rules AnalyzerId=""DiagnosticSuppressors"" RuleNamespace=""NUnit.NUnitAnalyzers"">
<Rule Id=""NUnit3001"" Action=""Info"" /> <!-- Possible Null Reference -->
<Rule Id=""NUnit3002"" Action=""Info"" /> <!-- NonNullableField/Property is Uninitialized -->
<Rule Id=""NUnit3003"" Action=""Info"" /> <!-- Avoid Uninstantiated Internal Classes -->
</Rules>
</RuleSet>
```
Expand Down
21 changes: 21 additions & 0 deletions src/nunit.analyzers.tests/PathHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System;
using System.IO;

namespace NUnit.Analyzers.Tests
{
internal static class PathHelper
{
public static string GetNuGetPackageDirectory()
{
return Environment.GetEnvironmentVariable("NUGET_PACKAGES") ??
Path.Combine(GetHomeDirectory(), ".nuget/packages");
}

public static string GetHomeDirectory()
{
return Environment.GetEnvironmentVariable("HOME") ?? // Linux
Environment.GetEnvironmentVariable("USERPROFILE") ?? // Windows
throw new NotSupportedException("Cannot determine home directory");
}
}
}
43 changes: 38 additions & 5 deletions src/nunit.analyzers.tests/TestHelpers.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,60 @@
using System;
using System.Collections.Immutable;
using System.Threading.Tasks;
using Gu.Roslyn.Asserts;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Framework;

namespace NUnit.Analyzers.Tests
{
internal static class TestHelpers
{
internal static Compilation CreateCompilation(string? code = null)
internal static Compilation CreateCompilation(string? code = null, Settings? settings = null)
{
var syntaxTrees = code is null
? null
: new[] { CSharpSyntaxTree.ParseText(code) };

settings ??= Settings.Default;

return CSharpCompilation.Create(Guid.NewGuid().ToString("N"),
syntaxTrees,
references: Settings.Default.MetadataReferences,
options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary,
nullableContextOptions: NullableContextOptions.Enable,
reportSuppressedDiagnostics: true));
references: settings.MetadataReferences,
options: settings.CompilationOptions);
}

internal static async Task SuppressedOrNot(DiagnosticAnalyzer analyzer, DiagnosticSuppressor suppressor, string code, bool isSuppressed, Settings? settings = null)
{
string id = analyzer.SupportedDiagnostics[0].Id;
Assert.That(suppressor.SupportedSuppressions[0].SuppressedDiagnosticId, Is.EqualTo(id));

settings ??= Settings.Default;
settings = settings.WithCompilationOptions(Settings.Default.CompilationOptions.WithWarningOrError(analyzer.SupportedDiagnostics));

Compilation compilation = CreateCompilation(code, settings);

CompilationWithAnalyzers compilationWithAnalyzer = compilation
.WithAnalyzers(ImmutableArray.Create(analyzer));
ImmutableArray<Diagnostic> diagnostics = await compilationWithAnalyzer.GetAllDiagnosticsAsync().ConfigureAwait(false);
Assert.That(diagnostics, Has.Length.EqualTo(1));
Assert.That(diagnostics[0].Id, Is.EqualTo(id));

CompilationWithAnalyzers compilationWithAnalyzerAndSuppressor = compilation
.WithAnalyzers(ImmutableArray.Create(analyzer, suppressor));
diagnostics = await compilationWithAnalyzerAndSuppressor.GetAllDiagnosticsAsync().ConfigureAwait(false);
Assert.That(diagnostics, Has.Length.EqualTo(1));
Assert.That(diagnostics[0].Id, Is.EqualTo(id));
Assert.That(diagnostics[0].IsSuppressed, Is.EqualTo(isSuppressed));
}

internal static Task NotSuppressed(DiagnosticAnalyzer analyzer, DiagnosticSuppressor suppressor, string code, Settings? settings = null)
=> SuppressedOrNot(analyzer, suppressor, code, false, settings);

internal static Task Suppressed(DiagnosticAnalyzer analyzer, DiagnosticSuppressor suppressor, string code, Settings? settings = null)
=> SuppressedOrNot(analyzer, suppressor, code, true, settings);

internal static async Task<(SyntaxNode Node, SemanticModel Model)> GetRootAndModel(string code)
{
var tree = CSharpSyntaxTree.ParseText(code);
Expand Down
1 change: 1 addition & 0 deletions src/nunit.analyzers.tests/nunit.analyzers.tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<PackageReference Include="Gu.Roslyn.Asserts.Analyzers" Version="4.0.0" />
<PackageReference Include="NUnit" Version="3.13.3" />
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
<PackageReference Include="System.Collections.Immutable" Version="6.0.0" />
</ItemGroup>

<ItemGroup>
Expand Down
1 change: 1 addition & 0 deletions src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ internal static class AnalyzerIdentifiers

internal const string DereferencePossibleNullReference = "NUnit3001";
internal const string NonNullableFieldOrPropertyIsUninitialized = "NUnit3002";
internal const string AvoidUninstantiatedInternalClasses = "NUnit3003";

#endregion
}
Expand Down
Loading

0 comments on commit b0adf70

Please sign in to comment.