CA1507 Suppression guidelines are too limited

The documentation states the following for "when to suppress" the rule:

It's safe to suppress a violation of this rule if you're not concerned about the maintainability of your code.

This is not only extremely shortsighted, but also naive. There are situations where the warning does not make sense and definitely should be suppressed and the result of the suppression is a gain of maintainability, not loss.

I went through one such scenario yesterday: I had a parameterized unit test method that was validating the implementation was throwing the correct exception. Something along these lines (not real code but shows the issue):

    [InlineData(1)]
    [InlineData(2)]
    [InlineData(3)]
    public void DoingStuffWithValueThrowsArgumentOutOfRangeException(int someValue)
    {
        // Arrange
        var myObject = new MyObject();

        // Act
        var doingStuffWithValue = () => myObject.DoStuff(someValue);

        // Assert
        doingStuffWithValue.Should()
            .ThrowExactly<ArgumentOutOfRangeException>()
            .WithParameterName("someValue");
    }

Note how the name of the argument on the test method (someValue) matches the name of the argument of the method under test. Visual Studio raises CA1507 in this case, asking me to replace the "someValue" constant with nameof(someValue).

That however is a bad refactor: it creates coupling between 2 unrelated, but coincidentally equally named, variables. By following the advice here, I'd be making my test depend on something that it should not: an external argument on the test. If someone refactored my test later and decided to change the test method argument name for any reason (say, readability, or semantics), the assertion would suddenly start to fail.

I think this suppression guideline should be updated to be less naive and more realistic: there are clearly situations where applying this rule's fix is ill-advised and should not be followed, and that does not mean "the developer just didn't care about maintainability" in that case.

If I'm honest, the current wording even sounds ridiculous: "who would ever not care about maintainability in the first place?". It looks unprofessional in a way.

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 4956d3f2-6966-3a9e-dc29-ba2a61dba3cf
Version Independent ID: 79e5268e-8cf5-cde5-d8cd-7e5bf469ca02
Content: CA1507: Use nameof instead of string (code analysis) - .NET
Content Source: docs/fundamentals/code-analysis/quality-rules/ca1507.md
Product: dotnet-fundamentals
GitHub Login: @gewarren
Microsoft Alias: gewarren

dotnet / docs

CA1507 Suppression guidelines are too limited #28793

Document Details