diff --git a/documentation/NUnit2040.md b/documentation/NUnit2040.md new file mode 100644 index 00000000..af89e157 --- /dev/null +++ b/documentation/NUnit2040.md @@ -0,0 +1,67 @@ +# NUnit2040 + +## Non-reference types for SameAs constraint. + +| Topic | Value +| :-- | :-- +| Id | NUnit2040 +| Severity | Error +| Enabled | True +| Category | Assertion +| Code | [SameAsOnValueTypesAnalyzer](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesAnalyzer.cs) + +## Description + +The SameAs constraint always fails on value types as the actual and the expected value cannot be the same reference. + +## Motivation + +```csharp +var expected = Guid.Empty; +var actual = expected; + +Assert.That(actual, Is.SameAs(expected)); +``` + +As `Guid` is a `struct`, actual will be a copy of expected but not have the same reference. + +## How to fix violations + +Replace `Is.SameAs` with `Is.EqualTo`. + +```csharp +var expected = Guid.Empty; +var actual = expected; + +Assert.That(actual, Is.EqualTo(expected)); +``` + + +## Configure severity + +### Via ruleset file. + +Configure the severity per project, for more info see [MSDN](https://msdn.microsoft.com/en-us/library/dd264949.aspx). + +### Via #pragma directive. + +```csharp +#pragma warning disable NUnit2040 // Non-reference types for SameAs constraint. +Code violating the rule here +#pragma warning restore NUnit2040 // Non-reference types for SameAs constraint. +``` + +Or put this at the top of the file to disable all instances. + +```csharp +#pragma warning disable NUnit2040 // Non-reference types for SameAs constraint. +``` + +### Via attribute `[SuppressMessage]`. + +```csharp +[System.Diagnostics.CodeAnalysis.SuppressMessage("Assertion", + "NUnit2040:Non-reference types for SameAs constraint.", + Justification = "Reason...")] +``` + diff --git a/documentation/index.md b/documentation/index.md index ca14f5b9..cde07bb5 100644 --- a/documentation/index.md +++ b/documentation/index.md @@ -67,3 +67,4 @@ | [NUnit2037](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2037.md)| Consider using Assert.That(collection, Does.Contain(instance)) instead of Assert.Contains(instance, collection). | :white_check_mark: | | [NUnit2038](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2038.md)| Consider using Assert.That(actual, Is.InstanceOf(expected)) instead of Assert.IsInstanceOf(expected, actual). | :white_check_mark: | | [NUnit2039](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2039.md)| Consider using Assert.That(actual, Is.Not.InstanceOf(expected)) instead of Assert.IsNotInstanceOf(expected, actual). | :white_check_mark: | +| [NUnit2040](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit2040.md)| Non-reference types for SameAs constraint. | :white_check_mark: | diff --git a/src/nunit.analyzers.sln b/src/nunit.analyzers.sln index a312fc8a..e1773272 100644 --- a/src/nunit.analyzers.sln +++ b/src/nunit.analyzers.sln @@ -79,6 +79,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{2F501E14-3 ..\documentation\NUnit2037.md = ..\documentation\NUnit2037.md ..\documentation\NUnit2038.md = ..\documentation\NUnit2038.md ..\documentation\NUnit2039.md = ..\documentation\NUnit2039.md + ..\documentation\NUnit2040.md = ..\documentation\NUnit2040.md ..\README.md = ..\README.md EndProjectSection EndProject diff --git a/src/nunit.analyzers.tests/SameAsOnValueTypes/SameAsOnValueTypesAnalyzerTests.cs b/src/nunit.analyzers.tests/SameAsOnValueTypes/SameAsOnValueTypesAnalyzerTests.cs new file mode 100644 index 00000000..4ee84cfe --- /dev/null +++ b/src/nunit.analyzers.tests/SameAsOnValueTypes/SameAsOnValueTypesAnalyzerTests.cs @@ -0,0 +1,97 @@ +using Gu.Roslyn.Asserts; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Analyzers.Constants; +using NUnit.Analyzers.SameAsOnValueTypes; +using NUnit.Framework; + +namespace NUnit.Analyzers.Tests.SameAsOnValueTypes +{ + public class SameAsOnValueTypesAnalyzerTests + { + private static readonly DiagnosticAnalyzer analyzer = new SameAsOnValueTypesAnalyzer(); + private static readonly ExpectedDiagnostic expectedDiagnostic = + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SameAsOnValueTypes); + + [Test] + public void AnalyzeWhenLiteralTypesProvided() + { + var testCode = TestUtility.WrapInTestMethod( + "↓Assert.That(1, Is.SameAs(1));"); + + AnalyzerAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + + [Test] + public void AnalyzeWhenStructTypesProvided() + { + var testCode = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + var actual = expected; + + ↓Assert.That(actual, Is.SameAs(expected));"); + + AnalyzerAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + + [Test] + public void AnalyzeWhenStructDelegateProvided() + { + var testCode = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + + ↓Assert.That(() => expected, Is.SameAs(expected));"); + + AnalyzerAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + + [Test] + public void AnalyzeWhenStructTaskProvided() + { + var testCode = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + + ↓Assert.That(Task.FromResult(expected), Is.SameAs(expected));"); + + AnalyzerAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + + [Test] + public void AnalyzeWhenActualIsReferenceTypeAndExpectedIsValueType() + { + var testCode = TestUtility.WrapInTestMethod( + @"↓Assert.That(""3"", Is.Not.SameAs(3));"); + + AnalyzerAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + + [Test] + public void AnalyzeWhenActualIsValueTypeAndExpectedIsReferenceType() + { + var testCode = TestUtility.WrapInTestMethod( + @"↓Assert.That(3, Is.Not.SameAs(""3""));"); + + AnalyzerAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); + } + + [Test] + public void AnalyzeWhenBothArgumentsAreReferenceType() + { + var testCode = TestUtility.WrapInTestMethod( + @"Assert.That(""3"", Is.SameAs(""3""));"); + + AnalyzerAssert.Valid(analyzer, testCode); + } + + [Test] + public void AnalyzeWhenCollectionsWithValueTypesProvided() + { + var testCode = TestUtility.WrapInTestMethod(@" + var actual = new List { 1 }; + var expected = new [] { 1 }; + Assert.That(actual, Is.SameAs(expected));", + additionalUsings: "using System.Collections.Generic;"); + + AnalyzerAssert.Valid(analyzer, testCode); + } + } +} diff --git a/src/nunit.analyzers.tests/SameAsOnValueTypes/SameAsOnValueTypesCodeFixTests.cs b/src/nunit.analyzers.tests/SameAsOnValueTypes/SameAsOnValueTypesCodeFixTests.cs new file mode 100644 index 00000000..51c66e2a --- /dev/null +++ b/src/nunit.analyzers.tests/SameAsOnValueTypes/SameAsOnValueTypesCodeFixTests.cs @@ -0,0 +1,78 @@ +using System.Collections.Immutable; +using Gu.Roslyn.Asserts; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Analyzers.Constants; +using NUnit.Analyzers.SameAsOnValueTypes; +using NUnit.Framework; + +namespace NUnit.Analyzers.Tests.SameAsOnValueTypes +{ + [TestFixture] + public sealed class SameAsOnValueTypesCodeFixTests + { + private static readonly DiagnosticAnalyzer analyzer = new SameAsOnValueTypesAnalyzer(); + private static readonly CodeFixProvider fix = new SameAsOnValueTypesCodeFix(); + private static readonly ExpectedDiagnostic expectedDiagnostic = + ExpectedDiagnostic.Create(AnalyzerIdentifiers.SameAsOnValueTypes); + + [Test] + public void VerifyGetFixableDiagnosticIds() + { + var fix = new SameAsOnValueTypesCodeFix(); + var ids = fix.FixableDiagnosticIds.ToImmutableArray(); + + Assert.That(ids, Is.EquivalentTo(new[] { AnalyzerIdentifiers.SameAsOnValueTypes })); + } + + [Test] + public void VerifySameAsIntoEqualToFix() + { + var code = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + var actual = expected; + + ↓Assert.That(actual, Is.SameAs(expected)); + "); + var fixedCode = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + var actual = expected; + + Assert.That(actual, Is.EqualTo(expected)); + "); + AnalyzerAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: CodeFixConstants.UseIsEqualToDescription); + } + + [Test] + public void VerifySameAsIntoEqualToFixWithMessage() + { + var code = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + + ↓Assert.That(() => expected, Is.Not.SameAs(expected), ""message""); + "); + var fixedCode = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + + Assert.That(() => expected, Is.Not.EqualTo(expected), ""message""); + "); + AnalyzerAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: CodeFixConstants.UseIsEqualToDescription); + } + + [Test] + public void VerifySameAsIntoEqualToFixWithMessageAndParams() + { + var code = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + + ↓Assert.That(Task.FromResult(expected), Is.SameAs(expected), ""message"", Guid.Empty); + "); + var fixedCode = TestUtility.WrapInTestMethod(@" + var expected = Guid.Empty; + + Assert.That(Task.FromResult(expected), Is.EqualTo(expected), ""message"", Guid.Empty); + "); + AnalyzerAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode, fixTitle: CodeFixConstants.UseIsEqualToDescription); + } + } +} diff --git a/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs b/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs index ca7e3dca..e67472df 100644 --- a/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs +++ b/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs @@ -76,6 +76,7 @@ internal static class AnalyzerIdentifiers internal const string ContainsUsage = "NUnit2037"; internal const string IsInstanceOfUsage = "NUnit2038"; internal const string IsNotInstanceOfUsage = "NUnit2039"; + internal const string SameAsOnValueTypes = "NUnit2040"; #endregion Assertion } diff --git a/src/nunit.analyzers/Constants/CodeFixConstants.cs b/src/nunit.analyzers/Constants/CodeFixConstants.cs index ce0ada41..11010787 100644 --- a/src/nunit.analyzers/Constants/CodeFixConstants.cs +++ b/src/nunit.analyzers/Constants/CodeFixConstants.cs @@ -8,5 +8,6 @@ internal static class CodeFixConstants internal const string UsePropertyDescriptionFormat = "Use '{0}' property"; internal const string MakeTestMethodPublic = "Make test method public"; internal const string MakeNonTestMethodPrivate = "Make non-test method private"; + internal const string UseIsEqualToDescription = "Use IsEqualTo on value types"; } } diff --git a/src/nunit.analyzers/Constants/SameAsOnValueTypesConstants.cs b/src/nunit.analyzers/Constants/SameAsOnValueTypesConstants.cs new file mode 100644 index 00000000..e54799b5 --- /dev/null +++ b/src/nunit.analyzers/Constants/SameAsOnValueTypesConstants.cs @@ -0,0 +1,9 @@ +namespace NUnit.Analyzers.Constants +{ + internal static class SameAsOnValueTypesConstants + { + internal static string Title = "Non-reference types for SameAs constraint."; + internal static string Message = "The SameAs constraint always fails on value types as the actual and the expected value cannot be the same reference."; + internal static string Description = "The SameAs constraint always fails on value types as the actual and the expected value cannot be the same reference."; + } +} diff --git a/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesAnalyzer.cs b/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesAnalyzer.cs new file mode 100644 index 00000000..2ab54482 --- /dev/null +++ b/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesAnalyzer.cs @@ -0,0 +1,84 @@ +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Analyzers.Constants; +using NUnit.Analyzers.Extensions; +using NUnit.Analyzers.Helpers; +using NUnit.Analyzers.Syntax; + +namespace NUnit.Analyzers.SameAsOnValueTypes +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public class SameAsOnValueTypesAnalyzer : BaseAssertionAnalyzer + { + private static readonly DiagnosticDescriptor descriptor = DiagnosticDescriptorCreator.Create( + id: AnalyzerIdentifiers.SameAsOnValueTypes, + title: SameAsOnValueTypesConstants.Title, + messageFormat: SameAsOnValueTypesConstants.Message, + category: Categories.Assertion, + defaultSeverity: DiagnosticSeverity.Error, + description: SameAsOnValueTypesConstants.Description); + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(descriptor); + + protected override void AnalyzeAssertInvocation(SyntaxNodeAnalysisContext context, + InvocationExpressionSyntax assertExpression, IMethodSymbol methodSymbol) + { + var cancellationToken = context.CancellationToken; + var semanticModel = context.SemanticModel; + + if (!AssertHelper.TryGetActualAndConstraintExpressions(assertExpression, semanticModel, + out var actualExpression, out var constraintExpression)) + { + return; + } + + foreach (var constraintPartExpression in constraintExpression.ConstraintParts) + { + if (HasIncompatiblePrefixes(constraintPartExpression) + || constraintPartExpression.HasUnknownExpressions()) + { + return; + } + + var constraintMethod = constraintPartExpression.GetConstraintMethod(); + + if (constraintMethod?.Name != NunitFrameworkConstants.NameOfIsSameAs + || constraintMethod.ReturnType?.GetFullMetadataName() != NunitFrameworkConstants.FullNameOfSameAsConstraint) + { + continue; + } + + var expectedArgumentExpression = constraintPartExpression.GetExpectedArgumentExpression(); + + if (expectedArgumentExpression == null) + continue; + + var actualType = AssertHelper.GetUnwrappedActualType(actualExpression, semanticModel, cancellationToken); + + if (actualType == null) + continue; + + var expectedType = semanticModel.GetTypeInfo(expectedArgumentExpression, cancellationToken).Type; + + if (actualType.IsValueType || expectedType.IsValueType) + { + context.ReportDiagnostic(Diagnostic.Create( + descriptor, + assertExpression.GetLocation())); + } + } + } + + private static bool HasIncompatiblePrefixes(ConstraintPartExpression constraintPartExpression) + { + // Currently only 'Not' suffix supported, as all other suffixes change actual type for constraint + // (e.g. All, Some, Property, Count, etc.) + + return constraintPartExpression.GetPrefixesNames().Any(s => s != NunitFrameworkConstants.NameOfIsNot); + } + } +} diff --git a/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesCodeFix.cs b/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesCodeFix.cs new file mode 100644 index 00000000..31a7bad6 --- /dev/null +++ b/src/nunit.analyzers/SameAsOnValueTypes/SameAsOnValueTypesCodeFix.cs @@ -0,0 +1,63 @@ +using System.Collections.Generic; +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.SameAsOnValueTypes +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class SameAsOnValueTypesCodeFix : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds + => ImmutableArray.Create(AnalyzerIdentifiers.SameAsOnValueTypes); + + public sealed override FixAllProvider GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + context.CancellationToken.ThrowIfCancellationRequested(); + + var diagnostic = context.Diagnostics.First(); + var invocationNode = root.FindNode(diagnostic.Location.SourceSpan) as InvocationExpressionSyntax; + + if (invocationNode == null) + return; + + // Find the 'SameAs' member access expression + var constraintArgument = invocationNode.ArgumentList.Arguments[1]; + var isSameExpression = constraintArgument + .DescendantNodesAndSelf() + .Where(s => s.IsKind(SyntaxKind.SimpleMemberAccessExpression)) + .OfType() + .SingleOrDefault(m => m.Name.ToString() == NunitFrameworkConstants.NameOfIsSameAs); + + if (isSameExpression == null) + return; + + var isEqualToExpression = isSameExpression.WithName(SyntaxFactory.IdentifierName(NunitFrameworkConstants.NameOfIsEqualTo)); + + context.CancellationToken.ThrowIfCancellationRequested(); + + var newRoot = root.ReplaceNode(isSameExpression, isEqualToExpression); + + context.RegisterCodeFix( + CodeAction.Create( + CodeFixConstants.UseIsEqualToDescription, + _ => Task.FromResult(context.Document.WithSyntaxRoot(newRoot)), + CodeFixConstants.UseIsEqualToDescription), diagnostic); + } + } +}