Analyzers should not just tell you you're doing it wrong but what to do instead

[NinoFloris]

xUnit2013/xUnit2015/xUnit2017/etc are nice but the warnings themselves are not talking about solutions.only about 'wrongdoings'.

Edit:
As not every dev uses an ide and the dotnet cli now runs these analyzers on build they could be a bit more descriptive

[jamesqo]

@NinoFloris Thanks for your feedback! I understand that you feel like we're telling the user what they shouldn't do, and not enough of what they should do.

I didn't write most of the rules you mention, but I believe the reason behind that is we offer code fixes that will show you how to correct your code. For xUnit2017, for example, if you press Ctrl+. in Visual Studio around the site of the warning, it should offer to convert Assert.True(c.Contains(...)) to Assert.Contains(..., c), or Assert.False(c.Contains(...)) to Assert.DoesNotContain(..., c).

/cc @marcind

[NinoFloris]

Problem is that doesn't happen in vscode or I presume not in Rider either for that matter.

…

On Wed, Aug 23, 2017, 02:03 James Ko ***@***.***> wrote: @NinoFloris <https://github.com/ninofloris> Thanks for your feedback! I understand that you feel like we're telling the user what they *shouldn't* do, and not enough of what they *should* do. I didn't write most of the rules you mention, but I believe the reason behind that is we offer code fixes that will show you how to correct your code. For xUnit2017, for example, if you press Ctrl+. in Visual Studio around the site of the warning, it should offer to convert Assert.True(c.Contains(...)) to Assert.Contains(..., c), or Assert.False(c.Contains(...)) to Assert.DoesNotContain(..., c). /cc @marcind <https://github.com/marcind> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1423 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEBfucctYZr2s4MK8r4bab_CjL1iCR9uks5sa2xZgaJpZM4O-z7j> .

[jamesqo]

@NinoFloris Are you saying you got warning in VSCode, but not a codefix? That's strange, since from this issue it looks like VSCode supports neither analyzers nor codefixes yet: dotnet/vscode-csharp#43. Also, Project Rider doesn't use Roslyn.

[NinoFloris]

The dotnet cli gave these as build warnings

…

On Wed, Aug 23, 2017, 02:45 James Ko ***@***.***> wrote: @NinoFloris <https://github.com/ninofloris> Are you saying you got warning in VSCode, but not a codefix? That's strange, since from this issue it looks like VSCode supports neither analyzers nor codefixes yet: dotnet/vscode-csharp#43 <dotnet/vscode-csharp#43>. Also, Project Rider doesn't use Roslyn. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1423 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEBfuQ7C6g98E00MJENBCeF3DPZChWxQks5sa3YIgaJpZM4O-z7j> .

[jamesqo]

@NinoFloris I see. I guess it couldn't hurt to be a little more detailed in the message, then. I'm not an official maintainer, but @marcind if you concur you can mark this up for grabs.

[vcsjones]

Agree. The quick fixes aren't enough to document what the expectation is, especially for non-Visual Studio users. This has resulted in at least one Stack Overflow question.

[julielerman]

This has me going in circles today..."don't use equal to verify collection size" but no hints and I can't figure out another way.

[bradwilson]

The documentation still has holes, but each analyzer should link to the documentation page for the rule, which should include examples of both "wrong" and "right" code. For example: https://xunit.github.io/xunit.analyzers/rules/xUnit2013

These links are included in the "metadata" passed back to the analyzer engine when rules are flagged. Are these links not made visible when the .NET CLI flags the rules?

[julielerman]

I finally woke up to the fact that I was comparing it to 1 and changed it to single. In the meantime, I'm using VS Code and this is what I got on dotnet build before I made the change:

Build succeeded.

UnitTest1.cs(43,13): warning xUnit2013: Do not use Assert.Equal() to check for collection size. [/Users/julielerman/blahblahblah/Demos/v1/test/test.csproj]
    1 Warning(s)
    0 Error(s)

[marcind]

Looks like the descriptor messageFormats could be augmented with a short bit on what should be done instead.

[bradwilson]

Yeah, that's a bummer that they don't include the help link. 😞

[ThorstenHans]

Thanks to @bradwilson for posting the link here. Luckily Assert.Single(collection); works for my test scenario too, but I'd love to see something like:

Assert.CountEquals(10, collection)