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 11, 2023
1 parent 35fe3df commit 40d51f4
Show file tree
Hide file tree
Showing 10 changed files with 332 additions and 18 deletions.
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 -->
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);
}
}
}
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/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
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#if !NETSTANDARD1_6

using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Analyzers.Constants;
using NUnit.Analyzers.Extensions;

namespace NUnit.Analyzers.DiagnosticSuppressors
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class AvoidUninstantiatedInternalClassesSuppressor : DiagnosticSuppressor
{
internal static readonly SuppressionDescriptor AvoidUninstantiatedInternalTestFixtureClasses = new(
id: AnalyzerIdentifiers.AvoidUninstantiatedInternalClasses,
suppressedDiagnosticId: "CA1812",
justification: "Class is a NUnit TestFixture and called by reflection");

public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions { get; } =
ImmutableArray.Create(AvoidUninstantiatedInternalTestFixtureClasses);

public override void ReportSuppressions(SuppressionAnalysisContext context)
{
foreach (var diagnostic in context.ReportedDiagnostics)
{
SyntaxTree? sourceTree = diagnostic.Location.SourceTree;

if (sourceTree is null)
{
continue;
}

SyntaxNode node = sourceTree.GetRoot(context.CancellationToken)
.FindNode(diagnostic.Location.SourceSpan);

if (node is not ClassDeclarationSyntax classDeclaration)
{
continue;
}

SemanticModel semanticModel = context.GetSemanticModel(sourceTree);
INamedTypeSymbol? typeSymbol = (INamedTypeSymbol?)semanticModel.GetDeclaredSymbol(classDeclaration, context.CancellationToken);

// Does the class have any Test related methods
if (typeSymbol is not null &&
typeSymbol.GetMembers().OfType<IMethodSymbol>().Any(m => m.IsTestRelatedMethod(context.Compilation)))
{
context.ReportSuppression(Suppression.Create(AvoidUninstantiatedInternalTestFixtureClasses, diagnostic));
}
}
}
}
}

#endif
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
<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>
12 changes: 12 additions & 0 deletions src/nunit.analyzers/Extensions/IMethodSymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,17 @@ internal static bool IsInterfaceImplementation(this IMethodSymbol @this, string
.Any(interfaceMethod => interfaceMethod.Name == @this.Name
&& SymbolEqualityComparer.Default.Equals(@this.ContainingType.FindImplementationForInterfaceMember(interfaceMethod), @this));
}

internal static bool IsTestRelatedMethod(this IMethodSymbol methodSymbol, Compilation compilation)
{
return methodSymbol.HasTestRelatedAttributes(compilation) ||
(methodSymbol.OverriddenMethod is not null && methodSymbol.OverriddenMethod.IsTestRelatedMethod(compilation));
}

internal static bool HasTestRelatedAttributes(this IMethodSymbol methodSymbol, Compilation compilation)
{
return methodSymbol.GetAttributes().Any(
a => a.IsTestMethodAttribute(compilation) || a.IsSetUpOrTearDownMethodAttribute(compilation));
}
}
}
Loading

0 comments on commit 40d51f4

Please sign in to comment.