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

test fields should be readonly #6

Merged
merged 2 commits into from
Aug 21, 2024
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
8 changes: 8 additions & 0 deletions NUnit.Analyzers/Constants/AnalyzerIdentifiers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
namespace NUnit.Analyzers.Constants
{
internal static class AnalyzerIdentifiers
{
internal const string TestFieldIsNotReadonly = "NU0001";
internal const string TestUsesSetupAttributes = "NU0002";
}
}
7 changes: 7 additions & 0 deletions NUnit.Analyzers/Constants/Categories.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace NUnit.Analyzers.Constants
{
internal static class Categories
{
internal const string ParallelExecution = nameof(ParallelExecution);
}
}
13 changes: 13 additions & 0 deletions NUnit.Analyzers/Constants/NUnitFrameworkConstants.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
namespace NUnit.Analyzers.Constants
{
public static class NUnitFrameworkConstants
{
public const string FullNameOfTypeITestBuilder = "NUnit.Framework.Interfaces.ITestBuilder";
public const string FullNameOfTypeISimpleTestBuilder = "NUnit.Framework.Interfaces.ISimpleTestBuilder";

public const string FullNameOfTypeOneTimeSetUpAttribute = "NUnit.Framework.OneTimeSetUpAttribute";
public const string FullNameOfTypeOneTimeTearDownAttribute = "NUnit.Framework.OneTimeTearDownAttribute";
public const string FullNameOfTypeSetUpAttribute = "NUnit.Framework.SetUpAttribute";
public const string FullNameOfTypeTearDownAttribute = "NUnit.Framework.TearDownAttribute";
}
}
53 changes: 53 additions & 0 deletions NUnit.Analyzers/Extensions/AttributeDataExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
using System.Linq;

using Microsoft.CodeAnalysis;

using NUnit.Analyzers.Constants;

namespace NUnit.Analyzers.Extensions
{
public static class AttributeDataExtensions
{
public static bool DerivesFromISimpleTestBuilder(this AttributeData @this, Compilation compilation)
{
return DerivesFromInterface(compilation, @this, NUnitFrameworkConstants.FullNameOfTypeISimpleTestBuilder);
}

public static bool DerivesFromITestBuilder(this AttributeData @this, Compilation compilation)
{
return DerivesFromInterface(compilation, @this, NUnitFrameworkConstants.FullNameOfTypeITestBuilder);
}

public static bool IsTestMethodAttribute(this AttributeData @this, Compilation compilation)
{
return @this.DerivesFromITestBuilder(compilation) ||
@this.DerivesFromISimpleTestBuilder(compilation);
}

public static bool IsSetUpOrTearDownMethodAttribute(this AttributeData @this, Compilation compilation)
{
var attributeType = @this.AttributeClass;

if (attributeType is null)
return false;

return attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeOneTimeSetUpAttribute, compilation)
|| attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeOneTimeTearDownAttribute, compilation)
|| attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeSetUpAttribute, compilation)
|| attributeType.IsType(NUnitFrameworkConstants.FullNameOfTypeTearDownAttribute, compilation);
}

private static bool DerivesFromInterface(Compilation compilation, AttributeData attributeData, string interfaceTypeFullName)
{
if (attributeData.AttributeClass is null)
return false;

var interfaceType = compilation.GetTypeByMetadataName(interfaceTypeFullName);

if (interfaceType is null)
return false;

return attributeData.AttributeClass.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(i, interfaceType));
}
}
}
21 changes: 21 additions & 0 deletions NUnit.Analyzers/Extensions/MethodSymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
using System.Linq;

using Microsoft.CodeAnalysis;

namespace NUnit.Analyzers.Extensions
{
internal static class MethodSymbolExtensions
{
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));
}
}
}
14 changes: 14 additions & 0 deletions NUnit.Analyzers/Extensions/TypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Microsoft.CodeAnalysis;

namespace NUnit.Analyzers.Extensions
{
public static class TypeSymbolExtensions
{
internal static bool IsType(this ITypeSymbol? @this, string fullMetadataName, Compilation compilation)
{
var typeSymbol = compilation.GetTypeByMetadataName(fullMetadataName);

return SymbolEqualityComparer.Default.Equals(typeSymbol, @this);
}
}
}
13 changes: 13 additions & 0 deletions NUnit.Analyzers/NUnit.Analyzers.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<Nullable>enable</Nullable>
<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.8.0" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
using System.Collections.Immutable;
using System.Linq;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

using NUnit.Analyzers.Constants;
using NUnit.Analyzers.Extensions;

namespace NUnit.Analyzers.TestFieldIsNotReadonly
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class TestFieldIsNotReadonlyAnalyzer : DiagnosticAnalyzer
{
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSymbolAction(AnalyzeType, SymbolKind.NamedType);
}

