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

Create ValuesUsageAnalyzer. #570

Merged
merged 19 commits into from
Aug 29, 2023
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
100 changes: 100 additions & 0 deletions documentation/NUnit1031.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# NUnit1031

## The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method

| Topic | Value
| :-- | :--
| Id | NUnit1031
| Severity | Error
| Enabled | True
| Category | Structure
| Code | [ValuesUsageAnalyzer](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/ValuesUsage/ValuesUsageAnalyzer.cs)

## Description

The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method.

## Motivation

To prevent tests that will fail at runtime due to improper construction.

## How to fix violations

manfred-brands marked this conversation as resolved.
Show resolved Hide resolved
### Example Violation

```csharp
[Test]
public void SampleTest([Values(0.0, 1.0)] int numberValue)
{
Assert.That(numberValue, Is.AnyOf(0, 1));
}
```

### Problem

In the test above, `numberValue` is declared as an integer. However, `[Values(0.0, 1.0)]` provides values as doubles. This will lead to a runtime failure.

### Fix
manfred-brands marked this conversation as resolved.
Show resolved Hide resolved

Ensure that the type of the objects used by the `ValuesAttribute` matches that of the parameter.

So, this fix would be acceptable:

```csharp
// Both use type int.
[Test]
public void SampleTest([Values(0, 1)] int numberValue)
{
Assert.That(numberValue, Is.AnyOf(0, 1));
}
```

And this would also work:

```csharp
// Both use type double
[Test]
public void SampleTest([Values(0.0, 1.0)] double numberValue)
{
Assert.That(numberValue, Is.AnyOf(0, 1));
}
```

<!-- 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
# NUnit1031: The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method
dotnet_diagnostic.NUnit1031.severity = chosenSeverity
```

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

### Via #pragma directive

```csharp
#pragma warning disable NUnit1031 // The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method
Code violating the rule here
#pragma warning restore NUnit1031 // The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method
```

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

```csharp
#pragma warning disable NUnit1031 // The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method
```

### Via attribute `[SuppressMessage]`

```csharp
[System.Diagnostics.CodeAnalysis.SuppressMessage("Structure",
"NUnit1031:The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method",
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 @@ -49,6 +49,7 @@ Rules which enforce structural requirements on the test code.
| [NUnit1028](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1028.md) | The non-test method is public | :white_check_mark: | :information_source: | :white_check_mark: |
| [NUnit1029](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1029.md) | The number of parameters provided by the TestCaseSource does not match the number of parameters in the Test method | :white_check_mark: | :exclamation: | :x: |
| [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: |

### 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\NUnit1028.md = ..\documentation\NUnit1028.md
..\documentation\NUnit1029.md = ..\documentation\NUnit1029.md
..\documentation\NUnit1030.md = ..\documentation\NUnit1030.md
..\documentation\NUnit1031.md = ..\documentation\NUnit1031.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 @@ -118,6 +118,7 @@ public sealed class NUnitFrameworkConstantsTests
(nameof(NUnitFrameworkConstants.NameOfTestAttribute), nameof(TestAttribute)),
(nameof(NUnitFrameworkConstants.NameOfParallelizableAttribute), nameof(ParallelizableAttribute)),
(nameof(NUnitFrameworkConstants.NameOfValueSourceAttribute), nameof(ValueSourceAttribute)),
(nameof(NUnitFrameworkConstants.NameOfValuesAttribute), nameof(ValuesAttribute)),

(nameof(NUnitFrameworkConstants.NameOfExpectedResult), nameof(TestAttribute.ExpectedResult)),

Expand All @@ -137,6 +138,7 @@ public sealed class NUnitFrameworkConstantsTests
(nameof(NUnitFrameworkConstants.FullNameOfTypeTestAttribute), typeof(TestAttribute)),
(nameof(NUnitFrameworkConstants.FullNameOfTypeParallelizableAttribute), typeof(ParallelizableAttribute)),
(nameof(NUnitFrameworkConstants.FullNameOfTypeValueSourceAttribute), typeof(ValueSourceAttribute)),
(nameof(NUnitFrameworkConstants.FullNameOfTypeValuesAttribute), typeof(ValuesAttribute)),
(nameof(NUnitFrameworkConstants.FullNameOfTypeITestBuilder), typeof(ITestBuilder)),
(nameof(NUnitFrameworkConstants.FullNameOfTypeISimpleTestBuilder), typeof(ISimpleTestBuilder)),
(nameof(NUnitFrameworkConstants.FullNameOfTypeIParameterDataSource), typeof(IParameterDataSource)),
Expand Down
20 changes: 10 additions & 10 deletions src/nunit.analyzers.tests/DocumentationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void EnsureAllDescriptorsHaveDocumentation(DescriptorInfo descriptorInfo)
{
var descriptor = descriptorInfo.Descriptor;
var id = descriptor.Id;
DumpIfDebug(descriptorInfo.Stub);
DumpIfRequested(descriptorInfo.Stub);
File.WriteAllText(descriptorInfo.DocumentationFile.Name + ".generated", descriptorInfo.Stub);
Assert.Fail($"Documentation is missing for {id}");
}
Expand All @@ -98,7 +98,7 @@ public void EnsureAllSuppressorsHaveDocumentation(SuppressorInfo suppressorInfo)
{
var descriptor = suppressorInfo.Descriptor;
var id = descriptor.Id;
DumpIfDebug(suppressorInfo.Stub);
DumpIfRequested(suppressorInfo.Stub);
File.WriteAllText(suppressorInfo.DocumentationFile.Name + ".generated", suppressorInfo.Stub);
Assert.Fail($"Documentation is missing for {id}");
}
Expand Down Expand Up @@ -156,8 +156,8 @@ public void Description(DescriptorInfo descriptorInfo)
.First(l => !string.IsNullOrWhiteSpace(l));
var actual = Replace(actualRaw, "`", string.Empty);

DumpIfDebug(expected);
DumpIfDebug(actual);
DumpIfRequested(expected);
DumpIfRequested(actual);
Assert.That(actual, Is.EqualTo(expected));
}

Expand All @@ -166,7 +166,7 @@ public void EnsureThatTableIsAsExpected(BaseInfo info)
{
const string headerRow = "| Topic | Value";
var expected = GetTable(info.Stub, headerRow);
DumpIfDebug(expected);
DumpIfRequested(expected);
var actual = GetTable(info.DocumentationFile.AllText, headerRow);
CodeAssert.AreEqual(expected, actual);
}
Expand All @@ -175,7 +175,7 @@ public void EnsureThatTableIsAsExpected(BaseInfo info)
public void EnsureThatConfigSeverityIsAsExpected(BaseInfo info)
{
var expected = GetConfigSeverity(info.Stub);
DumpIfDebug(expected);
DumpIfRequested(expected);
var actual = GetConfigSeverity(info.DocumentationFile.AllText);
CodeAssert.AreEqual(expected, actual);

Expand Down Expand Up @@ -224,7 +224,7 @@ public void EnsureThatAnalyzerIndexIsAsExpected(string category, int tableNumber
}

var expected = builder.ToString();
DumpIfDebug(expected);
DumpIfRequested(expected);
var actual = GetTable(File.ReadAllText(Path.Combine(DocumentsDirectory.FullName, "index.md")), headerRow, tableNumber);
CodeAssert.AreEqual(expected, actual);
}
Expand Down Expand Up @@ -254,7 +254,7 @@ public void EnsureThatSuppressionIndexIsAsExpected(int tableNumber)
}

var expected = builder.ToString();
DumpIfDebug(expected);
DumpIfRequested(expected);
var actual = GetTable(File.ReadAllText(Path.Combine(DocumentsDirectory.FullName, "index.md")), headerRow, tableNumber);
CodeAssert.AreEqual(expected, actual);
}
Expand Down Expand Up @@ -290,8 +290,8 @@ int TableLength()
}
}

