Skip to content

Commit

Permalink
Analyzer for SameAs when used on Value Types.
Browse files Browse the repository at this point in the history
CodeFix to replace SameAs with IsEqualTo
  • Loading branch information
manfred-brands committed Aug 8, 2020
1 parent 0aba656 commit d0ed4f4
Show file tree
Hide file tree
Showing 10 changed files with 405 additions and 0 deletions.
67 changes: 67 additions & 0 deletions documentation/NUnit2040.md
Original file line number Diff line number Diff line change
@@ -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));
```

<!-- start generated config severity -->
## 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...")]
```
<!-- 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 @@ -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: |
1 change: 1 addition & 0 deletions src/nunit.analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<int> { 1 };
var expected = new [] { 1 };
Assert.That(actual, Is.SameAs(expected));",
additionalUsings: "using System.Collections.Generic;");

AnalyzerAssert.Valid(analyzer, testCode);
}
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
1 change: 1 addition & 0 deletions src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions src/nunit.analyzers/Constants/CodeFixConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
}
9 changes: 9 additions & 0 deletions src/nunit.analyzers/Constants/SameAsOnValueTypesConstants.cs
Original file line number Diff line number Diff line change
@@ -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.";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
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<DiagnosticDescriptor> 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 actualTypeInfo = semanticModel.GetTypeInfo(actualExpression, cancellationToken);
var actualType = actualTypeInfo.Type ?? actualTypeInfo.ConvertedType;

if (actualType == null)
continue;

actualType = AssertHelper.UnwrapActualType(actualType);

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);
}
}
}
Loading

0 comments on commit d0ed4f4

Please sign in to comment.