Skip to content

Commit

Permalink
Code review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
manfred-brands committed Aug 9, 2024
1 parent 2149eca commit 9501ab8
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 14 deletions.
24 changes: 15 additions & 9 deletions documentation/NUnit1033.md
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
# NUnit1033

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

| Topic | Value
| :-- | :--
| Id | NUnit1033
| Severity | Error
| 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.
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

Expand All @@ -23,9 +28,10 @@ Besides this being inconsistent, later versions of .NET added new overloads,
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

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

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

Expand All @@ -44,7 +50,7 @@ Configure the severity per project, for more info see
### Via .editorconfig file

```ini
# NUnit1033: The Write methods on TestContext are Obsolete
# NUnit1033: The Write methods on TestContext will be marked as Obsolete and eventually removed
dotnet_diagnostic.NUnit1033.severity = chosenSeverity
```

Expand All @@ -53,22 +59,22 @@ where `chosenSeverity` can be one of `none`, `silent`, `suggestion`, `warning`,
### Via #pragma directive

```csharp
#pragma warning disable NUnit1033 // The Write methods on TestContext are Obsolete
#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 are Obsolete
#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 are Obsolete
#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 are Obsolete",
"NUnit1033:The Write methods on TestContext will be marked as Obsolete and eventually removed",
Justification = "Reason...")]
```
<!-- end generated config severity -->
2 changes: 1 addition & 1 deletion documentation/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +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 are Obsolete | :white_check_mark: | :exclamation: | :white_check_mark: |
| [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 @@ -18,7 +18,7 @@ public void AnyDirectWriteMethod(string writeMethodAndParameters)
var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
TestContext.{writeMethodAndParameters};
TestContext.{writeMethodAndParameters};
}}");

RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void AnyWriteMethod(string writeMethodAndParameters)
var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
public void Test()
{{
TestContext.{writeMethodAndParameters};
TestContext.{writeMethodAndParameters};
}}");

var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class TestContextWriteIsObsoleteAnalyzer : DiagnosticAnalyzer
title: TestContextWriteIsObsoleteAnalyzerConstants.Title,
messageFormat: TestContextWriteIsObsoleteAnalyzerConstants.Message,
category: Categories.Structure,
defaultSeverity: DiagnosticSeverity.Error,
defaultSeverity: DiagnosticSeverity.Warning,
description: TestContextWriteIsObsoleteAnalyzerConstants.Description);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(descriptor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ namespace NUnit.Analyzers.TestContextWriteIsObsolete
{
internal static class TestContextWriteIsObsoleteAnalyzerConstants
{
public const string Title = "The Write methods on TestContext are Obsolete";
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.";
}
Expand Down

0 comments on commit 9501ab8

Please sign in to comment.