private static void AnalyzeType(SymbolAnalysisContext context)
{
var typeSymbol = (INamedTypeSymbol)context.Symbol;

var hasTests = typeSymbol
.GetMembers()
.OfType<IMethodSymbol>()
.Any(x => x.MethodKind == MethodKind.Ordinary && x.IsTestRelatedMethod(context.Compilation));

if (!hasTests)
return;

var fields = typeSymbol
.GetMembers()
.OfType<IFieldSymbol>()
.Where(x => !x.IsReadOnly && !x.IsConst);

foreach (var field in fields)
context.ReportDiagnostic(Diagnostic.Create(testFieldIsNotReadonly, field.Locations[0]));
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create(testFieldIsNotReadonly);

private static readonly DiagnosticDescriptor testFieldIsNotReadonly = new DiagnosticDescriptor(
AnalyzerIdentifiers.TestFieldIsNotReadonly,

Check warning on line 47 in NUnit.Analyzers/TestFieldIsNotReadonly/TestFieldIsNotReadonlyAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0001' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 47 in NUnit.Analyzers/TestFieldIsNotReadonly/TestFieldIsNotReadonlyAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0001' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 47 in NUnit.Analyzers/TestFieldIsNotReadonly/TestFieldIsNotReadonlyAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0001' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 47 in NUnit.Analyzers/TestFieldIsNotReadonly/TestFieldIsNotReadonlyAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0001' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)
TestFieldIsNotReadonlyConstants.TestFieldIsNotReadonlyTitle,
TestFieldIsNotReadonlyConstants.TestFieldIsNotReadonlyMessage,
Categories.ParallelExecution,
DiagnosticSeverity.Warning,
true,
TestFieldIsNotReadonlyConstants.TestFieldIsNotReadonlyDescription
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading.Tasks;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

using NUnit.Analyzers.Constants;

namespace NUnit.Analyzers.TestFieldIsNotReadonly
{
[Shared]
[ExportCodeFixProvider(LanguageNames.CSharp)]
public class TestFieldIsNotReadonlyCodeFix : CodeFixProvider
{
private const string makeTestFieldReadonly = "Make test field readonly";

public override sealed FixAllProvider GetFixAllProvider()
{
return WellKnownFixAllProviders.BatchFixer;
}

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

if (root is null)
{
return;
}

context.CancellationToken.ThrowIfCancellationRequested();

var node = root.FindNode(context.Span);
if (node is not FieldDeclarationSyntax originalExpression)
return;

if (originalExpression.Modifiers.Any(IsReadonlyModifier))
return;

var readonlySyntax = SyntaxFactory.Token(
SyntaxTriviaList.Empty,
SyntaxKind.ReadOnlyKeyword,
SyntaxTriviaList.Create(SyntaxFactory.Whitespace(" ")));

var addedReadonlyModifier = originalExpression.Modifiers.Add(readonlySyntax);
var newExpression = originalExpression.WithModifiers(addedReadonlyModifier);

var newRoot = root.ReplaceNode(originalExpression, newExpression);

var codeAction = CodeAction.Create(
makeTestFieldReadonly,
_ => Task.FromResult(context.Document.WithSyntaxRoot(newRoot)),
makeTestFieldReadonly);

context.RegisterCodeFix(codeAction, context.Diagnostics);
}

private static bool IsReadonlyModifier(SyntaxToken syntaxToken) =>
syntaxToken.IsKind(SyntaxKind.ConstKeyword) ||
syntaxToken.IsKind(SyntaxKind.ReadOnlyKeyword);

public override ImmutableArray<string> FixableDiagnosticIds
=> ImmutableArray.Create(AnalyzerIdentifiers.TestFieldIsNotReadonly);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace NUnit.Analyzers.TestFieldIsNotReadonly
{
internal static class TestFieldIsNotReadonlyConstants
{
internal const string TestFieldIsNotReadonlyTitle = "The field in test class is not readonly";
internal const string TestFieldIsNotReadonlyMessage = "Fields in test classes should be readonly";
internal const string TestFieldIsNotReadonlyDescription = "A test fixture should not contain any modifiable shared state to simplify tests parallel execution.";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
using System.Collections.Immutable;
using System.Linq;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

using NUnit.Analyzers.Constants;
using NUnit.Analyzers.Extensions;

namespace NUnit.Analyzers.TestUsesSetupMethods
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class TestUsesSetupMethodsAnalyzer : DiagnosticAnalyzer
{
public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterSymbolAction(AnalyzeMethod, SymbolKind.Method);
}

private static void AnalyzeMethod(SymbolAnalysisContext context)
{
var methodSymbol = (IMethodSymbol)context.Symbol;
if (IsSetUpTearDownMethod(context.Compilation, methodSymbol))
{
context.ReportDiagnostic(Diagnostic.Create(testUsesSetupMethods, methodSymbol.Locations[0]));
}
}

private static bool IsSetUpTearDownMethod(Compilation compilation, IMethodSymbol methodSymbol)
{
return methodSymbol.GetAttributes().Any(a => a.IsSetUpOrTearDownMethodAttribute(compilation));
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create(testUsesSetupMethods);

private static readonly DiagnosticDescriptor testUsesSetupMethods = new DiagnosticDescriptor(
AnalyzerIdentifiers.TestUsesSetupAttributes,

Check warning on line 40 in NUnit.Analyzers/TestUsesSetupMethods/TestUsesSetupMethodsAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0002' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 40 in NUnit.Analyzers/TestUsesSetupMethods/TestUsesSetupMethodsAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0002' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 40 in NUnit.Analyzers/TestUsesSetupMethods/TestUsesSetupMethodsAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0002' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)

Check warning on line 40 in NUnit.Analyzers/TestUsesSetupMethods/TestUsesSetupMethodsAnalyzer.cs

View workflow job for this annotation

GitHub Actions / test

Enable analyzer release tracking for the analyzer project containing rule 'NU0002' (https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md)
TestUsesSetupMethodsConstants.TestUsesSetupMethodsTitle,
TestUsesSetupMethodsConstants.TestUsesSetupMethodsMessage,
Categories.ParallelExecution,
DiagnosticSeverity.Warning,
true,
TestUsesSetupMethodsConstants.TestUsesSetupMethodsDescription,
TestUsesSetupMethodsConstants.TestUsesSetupMethodsUri
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace NUnit.Analyzers.TestUsesSetupMethods
{
internal class TestUsesSetupMethodsConstants
{
internal const string TestUsesSetupMethodsTitle = "Test uses setup methods";
internal const string TestUsesSetupMethodsMessage = "Setup methods are considered harmful";
internal const string TestUsesSetupMethodsDescription = "If you require a similar object or state for your tests, prefer a helper method than using Setup and Teardown attributes.";
internal const string TestUsesSetupMethodsUri = "https://learn.microsoft.com/en-us/dotnet/core/testing/unit-testing-best-practices#prefer-helper-methods-to-setup-and-teardown";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ protected override void Configure(ISetupBuilder fixture, ISetupBuilder test)
{
t.Properties.Set("simple-counter", new DisposableCounter());

return () => ((IDisposable)t.Properties.Get("simple-counter")).Dispose();
return () => ((IDisposable)t.Properties.Get("simple-counter")!).Dispose();
});
}
}
Expand Down
11 changes: 5 additions & 6 deletions NUnit.Extensions.Tests/NUnit.Extensions.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="6.12.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
<PackageReference Include="NUnit.Analyzers" Version="4.0.1">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
<PackageReference Include="FluentAssertions.Analyzers" Version="0.33.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.0" />
<PackageReference Include="NUnit.Analyzers" Version="4.3.0" />
<PackageReference Include="NUnit3TestAdapter" Version="4.6.0" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\NUnit.Analyzers\NUnit.Analyzers.csproj" OutputItemType="Analyzer" ReferenceOutputAssembly="false" />
<ProjectReference Include="..\NUnit.Middlewares\NUnit.Middlewares.csproj" />
<ProjectReference Include="..\NUnit.Retries\NUnit.Retries.csproj" />
</ItemGroup>
Expand Down
6 changes: 6 additions & 0 deletions nunit-extensions.sln
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NUnit.Extensions.Tests", "N
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NUnit.Retries", "NUnit.Retries\NUnit.Retries.csproj", "{7EEE6F2E-F282-410D-A82A-6A5C4B9A9ECF}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "NUnit.Analyzers", "NUnit.Analyzers\NUnit.Analyzers.csproj", "{5BEC009E-0FB1-4747-A011-A8B20CBE1EFC}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand All @@ -24,5 +26,9 @@ Global
{7EEE6F2E-F282-410D-A82A-6A5C4B9A9ECF}.Debug|Any CPU.Build.0 = Debug|Any CPU
{7EEE6F2E-F282-410D-A82A-6A5C4B9A9ECF}.Release|Any CPU.ActiveCfg = Release|Any CPU
{7EEE6F2E-F282-410D-A82A-6A5C4B9A9ECF}.Release|Any CPU.Build.0 = Release|Any CPU
{5BEC009E-0FB1-4747-A011-A8B20CBE1EFC}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{5BEC009E-0FB1-4747-A011-A8B20CBE1EFC}.Debug|Any CPU.Build.0 = Debug|Any CPU
{5BEC009E-0FB1-4747-A011-A8B20CBE1EFC}.Release|Any CPU.ActiveCfg = Release|Any CPU
{5BEC009E-0FB1-4747-A011-A8B20CBE1EFC}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
EndGlobal
Loading