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

Added TestContext.Write Is Obsolete Analyzer #772

Merged
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
79 changes: 79 additions & 0 deletions documentation/NUnit1033.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# NUnit1033

## The Write methods on TestContext will be marked as Obsolete and eventually removed

| Topic | Value
| :-- | :--
| Id | NUnit1033
| Severity | Warning
| Enabled | True
| Category | Structure
| Code | [TestContextWriteIsObsoleteAnalyzer](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs)

## Description

Direct `Write` calls should be replaced with `Out.Write`.

Future version of NUnit will first mark the `.Write` methods on `TestContext`
as `Obsolete` and eventually remove them.

This rule allows updating your code before the methods are removed.

## Motivation

The `Write` methods are simple wrappers calling `Out.Write`.
There is no wrapper for `Error` which always required to use `TestContext.Error.Write`.
Besides this being inconsistent, later versions of .NET added new overloads,
e.g. for `ReadOnlySpan<char>` and `async` methods like `WriteAsync`.
Instead of adding more and more dummy wrappers, it was decided that user code should use
the `Out` property and then can use any `Write` overload available on `TextWriter`.

## How to fix violations

Simply insert `.Out` between `TestContext` and `.Write`.

`TestContext.WriteLine("This isn't right");`

becomes

`TestContext.Out.WriteLine("This isn't right");`

<!-- start generated config severity -->
## 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
# NUnit1033: The Write methods on TestContext will be marked as Obsolete and eventually removed
dotnet_diagnostic.NUnit1033.severity = chosenSeverity
```

where `chosenSeverity` can be one of `none`, `silent`, `suggestion`, `warning`, or `error`.

### Via #pragma directive

```csharp
#pragma warning disable NUnit1033 // The Write methods on TestContext will be marked as Obsolete and eventually removed
Code violating the rule here
#pragma warning restore NUnit1033 // The Write methods on TestContext will be marked as Obsolete and eventually removed
```

Or put this at the top of the file to disable all instances.

```csharp
#pragma warning disable NUnit1033 // The Write methods on TestContext will be marked as Obsolete and eventually removed
```

### Via attribute `[SuppressMessage]`

```csharp
[System.Diagnostics.CodeAnalysis.SuppressMessage("Structure",
"NUnit1033:The Write methods on TestContext will be marked as Obsolete and eventually removed",
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 @@ -51,6 +51,7 @@ Rules which enforce structural requirements on the test code.
| [NUnit1030](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1030.md) | The type of parameter provided by the TestCaseSource does not match the type of the parameter in the Test method | :white_check_mark: | :exclamation: | :x: |
| [NUnit1031](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1031.md) | The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method | :white_check_mark: | :exclamation: | :x: |
| [NUnit1032](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1032.md) | An IDisposable field/property should be Disposed in a TearDown method | :white_check_mark: | :exclamation: | :x: |
| [NUnit1033](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1033.md) | The Write methods on TestContext will be marked as Obsolete and eventually removed | :white_check_mark: | :warning: | :white_check_mark: |

## Assertion Rules (NUnit2001 - )

Expand Down
1 change: 1 addition & 0 deletions src/nunit.analyzers.sln
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "documentation", "documentat
..\documentation\NUnit1030.md = ..\documentation\NUnit1030.md
..\documentation\NUnit1031.md = ..\documentation\NUnit1031.md
..\documentation\NUnit1032.md = ..\documentation\NUnit1032.md
..\documentation\NUnit1033.md = ..\documentation\NUnit1033.md
..\documentation\NUnit2001.md = ..\documentation\NUnit2001.md
..\documentation\NUnit2002.md = ..\documentation\NUnit2002.md
..\documentation\NUnit2003.md = ..\documentation\NUnit2003.md
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ public sealed class NUnitFrameworkConstantsTests
(nameof(NUnitFrameworkConstants.NameOfMultipleAsync), "MultipleAsync"),
#endif

(nameof(NUnitFrameworkConstants.NameOfOut), nameof(TestContext.Out)),
(nameof(NUnitFrameworkConstants.NameOfWrite), nameof(TestContext.Out.Write)),
(nameof(NUnitFrameworkConstants.NameOfWriteLine), nameof(TestContext.Out.WriteLine)),

(nameof(NUnitFrameworkConstants.NameOfThrows), nameof(Throws)),
(nameof(NUnitFrameworkConstants.NameOfThrowsArgumentException), nameof(Throws.ArgumentException)),
(nameof(NUnitFrameworkConstants.NameOfThrowsArgumentNullException), nameof(Throws.ArgumentNullException)),
Expand Down Expand Up @@ -208,6 +212,8 @@ public sealed class NUnitFrameworkConstantsTests
(nameof(NUnitFrameworkConstants.FullNameOfCancelAfterAttribute), typeof(CancelAfterAttribute)),
(nameof(NUnitFrameworkConstants.FullNameOfCancellationToken), typeof(CancellationToken)),

(nameof(NUnitFrameworkConstants.FullNameOfTypeTestContext), typeof(TestContext)),

(nameof(NUnitFrameworkConstants.FullNameOfSameAsConstraint), typeof(SameAsConstraint)),
(nameof(NUnitFrameworkConstants.FullNameOfSomeItemsConstraint), typeof(SomeItemsConstraint)),
(nameof(NUnitFrameworkConstants.FullNameOfEqualToConstraint), typeof(EqualConstraint)),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using Gu.Roslyn.Asserts;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Analyzers.Constants;
using NUnit.Analyzers.TestContextWriteIsObsolete;
using NUnit.Framework;

namespace NUnit.Analyzers.Tests.TestContextWriteIsObsolete
{
public class TestContextWriteIsObsoleteAnalyzerTests
{
private static readonly DiagnosticAnalyzer analyzer = new TestContextWriteIsObsoleteAnalyzer();
private static readonly ExpectedDiagnostic expectedDiagnostic =
ExpectedDiagnostic.Create(AnalyzerIdentifiers.TestContextWriteIsObsolete);

[TestCaseSource(typeof(TestContextWriteIsObsoleteTestCases), nameof(TestContextWriteIsObsoleteTestCases.WriteInvocations))]
public void AnyDirectWriteMethod(string writeMethodAndParameters)
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
↓TestContext.{writeMethodAndParameters};
}}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
}

[TestCaseSource(typeof(TestContextWriteIsObsoleteTestCases), nameof(TestContextWriteIsObsoleteTestCases.WriteInvocations))]
public void AnyIndirectWriteMethod(string writeMethodAndParameters)
{
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
TestContext.Out.{writeMethodAndParameters};
}}");

