CA1309 violation is observed when using == between strings with .NET 4.5.1

[srivatsn]

Ported from dotnet/roslyn#14355 reported by @wargamer
Version Used: 1.1.0

Steps to Reproduce:

Create a Console Application
Write code like string a = "a"; string b = "b"; if(a == b) { }
Observe a CA1309 violation
Expected Behavior:
I expect a CA1309 violation when I compare strings with something other than StringComparison.Ordinal
Actual Behavior:
A CA1309 violation pops up when I use == which, according to the reference source, does a comparison with StringComparison.Ordinal. And that should not generate a CA1309

It might also be worth to note that I'm building against .NET 4.5.1 and I also made a StackOverflow post regarding this issue.

[mavasani]

Verify if this repoes on latest bits..

[jinujoseph]

@mattscheffer re verified and confirmed it to be No-repro

[gh3x78E]

Milestone 15.3 relates to which version of the nuget package? I have Microsoft.CodeAnalysis.FxCopAnalyzers 1.1.0 and I'm getting the warning with .Net 4.5.1 and 4.6.1.

[jinujoseph]

No 15.3 maps to our VS release milestone.
We were not able to repro this with our latest nuget here .

Feel free to re-open the issue if you can repro this with the latest nuget.

[onyxmaster]

Please take a look at https://github.com/onyxmaster/repro-ca1309. The problem still persists even with the 2.3.0-beta1.

[msbasanth]

I am using Microsoft FxCop Analyzers v2.3.0-beta1 in my code base. I am seeing violations from CA1309 when using '==' for comparison.
Any idea which version of FxCop Analyzer we have this fix?

[mavasani]

Can you try 2.6.0-beta1? Details here: https://github.com/dotnet/roslyn-analyzers#recommended-version-of-analyzer-packages

[msbasanth]

I am working with Visual Studio 2017 v15.4.4 and tried to import 2.6.0-beta1 analyzers using,

Install-Package Microsoft.CodeAnalysis.FxCopAnalyzers -Version 2.6.0-beta1-62231-02 -Source https://dotnet.myget.org/F/roslyn-analyzers/api/v3/index.json

It looks like analyzer is unable to load properly, I am seeing an info sign in the Analyzers section in References.

As mentioned in the recommended version of analyzer packages, we should use Visual Studio 2017 15.5 Preview 3 for v2.6.0-beta1?

Thanks
Basanth

[333fred]

Yes, although it's Preview 4 now. 2.6.0-beta1 will not run on 15.3.

[msbasanth]

In our environment, we are using VS2017 but not strict about which version. If we roll out FxCop Analyzers we have to make sure everybody use the same version of Visual Studio 2017.

Any idea when IOperation API will be shipped so that we will have an analyzer package which won't be tightly tied to a Visual Studio 2017 Version.

[jinujoseph]

Ioperation API are shipped as part of Preview 4 and tentative plan is to go RTW by end of the year.

[onyxmaster]

VS 15.5.1 still has this problem, even though I refreshed https://github.com/onyxmaster/repro-ca1309 to use 2.6.0-beta1 analyzers.

>dotnet --info
.NET Command Line Tools (2.1.2)

Product Information:
 Version:            2.1.2
 Commit SHA-1 hash:  5695315371

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.16299
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.1.2\

Microsoft .NET Core Shared Framework Host

  Version  : 2.0.3
  Build    : a9190d4a75f4a982ae4b4fa8d1a24526566c69df

>dotnet build
Microsoft (R) Build Engine version 15.5.179.9764 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restoring packages for C:\Users\xm\Projects\repro-ca1309\Test.csproj...
  Generating MSBuild file C:\Users\xm\Projects\repro-ca1309\obj\Test.csproj.nuget.g.props.
  Generating MSBuild file C:\Users\xm\Projects\repro-ca1309\obj\Test.csproj.nuget.g.targets.
  Restore completed in 108,85 ms for C:\Users\xm\Projects\repro-ca1309\Test.csproj.
TestClass.cs(3,25): warning CA1309: Use ordinal stringcomparison [C:\Users\xm\Projects\repro-ca1309\Test.csproj]
  Test -> C:\Users\xm\Projects\repro-ca1309\bin\Debug\netstandard2.0\Test.dll

Build succeeded.

TestClass.cs(3,25): warning CA1309: Use ordinal stringcomparison [C:\Users\xm\Projects\repro-ca1309\Test.csproj]
    1 Warning(s)
    0 Error(s)

Any other version of VS I should look into, or was this case closed prematurely? Should I mention someone explicitly to take a look at this?

[333fred]

@onyxmaster we'll look into it.

[AArnott]

The stable 2.6.0 package still repros this.

[333fred]

@mavasani, do you know any reason why we are analyzing binary expressions in CA1309 at all? As far as I can tell, there's nothing in the original FXCop implementation to analyze them, so I'm not sure why this was added. @srivatsn or @genlu, you're the only other authors I can track through git history (though that was just moving files around), do either of you have any context?

[mavasani]

@333fred I could not find any reason why we are analyzing binary expressions in this analyzer. I also confirmed FxCop does not complain about these cases. Let us match FxCop here and change our existing unit tests to match this behavior.

[mavasani]

This issue did not get fixed from the prior PR. I have sent out a new PR to fix this issue: #1682

Once merged in, I will publish new NuGet packages on NuGet.org, hopefully in a day or two.

[mavasani]

Fixed with #1682