[Conditional("DEBUG")]
private static void DumpIfDebug(string text)
[Conditional("DUMP_DOCUMENTATION")]
private static void DumpIfRequested(string text)
{
Console.Write(text);
Console.WriteLine();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,32 @@ public void Test(byte[] buffer) { }
RoslynAssert.Valid(this.analyzer, testCode);
}

[Test]
public void AnalyzeParameterIsArrayOfObjectWithGoodArguments()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
class AnalyzeArgumentIsArrayOfObject
{
[TestCase(new object[] { 1, 2.0 })]
public void Test(int i, double d) { }
}");
RoslynAssert.Valid(this.analyzer, testCode);
}

[Test]
public void AnalyzeParameterIsArrayOfObjectWithBadArguments()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
class AnalyzeArgumentIsArrayOfObject
{
[TestCase(new object[] { 1, ↓2.0 })]
public void Test(int i, int ii) { }
}");
RoslynAssert.Diagnostics(this.analyzer,
ExpectedDiagnostic.Create(AnalyzerIdentifiers.TestCaseParameterTypeMismatchUsage),
testCode);
}

[Test]
public void AnalyzeArgumentIsStringConvertedToEnum()
{
Expand Down Expand Up @@ -386,14 +412,56 @@ public void Test(char a) { }
public void AnalyzeWhenArgumentPassesNullToNullableType()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public sealed class AnalyzeWhenArgumentPassesNullToNullableType
public sealed class AnalyzeWhenArgumentPassesNullToNullableValueType
{
[TestCase(null)]
public void Test(int? a) { }
}");
RoslynAssert.Valid(this.analyzer, testCode);
}

[Test]
public void AnalyzeWhenArgumentPassesNullToNullableReferenceType()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public sealed class AnalyzeWhenArgumentPassesNullToNullableType
{
[TestCase(null)]
public void Test(object? a) { }
}");
RoslynAssert.Valid(this.analyzer, testCode);
}

[Test]
public void AnalyzeWhenArgumentPassesNullToUnspecifiedReferenceType()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
#nullable disable
public sealed class AnalyzeWhenArgumentPassesNullToNullableType
{
[TestCase(null)]
public void Test(object a) { }
}");
RoslynAssert.Valid(this.analyzer, testCode);
}

[Test]
public void AnalyzeWhenArgumentPassesNullToNonNullableReferenceType()
{
var expectedDiagnostic = ExpectedDiagnostic.Create(
AnalyzerIdentifiers.TestCaseParameterTypeMismatchUsage,
string.Format(CultureInfo.InvariantCulture,
TestCaseUsageAnalyzerConstants.ParameterTypeMismatchMessage, 0, "<null>", "a", "object"));

var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
public sealed class AnalyzeWhenArgumentPassesNullToNullableType
{
[TestCase(↓null)]
public void Test(object a) { }
}");
RoslynAssert.Diagnostics(this.analyzer, expectedDiagnostic, testCode);
}

[Test]
public void AnalyzeWhenArgumentPassesValueToNullableType()
{
Expand Down Expand Up @@ -534,7 +602,7 @@ public void AnalyzeWhenMethodHasOnlyParamsAndArgumentPassesNullToReferenceType()
public sealed class AnalyzeWhenMethodHasOnlyParamsAndArgumentPassesNullToReferenceType
{
[TestCase(null)]
public void Test(params string[] a) { }
public void Test(params string[]? a) { }
}");
RoslynAssert.Valid(this.analyzer, testCode);
}
Expand Down
Loading