CA1416 Fix Version comparison ambiguity

[buyaa-n]

Fixes #4932
The comparison operations of the Version type is a bit strange:

Console.WriteLine(Version.Parse("10.0.17763.0") < Version.Parse("10.0.17763")); // False
Console.WriteLine(Version.Parse("10.0.17763.0") > Version.Parse("10.0.17763")); // True
Console.WriteLine(Version.Parse("10.0.17763.0") == Version.Parse("10.0.17763")); // False

Because of that now [SupportedOSPlatform("windows10.0.17763.0")] is not equal to [SupportedOSPlatform("windows10.0.17763")] because Version.Revision will be defaulted to -1.
Accordingly API with [SupportedOSPlatform("windows10.0.17763.0")] attribute cannot be guarded with OperatingSystem.IsWindowsVersionAtLeast(10, 0, 17763)

Added extension method for version comparison

[codecov]

Codecov Report

Merging #4943 (eeaa073) into release/6.0.1xx-preview3 (940c426) will increase coverage by 0.00%.
The diff coverage is 97.05%.

@@                    Coverage Diff                     @@
##           release/6.0.1xx-preview3    #4943    +/-   ##
==========================================================
  Coverage                     95.57%   95.57%            
==========================================================
  Files                          1175     1176     +1     
  Lines                        269300   269402   +102     
  Branches                      16314    16323     +9     
==========================================================
+ Hits                         257377   257494   +117     
+ Misses                         9804     9787    -17     
- Partials                       2119     2121     +2     

[Youssef1313]

There are still few comparisons left:

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Line 724 in 6628ba6

if (supportedVersion > unsupportedVersion)

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Line 727 in 6628ba6

csAttribute.UnsupportedFirst != null && csAttribute.UnsupportedFirst > supportedVersion))

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Line 801 in 6628ba6

else if (supportedVersion > csAttribute.UnsupportedFirst)

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Line 823 in 6628ba6

if (unsupportedVersion != null && unsupportedVersion > supportedVersion)

roslyn-analyzers/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs

Line 1159 in 6628ba6

if (attribute.UnsupportedFirst == null || attribute.UnsupportedFirst > attribute.SupportedFirst)

(and few more)

Was that intended?

[Youssef1313]

Is there a way to ban the comparison operators for Version?

[buyaa-n]

There are still few comparisons left:
...
Was that intended?

Yes the comparisons with > or < has no issue, only related to >= or <=

Is there a way to ban the comparison operators for Version?

That should be runtime related question, anyway, i don't think so because I heard there was a long discussion (idk when/where though) about the Version comparisons and if this behavior is correct or should be fixed etc, and ended up not fixing. I guess it didn't fix it because the existing behavior was made sense for some other scenarios

[Youssef1313]

That should be runtime related question

I meant using the BannedApiAnalyzer to warn if we use >= or <= for Version. If there is (currently) no way for BannedApiAnalyzer to ban operators, it may be a reasonable feature request.

@mavasani @Evangelink Do you know whether BannedApiAnalyzer supports this?

[mavasani]

Do you know whether BannedApiAnalyzer supports this?

Yes, we can ban usage of specific APIs in NetAnalyzers projects by adding the documentation comment IDs of the APIs to https://github.com/dotnet/roslyn-analyzers/blob/dc515b0412abf13d9d2b8e34543c61c9abbee87b/src/NetAnalyzers/BannedSymbols.txt

[carlossanlop]

@buyaa-n I left a couple of questions.

Can you please address @Youssef1313 's question about the other comparisons that were not modified?

Also, let's try to explore the suggestion to ban <= and >= for Version instances. Maybe we can find a way to ban them only inside the PlatformCompatibilityAnalyzer class.

[buyaa-n]

Can you please address @Youssef1313 's question about the other comparisons that were not modified?

That question is answered here, in short, < or > comparisons unrelated, only >= or <= related to this change

Also, let's try to explore the suggestion to ban <= and >= for Version instances. Maybe we can find a way to ban them only inside the PlatformCompatibilityAnalyzer class.

From what I have seen and what @mavasani answered seems we cannot do that for only PlatformCompatibilityAnalyzer, correct me if I am wrong, i am in doubt if we should do that for the entire NetAnalyzers project, existing Version comparisons might be useful for other analyzers

[carlossanlop]

LGTM. All questions answered.

[buyaa-n]

Followed up with @mavasani, we cannot ban version comparison only for PlatformCompatibilityAnalyzer, i don't think we should ban Version comparison in the entire project, so gonna merge the PR