RoslynAssert.Valid(analyzer, testCode);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
using Gu.Roslyn.Asserts;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Diagnostics;
using NUnit.Analyzers.Constants;
using NUnit.Analyzers.TestContextWriteIsObsolete;
using NUnit.Framework;

namespace NUnit.Analyzers.Tests.TestContextWriteIsObsolete
{
public class TestContextWriteIsObsoleteCodeFixTests
{
private static readonly DiagnosticAnalyzer analyzer = new TestContextWriteIsObsoleteAnalyzer();
private static readonly CodeFixProvider fix = new TestContextWriteIsObsoleteCodeFix();
private static readonly ExpectedDiagnostic expectedDiagnostic =
ExpectedDiagnostic.Create(AnalyzerIdentifiers.TestContextWriteIsObsolete);

[TestCaseSource(typeof(TestContextWriteIsObsoleteTestCases), nameof(TestContextWriteIsObsoleteTestCases.WriteInvocations))]
public void AnyWriteMethod(string writeMethodAndParameters)
{
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
↓TestContext.{writeMethodAndParameters};
}}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
TestContext.Out.{writeMethodAndParameters};
}}");

RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
fixTitle: TestContextWriteIsObsoleteCodeFix.InsertOutDescription);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
namespace NUnit.Analyzers.Tests.TestContextWriteIsObsolete
{
internal static class TestContextWriteIsObsoleteTestCases
{
public static readonly string[] WriteInvocations =
{
"Write(true)",
"Write('!')",
"Write(new char[] { '!', '!' })",
"Write(default(char[]))",
"Write(1D)",
"Write(1)",
"Write(1L)",
"Write(1M)",
"Write(default(object))",
"Write(1F)",
"Write(\"NUnit\")",
"Write(default(string))",
"Write(1U)",
"Write(1UL)",
"Write(\"{0}\", 1)",
"Write(\"{0} + {1}\", 1, 2)",
"Write(\"{0} + {1} = {2}\", 1, 2, 3)",
"Write(\"{0} + {1} = {2} + {3}\", 1, 2, 2, 1)",
"WriteLine()",
"WriteLine(true)",
"WriteLine('!')",
"WriteLine(new char[] { '!', '!' })",
"WriteLine(default(char[]))",
"WriteLine(1D)",
"WriteLine(1)",
"WriteLine(1L)",
"WriteLine(1M)",
"WriteLine(default(object))",
"WriteLine(1F)",
"WriteLine(\"NUnit\")",
"Write(default(string))",
"WriteLine(1U)",
"WriteLine(1UL)",
"WriteLine(\"{0}\", 1)",
"WriteLine(\"{0} + {1}\", 1, 2)",
"WriteLine(\"{0} + {1} = {2}\", 1, 2, 3)",
"WriteLine(\"{0} + {1} = {2} + {3}\", 1, 2, 2, 1)",
};
}
}
1 change: 1 addition & 0 deletions src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ internal static class AnalyzerIdentifiers
internal const string TestCaseSourceMismatchWithTestMethodParameterType = "NUnit1030";
internal const string ValuesParameterTypeMismatchUsage = "NUnit1031";
internal const string FieldIsNotDisposedInTearDown = "NUnit1032";
internal const string TestContextWriteIsObsolete = "NUnit1033";

