dotnet / roslyn-analyzers

MIT License
1.58k stars 464 forks source link

CA1822 raises for custom TestMethodAttribute #2785

Closed N-Olbert closed 4 years ago

N-Olbert commented 5 years ago

Analyzer package

Microsoft.CodeQuality.Analyzers

Package Version

v2.9.4

Diagnostic ID

CA1822

Repro steps

MSTestV2 enables custom TestMethodAttributes. For test methods which use these custom attributes a CA1822 is triggered.

    [AttributeUsage(AttributeTargets.Method)]
    public sealed class CustomTestAttribute : TestMethodAttribute
    {
        public override TestResult[] Execute(ITestMethod testMethod)
        {
            Trace.WriteLine("Custom...");
            return base.Execute(testMethod);
        }
    }

    [TestClass]
    public class ATest
    {
        [CustomTest]
        public void Test()
        {
            Assert.IsTrue(true);
        }
    }

If you use a plain TestMethodAttribute instead of the CustomTestAttribute no CA1822 occurs.

Expected behavior

No CA1822 for method AClass.Test

Actual behavior

CA1822 for method AClass.Test

Seems to be related to the fix of #1399 (Change set #1556) where only the plain "TestMethodAttribute" gets skipped.

Evangelink commented 4 years ago

@mavasani I am trying to have a look at this issue and I am facing some issue with the tests. With the architecture currently in place, I don't understand how to add the MsTest v2+ metadatareference dependency to the unit test. Could you help?

FYI, when I was working on Sonar.CSharp analyzer I had to develop some tool to download dependencies "on-the-fly" with NuGet without referencing them in the project (so that I can test multiple versions for example). You can see the classes here:

Evangelink commented 4 years ago

@N-Olbert Are you sure that the code sample you have provided does raise the issue CA1822 make static? I cannot reproduce with the version and code you have provided.

N-Olbert commented 4 years ago

@Evangelink I will verify this next week once again and let you know. If I can still reproduce it I will provide a working project sample.

Thanks for the effort :)

N-Olbert commented 4 years ago

@Evangelink I can confirm you are right. The issue indeed doesn't occur with the NugGet-Package-Analyzer, it only occurs if you are using the built in binary code analysis wich ships with visual studio.

I guess it is not an Issue anymore then.

Sorry for the inconvenience, I must have noticed this issue in an outdated project.

Evangelink commented 4 years ago

@mavasani I guess the ticket can be closed. Regarding the PR, are you interested in merging the new test or shall I close it?

Evangelink commented 4 years ago

@mavasani ping

mavasani commented 4 years ago

@Evangelink sorry, will close this issue and merge your test PR.