From 16e159ca3117fed14b8d237bce1093b487d79267 Mon Sep 17 00:00:00 2001 From: Jihoon Park Date: Thu, 20 Jun 2024 10:51:01 +0200 Subject: [PATCH] Simplify the ValuesAttribute (#758) * Close #755: Simplify the ValuesAttribute * Replace the expression-bodied property FixableDiagnosticIds with a read-only property with an initializer * Simplify AnalyzeParameter in SimplifyValuesAnalyzer * Rephrase SimplifyValuesDescription * Apply suggestions from code review Co-authored-by: Manfred Brands --------- Co-authored-by: Manfred Brands --- documentation/NUnit4001.md | 78 ++++++ documentation/index.md | 8 + .../SimplifyValuesAnalyzerTests.cs | 253 ++++++++++++++++++ .../SimplifyValuesCodeFixTests.cs | 112 ++++++++ .../Constants/AnalyzerIdentifiers.cs | 6 + src/nunit.analyzers/Constants/Categories.cs | 1 + .../SimplifyValues/SimplifyValuesAnalyzer.cs | 192 +++++++++++++ .../SimplifyValuesAnalyzerConstants.cs | 8 + .../SimplifyValues/SimplifyValuesCodeFix.cs | 48 ++++ 9 files changed, 706 insertions(+) create mode 100644 documentation/NUnit4001.md create mode 100644 src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesAnalyzerTests.cs create mode 100644 src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesCodeFixTests.cs create mode 100644 src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzer.cs create mode 100644 src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzerConstants.cs create mode 100644 src/nunit.analyzers/SimplifyValues/SimplifyValuesCodeFix.cs diff --git a/documentation/NUnit4001.md b/documentation/NUnit4001.md new file mode 100644 index 00000000..38f7e4c8 --- /dev/null +++ b/documentation/NUnit4001.md @@ -0,0 +1,78 @@ +# NUnit4001 + +## Simplify the Values attribute + +| Topic | Value +| :-- | :-- +| Id | NUnit4001 +| Severity | Info +| Enabled | True +| Category | Style +| Code | [SimplifyValuesAnalyzer](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzer.cs) + +## Description + +Consider removing unnecessary parameters from the ValuesAttribute. + +## Motivation + +When used without any arguments, the [Values] attribute on a (nullable) boolean or an (nullable) enum parameter +will automatically include all possible values. + +Therefore the `Values` attribute like + +```csharp +[Test] +public void MyBoolTest([Values(true, false)] bool value) { /* ... */ } +``` + +can be simplified to + +```csharp +[Test] +public void MyBoolTest([Values] bool value) { /* ... */ } +``` + +## How to fix violations + +Remove all arguments of the `Values` attribute. + + +## Configure severity + +### Via ruleset file + +Configure the severity per project, for more info 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 + +```ini +# NUnit4001: Simplify the Values attribute +dotnet_diagnostic.NUnit4001.severity = chosenSeverity +``` + +where `chosenSeverity` can be one of `none`, `silent`, `suggestion`, `warning`, or `error`. + +### Via #pragma directive + +```csharp +#pragma warning disable NUnit4001 // Simplify the Values attribute +Code violating the rule here +#pragma warning restore NUnit4001 // Simplify the Values attribute +``` + +Or put this at the top of the file to disable all instances. + +```csharp +#pragma warning disable NUnit4001 // Simplify the Values attribute +``` + +### Via attribute `[SuppressMessage]` + +```csharp +[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", + "NUnit4001:Simplify the Values attribute", + Justification = "Reason...")] +``` + diff --git a/documentation/index.md b/documentation/index.md index e27985d6..44ba0c2e 100644 --- a/documentation/index.md +++ b/documentation/index.md @@ -120,3 +120,11 @@ builds (version 3.0.0 and above) which require Visual Studio 2019 (version 16.3) | [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 an NUnit TestFixture and is instantiated using reflection | :white_check_mark: | :information_source: | :x: | | [NUnit3004](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit3004.md) | Field should be Disposed in TearDown or OneTimeTearDown method | :white_check_mark: | :information_source: | :x: | + +## Style Rules (NUnit4001 - ) + +Rules which help you write concise and readable NUnit test code. + +| Id | Title | :mag: | :memo: | :bulb: | +| :-- | :-- | :--: | :--: | :--: | +| [NUnit4001](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit4001.md) | Simplify the Values attribute | :white_check_mark: | :information_source: | :x: | diff --git a/src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesAnalyzerTests.cs b/src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesAnalyzerTests.cs new file mode 100644 index 00000000..3d3a2532 --- /dev/null +++ b/src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesAnalyzerTests.cs @@ -0,0 +1,253 @@ +using System.Globalization; +using Gu.Roslyn.Asserts; +using Microsoft.CodeAnalysis; +using NUnit.Analyzers.Constants; +using NUnit.Analyzers.SimplifyValues; +using NUnit.Framework; + +namespace NUnit.Analyzers.Tests.SimplifyValues; + +public class SimplifyValuesAnalyzerTests +{ + private readonly SimplifyValuesAnalyzer analyzer = new(); + + [Test] + public void VerifySupportedDiagnostics() + { + var diagnostics = this.analyzer.SupportedDiagnostics; + + Assert.That(diagnostics, Has.Length.EqualTo(1)); + var diagnostic = diagnostics[0]; + Assert.Multiple(() => + { + Assert.That(diagnostic.Id, Is.EqualTo(AnalyzerIdentifiers.SimplifyValues)); + Assert.That(diagnostic.Title.ToString(CultureInfo.InvariantCulture), Is.Not.Empty); + Assert.That(diagnostic.Category, Is.EqualTo(Categories.Style)); + Assert.That(diagnostic.DefaultSeverity, Is.EqualTo(DiagnosticSeverity.Info)); + }); + } + + [Test] + public void AnalyzeWhenAttributeIsNotInNUnit() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenAttributeIsNotInNUnit + { + [Test] + public void ATest([Values] bool b) { } + + private sealed class ValuesAttribute : Attribute + { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenCombinatorialStrategyIsNotUsed( + [Values("Sequential", "Pairwise")] string nonCombinatorialAttribute, + [Values] bool fullyQualify, + [Values] bool omitAttribute) + { + var prefix = fullyQualify ? "NUnit.Framework." : string.Empty; + var suffix = omitAttribute ? string.Empty : "Attribute"; + var attribute = $"{prefix}{nonCombinatorialAttribute}{suffix}"; + + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing($@" + public class AnalyzeWhenCombinatorialStrategyIsNotUsed + {{ + public enum TestEnum {{ A, B, C }} + + [Test] + [{attribute}] + public void Test([Values(TestEnum.A, TestEnum.B, TestEnum.C)] TestEnum e) {{ }} + }}"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenAttributeHasNoArguments() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenAttributeHasNoArguments + { + [Test] + public void ATest([Values] bool b) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenOneBooleanWasUsed() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenOneBooleanWasUsed + { + [Test] + public void ATest([Values(true)] bool b) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenTwoBooleanWereUsedForNullableBoolean() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenTwoBooleanWereUsedForNullableBoolean + { + [Test] + public void ATest([Values(true, false)] bool? b) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenNotAllEnumValuesWereUsed() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenNotAllEnumValuesWereUsed + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([Values(TestEnum.A, TestEnum.B)] TestEnum e) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenNotAllEnumValuesWereUsedForNullableBoolean() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenNotAllEnumValuesWereUsedForNullableBoolean + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([Values(TestEnum.A, TestEnum.B, TestEnum.C)] TestEnum? e) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeNotAllBooleanValuesAreInParameters() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeNotAllBooleanValuesAreInParameters + { + public void Test([Values(new object[] { true })] bool b) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeNotAllNullableBooleanValuesAreInParameters() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeNotAllNullableBooleanValuesAreInParameters + { + public void Test([Values(new object[] { true, false })] bool? b) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeNotAllEnumValuesAreInParameters() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeNotAllEnumValuesAreInParameters + { + public enum TestEnum { A, B, C } + + public void Test([Values(new object[] { TestEnum.A, TestEnum.B })] TestEnum e) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeNotAllNullableEnumValuesAreInParameters() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeNotAllNullableEnumValuesAreInParameters + { + public enum TestEnum { A, B, C } + + public void Test([Values(new object[] { TestEnum.A, TestEnum.B, TestEnum.C })] TestEnum? e) { } + }"); + RoslynAssert.Valid(this.analyzer, testCode); + } + + [Test] + public void AnalyzeWhenAllBooleanValuesWereUsed() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenAllBooleanValuesWereUsed + { + [Test] + public void Test([↓Values(true, false)] bool b) { } + }"); + RoslynAssert.Diagnostics(this.analyzer, + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SimplifyValues), + testCode); + } + + [Test] + public void AnalyzeWhenAllNullableBooleanValuesWereUsed() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenAllNullableBooleanValuesWereUsed + { + [Test] + public void Test([↓Values(true, false, null)] bool? b) { } + }"); + RoslynAssert.Diagnostics(this.analyzer, + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SimplifyValues), + testCode); + } + + [Test] + public void AnalyzeWhenAllEnumValuesWereUsed() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenAllEnumValuesWereUsed + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([↓Values(TestEnum.A, TestEnum.B, TestEnum.C)] TestEnum e) { } + }"); + RoslynAssert.Diagnostics(this.analyzer, + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SimplifyValues), + testCode); + } + + [Test] + public void AnalyzeWhenAllNullableEnumValuesWereUsed() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeWhenAllNullableEnumValuesWereUsed + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([↓Values(TestEnum.A, TestEnum.B, TestEnum.C, null)] TestEnum? e) { } + }"); + RoslynAssert.Diagnostics(this.analyzer, + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SimplifyValues), + testCode); + } + + [Test] + public void AnalyzeAllEnumValuesAreInParameters() + { + var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class AnalyzeAllEnumValuesAreInParameters + { + public enum TestEnum { A, B, C } + + public void Test([↓Values(new object[] { TestEnum.A, TestEnum.B, TestEnum.C })] TestEnum e) { } + }"); + RoslynAssert.Diagnostics(this.analyzer, + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SimplifyValues), + testCode); + } +} diff --git a/src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesCodeFixTests.cs b/src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesCodeFixTests.cs new file mode 100644 index 00000000..69766337 --- /dev/null +++ b/src/nunit.analyzers.tests/SimplifyValues/SimplifyValuesCodeFixTests.cs @@ -0,0 +1,112 @@ +using Gu.Roslyn.Asserts; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Analyzers.Constants; +using NUnit.Analyzers.SimplifyValues; +using NUnit.Framework; + +namespace NUnit.Analyzers.Tests.SimplifyValues; + +public class SimplifyValuesCodeFixTests +{ + private static readonly DiagnosticAnalyzer analyzer = new SimplifyValuesAnalyzer(); + private static readonly CodeFixProvider fix = new SimplifyValuesCodeFix(); + private static readonly ExpectedDiagnostic expectedDiagnostic = + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SimplifyValues); + + [Test] + public void VerifyGetFixableDiagnosticIds() + { + var ids = fix.FixableDiagnosticIds; + + Assert.That(ids, Is.EquivalentTo(new[] { AnalyzerIdentifiers.SimplifyValues })); + } + + [Test] + public void SimplifyValuesForEnum() + { + var code = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForEnum + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([↓Values(TestEnum.A, TestEnum.B, TestEnum.C)] TestEnum e) { } + }"); + + var fixedCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForEnum + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([Values] TestEnum e) { } + }"); + + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: SimplifyValuesCodeFix.SimplifyValuesTitle); + } + + [Test] + public void SimplifyValuesForBoolean() + { + var code = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForBoolean + { + [Test] + public void Test([↓Values(true, false)] bool b) { } + }"); + + var fixedCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForBoolean + { + [Test] + public void Test([Values] bool b) { } + }"); + + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: SimplifyValuesCodeFix.SimplifyValuesTitle); + } + + [Test] + public void SimplifyValuesForNullableBoolean() + { + var code = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForBoolean + { + [Test] + public void Test([↓Values(null, true, false)] bool? b) { } + }"); + + var fixedCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForBoolean + { + [Test] + public void Test([Values] bool? b) { } + }"); + + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: SimplifyValuesCodeFix.SimplifyValuesTitle); + } + + [Test] + public void SimplifyValuesForNullableEnum() + { + var code = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForNullableEnum + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([↓Values(TestEnum.A, TestEnum.B, TestEnum.C, null)] TestEnum? e) { } + }"); + + var fixedCode = TestUtility.WrapClassInNamespaceAndAddUsing(@" + public class SimplifyValuesForNullableEnum + { + public enum TestEnum { A, B, C } + + [Test] + public void Test([Values] TestEnum? e) { } + }"); + + RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: SimplifyValuesCodeFix.SimplifyValuesTitle); + } +} diff --git a/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs b/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs index 19f7a29d..8abaaa78 100644 --- a/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs +++ b/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs @@ -102,5 +102,11 @@ internal static class AnalyzerIdentifiers internal const string TypesThatOwnDisposableFieldsShouldBeDisposable = "NUnit3004"; #endregion + + #region Style + + internal const string SimplifyValues = "NUnit4001"; + + #endregion } } diff --git a/src/nunit.analyzers/Constants/Categories.cs b/src/nunit.analyzers/Constants/Categories.cs index 2bb5300d..5891973e 100644 --- a/src/nunit.analyzers/Constants/Categories.cs +++ b/src/nunit.analyzers/Constants/Categories.cs @@ -5,5 +5,6 @@ internal static class Categories internal const string Structure = nameof(Structure); internal const string Assertion = nameof(Assertion); internal const string Suppression = nameof(Suppression); + internal const string Style = nameof(Style); } } diff --git a/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzer.cs b/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzer.cs new file mode 100644 index 00000000..bd7b0160 --- /dev/null +++ b/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzer.cs @@ -0,0 +1,192 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Analyzers.Constants; +using NUnit.Analyzers.Extensions; + +namespace NUnit.Analyzers.SimplifyValues; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class SimplifyValuesAnalyzer : DiagnosticAnalyzer +{ + private static readonly DiagnosticDescriptor simplifyValues = DiagnosticDescriptorCreator.Create( + id: AnalyzerIdentifiers.SimplifyValues, + title: SimplifyValuesAnalyzerConstants.SimplifyValuesTitle, + messageFormat: SimplifyValuesAnalyzerConstants.SimplifyValuesMessage, + category: Categories.Style, + defaultSeverity: DiagnosticSeverity.Info, + description: SimplifyValuesAnalyzerConstants.SimplifyValuesDescription); + + private static readonly ImmutableHashSet nonCombinatorialAttributes = ImmutableHashSet.Create("SequentialAttribute", "PairwiseAttribute"); + + /// + /// Types for which NUnit's ValuesAttribute can deduce all values. + /// + private enum HandledType + { + Boolean, + NullableBoolean, + Enum, + NullableEnum, + NotHandled, + } + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(simplifyValues); + + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(AnalyzeCompilationStart); + } + + private static void AnalyzeCompilationStart(CompilationStartAnalysisContext context) + { + var valuesType = context.Compilation.GetTypeByMetadataName(NUnitFrameworkConstants.FullNameOfTypeValuesAttribute); + if (valuesType is null) + return; + + context.RegisterSymbolAction(symbolContext => AnalyzeParameter(symbolContext, valuesType), SymbolKind.Parameter); + } + + private static void AnalyzeParameter(SymbolAnalysisContext symbolContext, INamedTypeSymbol valuesType) + { + var parameterSymbol = symbolContext.Symbol as IParameterSymbol; + if (parameterSymbol is null) + return; + + var methodSymbol = parameterSymbol.ContainingSymbol as IMethodSymbol; + if (methodSymbol is null) + return; + + var methodAttributes = methodSymbol.GetAttributes(); + var methodHasNonCombinatorialAttribute = methodAttributes + .Any(attribute => attribute.AttributeClass is INamedTypeSymbol attributeClass + && StringComparer.Ordinal.Equals( + attributeClass.ContainingNamespace.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat), + "global::NUnit.Framework") + && nonCombinatorialAttributes.Contains(attributeClass.Name)); + + if (methodHasNonCombinatorialAttribute) + return; + + // Now, check the type of the parameter decorated with the Values attribute and get all possible values. + var parameterType = parameterSymbol.Type; + var handledType = GetHandledCase(parameterType); + if (handledType == HandledType.NotHandled) + return; + + HashSet? allPossibleValues = handledType switch + { + HandledType.Boolean => new() { true, false }, + HandledType.NullableBoolean => new() { true, false, null }, + HandledType.Enum => GetAllPossibleEnumValuesOrDefault(parameterType), + HandledType.NullableEnum => GetAllPossibleNullableEnumValuesOrDefault(parameterType), + _ => null, + }; + + if (allPossibleValues is null) + return; + + var valuesAttributes = parameterSymbol.GetAttributes() + .Where(x => x.ApplicationSyntaxReference is not null + && SymbolEqualityComparer.Default.Equals(x.AttributeClass, valuesType)); + foreach (var attribute in valuesAttributes) + { + symbolContext.CancellationToken.ThrowIfCancellationRequested(); + + HashSet argumentValues = new(); + var attributePositionalArguments = attribute.ConstructorArguments.AdjustArguments(); + for (var index = 0; index < attributePositionalArguments.Length; index++) + { + var argumentSyntax = attribute.GetAdjustedArgumentSyntax( + index, + attributePositionalArguments, + symbolContext.CancellationToken); + if (argumentSyntax is null) + return; + + var constructorArgument = attributePositionalArguments[index]; + argumentValues.Add(constructorArgument.IsNull ? null : constructorArgument.Value); + } + + // Use a set comparison, since the order doesn't matter. + if (argumentValues.SetEquals(allPossibleValues)) + { + var location = attribute.ApplicationSyntaxReference?.GetSyntax().GetLocation(); + if (location is not null) + { + var diagnostic = Diagnostic.Create(simplifyValues, location); + symbolContext.ReportDiagnostic(diagnostic); + } + } + } + } + + private static HandledType GetHandledCase(ITypeSymbol typeSymbol) + { + if (typeSymbol.SpecialType == SpecialType.System_Boolean) + return HandledType.Boolean; + + if (typeSymbol.TypeKind == TypeKind.Enum) + return HandledType.Enum; + + if (TryGetTypeArgumentFromNullableType(typeSymbol, out var typeArgument)) + { + if (typeArgument.SpecialType == SpecialType.System_Boolean) + return HandledType.NullableBoolean; + + if (typeArgument.TypeKind == TypeKind.Enum) + return HandledType.NullableEnum; + } + + return HandledType.NotHandled; + } + + private static bool TryGetTypeArgumentFromNullableType(ITypeSymbol typeSymbol, [NotNullWhen(true)] out ITypeSymbol? typeArgument) + { + if (typeSymbol.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T + && typeSymbol is INamedTypeSymbol namedTypeSymbol) + { + typeArgument = namedTypeSymbol.TypeArguments[0]; + return true; + } + + typeArgument = null; + return false; + } + + private static HashSet? GetAllPossibleEnumValuesOrDefault(ITypeSymbol typeSymbol) + { + var namedTypeSymbol = typeSymbol as INamedTypeSymbol; + if (namedTypeSymbol is null) + return null; + + return new( + namedTypeSymbol.GetMembers() + .OfType() + .Select(f => f.ConstantValue)); + } + + private static HashSet? GetAllPossibleNullableEnumValuesOrDefault(ITypeSymbol typeSymbol) + { + if (typeSymbol is INamedTypeSymbol namedTypeSymbol + && namedTypeSymbol.IsGenericType + && namedTypeSymbol.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T + && namedTypeSymbol.TypeArguments.FirstOrDefault() is INamedTypeSymbol enumTypeSymbol) + { + var allPossibleNullableEnumValues = GetAllPossibleEnumValuesOrDefault(enumTypeSymbol); + if (allPossibleNullableEnumValues is not null) + { + allPossibleNullableEnumValues.Add(null); + return allPossibleNullableEnumValues; + } + } + + return null; + } +} diff --git a/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzerConstants.cs b/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzerConstants.cs new file mode 100644 index 00000000..f09fd918 --- /dev/null +++ b/src/nunit.analyzers/SimplifyValues/SimplifyValuesAnalyzerConstants.cs @@ -0,0 +1,8 @@ +namespace NUnit.Analyzers.SimplifyValues; + +internal static class SimplifyValuesAnalyzerConstants +{ + internal const string SimplifyValuesTitle = "Simplify the Values attribute"; + internal const string SimplifyValuesMessage = "Simplify the Values attribute"; + internal const string SimplifyValuesDescription = "Consider removing unnecessary parameters from the ValuesAttribute."; +} diff --git a/src/nunit.analyzers/SimplifyValues/SimplifyValuesCodeFix.cs b/src/nunit.analyzers/SimplifyValues/SimplifyValuesCodeFix.cs new file mode 100644 index 00000000..199645c5 --- /dev/null +++ b/src/nunit.analyzers/SimplifyValues/SimplifyValuesCodeFix.cs @@ -0,0 +1,48 @@ +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using NUnit.Analyzers.Constants; + +namespace NUnit.Analyzers.SimplifyValues; + +[ExportCodeFixProvider(LanguageNames.CSharp)] +[Shared] +public class SimplifyValuesCodeFix : CodeFixProvider +{ + internal const string SimplifyValuesTitle = "Simplify the Values attribute"; + + public override ImmutableArray FixableDiagnosticIds { get; } + = ImmutableArray.Create(AnalyzerIdentifiers.SimplifyValues); + + public sealed override FixAllProvider GetFixAllProvider() + => WellKnownFixAllProviders.BatchFixer; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + var semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false); + + if (root is null || semanticModel is null) + return; + + context.CancellationToken.ThrowIfCancellationRequested(); + + var attributeNode = root.FindNode(context.Span) as AttributeSyntax; + if (attributeNode is null) + return; + + var newRoot = root + .ReplaceNode(attributeNode, attributeNode.WithArgumentList(null)); + + var codeAction = CodeAction.Create( + SimplifyValuesTitle, + _ => Task.FromResult(context.Document.WithSyntaxRoot(newRoot)), + SimplifyValuesTitle); + + context.RegisterCodeFix(codeAction, context.Diagnostics); + } +}