#endregion Structure

Expand Down
6 changes: 6 additions & 0 deletions src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public static class NUnitFrameworkConstants
public const string NameOfMultiple = "Multiple";
public const string NameOfMultipleAsync = "MultipleAsync";

public const string NameOfOut = "Out";
public const string NameOfWrite = "Write";
public const string NameOfWriteLine = "WriteLine";

public const string NameOfThrows = "Throws";
public const string NameOfThrowsArgumentException = "ArgumentException";
public const string NameOfThrowsArgumentNullException = "ArgumentNullException";
Expand Down Expand Up @@ -153,6 +157,8 @@ public static class NUnitFrameworkConstants
public const string FullNameOfCancelAfterAttribute = "NUnit.Framework.CancelAfterAttribute";
public const string FullNameOfCancellationToken = "System.Threading.CancellationToken";

public const string FullNameOfTypeTestContext = "NUnit.Framework.TestContext";

public const string NameOfConstraint = "Constraint";

public const string FullNameOfSameAsConstraint = "NUnit.Framework.Constraints.SameAsConstraint";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
using NUnit.Analyzers.Constants;

namespace NUnit.Analyzers.TestContextWriteIsObsolete
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class TestContextWriteIsObsoleteAnalyzer : DiagnosticAnalyzer
{
private static readonly DiagnosticDescriptor descriptor = DiagnosticDescriptorCreator.Create(
id: AnalyzerIdentifiers.TestContextWriteIsObsolete,
title: TestContextWriteIsObsoleteAnalyzerConstants.Title,
messageFormat: TestContextWriteIsObsoleteAnalyzerConstants.Message,
category: Categories.Structure,
defaultSeverity: DiagnosticSeverity.Warning,
description: TestContextWriteIsObsoleteAnalyzerConstants.Description);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(descriptor);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterCompilationStartAction(AnalyzeCompilationStart);
}

private static void AnalyzeCompilationStart(CompilationStartAnalysisContext context)
{
INamedTypeSymbol? testContextType = context.Compilation.GetTypeByMetadataName(NUnitFrameworkConstants.FullNameOfTypeTestContext);
if (testContextType is null)
{
return;
}

context.RegisterOperationAction(context => AnalyzeInvocation(testContextType, context), OperationKind.Invocation);
}

private static void AnalyzeInvocation(INamedTypeSymbol testContextType, OperationAnalysisContext context)
{
if (context.Operation is not IInvocationOperation invocationOperation)
return;

// TestContext.Write methods are static methods
if (invocationOperation.Instance is not null)
return;

IMethodSymbol targetMethod = invocationOperation.TargetMethod;

if (!targetMethod.ReturnsVoid)
return;

context.CancellationToken.ThrowIfCancellationRequested();

if (!SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, testContextType))
return;

if (targetMethod.Name is NUnitFrameworkConstants.NameOfWrite or
NUnitFrameworkConstants.NameOfWriteLine)
{
context.ReportDiagnostic(Diagnostic.Create(
descriptor,
invocationOperation.Syntax.GetLocation()));
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace NUnit.Analyzers.TestContextWriteIsObsolete
{
internal static class TestContextWriteIsObsoleteAnalyzerConstants
{
public const string Title = "The Write methods on TestContext will be marked as Obsolete and eventually removed";
public const string Message = "The Write methods are wrappers on TestContext.Out";
public const string Description = "Direct Write calls should be replaced with Out.Write.";
}
}
Loading