Skip to content

Commit

Permalink
CA1416 fix ambiguity in Version comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
@buyaa-n
buyaa-n committed Mar 11, 2021
1 parent 940c426 commit 6628ba6
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 39 deletions.
12 changes: 4 additions & 8 deletions ...s/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,11 @@ static Version CreateVersion(ArrayBuilder<int> versionBuilder)
{
return new Version(versionBuilder[0], versionBuilder[1]);
}
else
{
return new Version(versionBuilder[0], versionBuilder[1], versionBuilder[2]);
}
}
else
{
return new Version(versionBuilder[0], versionBuilder[1], versionBuilder[2], versionBuilder[3]);

return new Version(versionBuilder[0], versionBuilder[1], versionBuilder[2]);
}

return new Version(versionBuilder[0], versionBuilder[1], versionBuilder[2], versionBuilder[3]);
}
}

Expand Down
45 changes: 22 additions & 23 deletions ...alyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ static bool IsKnownValueGuarded(
if (info.Negated)
{
if (attribute.UnsupportedFirst != null &&
attribute.UnsupportedFirst >= info.Version)
attribute.UnsupportedFirst.IsGreaterThanOrEqualTo(info.Version))
{
if (DenyList(attribute))
{
Expand All @@ -418,7 +418,7 @@ static bool IsKnownValueGuarded(
}

if (attribute.UnsupportedSecond != null &&
attribute.UnsupportedSecond <= info.Version)
info.Version.IsGreaterThanOrEqualTo(attribute.UnsupportedSecond))
{
attribute.UnsupportedSecond = null;
}
Expand All @@ -434,29 +434,29 @@ static bool IsKnownValueGuarded(
{
if (attribute.UnsupportedFirst != null &&
capturedVersions.TryGetValue(info.PlatformName, out var version) &&
attribute.UnsupportedFirst >= version)
attribute.UnsupportedFirst.IsGreaterThanOrEqualTo(version))
{
attribute.UnsupportedFirst = null;
}

if (attribute.UnsupportedSecond != null &&
capturedVersions.TryGetValue(info.PlatformName, out version) &&
attribute.UnsupportedSecond <= version)
version.IsGreaterThanOrEqualTo(attribute.UnsupportedSecond))
{
attribute.UnsupportedSecond = null;
}
}

if (attribute.SupportedFirst != null &&
attribute.SupportedFirst <= info.Version)
info.Version.IsGreaterThanOrEqualTo(attribute.SupportedFirst))
{
attribute.SupportedFirst = null;
RemoveUnsupportedWithLessVersion(info.Version, attribute);
RemoveOtherSupportsOnDifferentPlatforms(attributes, info.PlatformName);
}

if (attribute.SupportedSecond != null &&
attribute.SupportedSecond <= info.Version)
info.Version.IsGreaterThanOrEqualTo(attribute.SupportedSecond))
{
attribute.SupportedSecond = null;
RemoveUnsupportedWithLessVersion(info.Version, attribute);
Expand Down Expand Up @@ -549,8 +549,7 @@ static void RemoveUnsupportsOnDifferentPlatforms(SmallDictionary<string, Version

static void RemoveUnsupportedWithLessVersion(Version supportedVersion, Versions attribute)
{
if (attribute.UnsupportedFirst != null &&
attribute.UnsupportedFirst <= supportedVersion)
if (supportedVersion.IsGreaterThanOrEqualTo(attribute.UnsupportedFirst))
{
attribute.UnsupportedFirst = null;
}
Expand Down Expand Up @@ -887,14 +886,14 @@ static bool HasSameVersionedPlatformSupport(SmallDictionary<string, Versions> at
var supportedVersion = attribute.SupportedSecond ?? attribute.SupportedFirst;
if (supportedVersion != null)
{
version = version == null || supportedVersion >= version ? supportedVersion : version;
version = supportedVersion.IsGreaterThanOrEqualTo(version) ? supportedVersion : version;
}
else
{
var unsupportedVersion = attribute.UnsupportedSecond ?? attribute.UnsupportedFirst;
if (unsupportedVersion != null)
{
version = version == null || unsupportedVersion >= version ? unsupportedVersion : version;
version = unsupportedVersion.IsGreaterThanOrEqualTo(version) ? unsupportedVersion : version;
}
else
{
Expand Down Expand Up @@ -1324,25 +1323,25 @@ static bool UnsupportedSecondSuppressed(Versions attribute, Versions callSiteAtt

static bool SuppressedByCallSiteUnsupported(Versions callSiteAttribute, Version unsupporteAttribute) =>
DenyList(callSiteAttribute) && callSiteAttribute.SupportedFirst != null ?
callSiteAttribute.UnsupportedSecond != null && unsupporteAttribute >= callSiteAttribute.UnsupportedSecond :
callSiteAttribute.UnsupportedFirst != null && unsupporteAttribute >= callSiteAttribute.UnsupportedFirst;
callSiteAttribute.UnsupportedSecond != null && unsupporteAttribute.IsGreaterThanOrEqualTo(callSiteAttribute.UnsupportedSecond) :
callSiteAttribute.UnsupportedFirst != null && unsupporteAttribute.IsGreaterThanOrEqualTo(callSiteAttribute.UnsupportedFirst);

static bool SuppressedByCallSiteSupported(Versions attribute, Version? callSiteSupportedFirst) =>
callSiteSupportedFirst != null && callSiteSupportedFirst >= attribute.SupportedFirst! &&
attribute.SupportedSecond != null && callSiteSupportedFirst >= attribute.SupportedSecond;
callSiteSupportedFirst != null && callSiteSupportedFirst.IsGreaterThanOrEqualTo(attribute.SupportedFirst) &&
attribute.SupportedSecond != null && callSiteSupportedFirst.IsGreaterThanOrEqualTo(attribute.SupportedSecond);

static bool UnsupportedFirstSuppressed(Versions attribute, Versions callSiteAttribute) =>
callSiteAttribute.SupportedFirst != null && callSiteAttribute.SupportedFirst >= attribute.SupportedFirst ||
callSiteAttribute.SupportedFirst != null && callSiteAttribute.SupportedFirst.IsGreaterThanOrEqualTo(attribute.SupportedFirst) ||
SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst!);

// As optional if call site supports that platform, their versions should match
static bool OptionalOsSupportSuppressed(Versions callSiteAttribute, Versions attribute) =>
(callSiteAttribute.SupportedFirst == null || attribute.SupportedFirst <= callSiteAttribute.SupportedFirst) &&
(callSiteAttribute.SupportedSecond == null || attribute.SupportedFirst <= callSiteAttribute.SupportedSecond);
(callSiteAttribute.SupportedFirst == null || callSiteAttribute.SupportedFirst.IsGreaterThanOrEqualTo(attribute.SupportedFirst)) &&
(callSiteAttribute.SupportedSecond == null || callSiteAttribute.SupportedSecond.IsGreaterThanOrEqualTo(attribute.SupportedFirst));

static bool MandatoryOsVersionsSuppressed(Versions callSitePlatforms, Version checkingVersion) =>
callSitePlatforms.SupportedFirst != null && checkingVersion <= callSitePlatforms.SupportedFirst ||
callSitePlatforms.SupportedSecond != null && checkingVersion <= callSitePlatforms.SupportedSecond;
callSitePlatforms.SupportedFirst != null && callSitePlatforms.SupportedFirst.IsGreaterThanOrEqualTo(checkingVersion) ||
callSitePlatforms.SupportedSecond != null && callSitePlatforms.SupportedSecond.IsGreaterThanOrEqualTo(checkingVersion);
}

private static Versions CopyAllAttributes(Versions copyTo, Versions copyFrom)
Expand Down Expand Up @@ -1469,7 +1468,7 @@ static void MergePlatformAttributes(ImmutableArray<AttributeData> immediateAttri
{
childAttribute = NormalizeAttribute(childAttribute);
// only later versions could narrow, other versions ignored
if (childAttribute.SupportedFirst >= attributes.SupportedFirst &&
if (childAttribute.SupportedFirst.IsGreaterThanOrEqualTo(attributes.SupportedFirst) &&
(attributes.SupportedSecond == null || attributes.SupportedSecond < childAttribute.SupportedFirst))
{
attributes.SupportedSecond = childAttribute.SupportedFirst;
Expand All @@ -1478,7 +1477,7 @@ static void MergePlatformAttributes(ImmutableArray<AttributeData> immediateAttri

if (childAttribute.UnsupportedFirst != null)
{
if (childAttribute.UnsupportedFirst <= attributes.SupportedFirst)
if (attributes.SupportedFirst.IsGreaterThanOrEqualTo(childAttribute.UnsupportedFirst))
{
parentAttributes.Callsite = Callsite.Empty;
attributes.SupportedFirst = childAttribute.SupportedFirst > attributes.SupportedFirst ? childAttribute.SupportedFirst : null;
Expand All @@ -1489,7 +1488,7 @@ static void MergePlatformAttributes(ImmutableArray<AttributeData> immediateAttri
attributes.UnsupportedFirst = childAttribute.UnsupportedFirst;
}

if (childAttribute.UnsupportedFirst <= attributes.SupportedSecond)
if (attributes.SupportedSecond.IsGreaterThanOrEqualTo(childAttribute.UnsupportedFirst))
{
attributes.SupportedSecond = null;
}
Expand Down Expand Up @@ -1659,7 +1658,7 @@ static void AddOrUpdateSupportedAttribute(Versions attributes, Version version)
/// <returns>true if it is allow list</returns>
private static bool AllowList(Versions attributes) =>
attributes.SupportedFirst != null &&
(attributes.UnsupportedFirst == null || attributes.SupportedFirst <= attributes.UnsupportedFirst);
(attributes.UnsupportedFirst == null || attributes.UnsupportedFirst.IsGreaterThanOrEqualTo(attributes.SupportedFirst));

/// <summary>
/// Determines if the attributes unsupported only for the platform (deny list)
Expand Down
111 changes: 103 additions & 8 deletions ...NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,17 @@ void M1()
}
else
{
[|WindowsOnlyMethod<BrowserOnlyType>()|]; // should flag for WindowsOnlyMethod method and BrowserOnlyType parameter
[|GenericMethod<WindowsOnlyType, BrowserOnlyType>()|]; // should flag for WindowsOnlyType and BrowserOnlyType parameters
{|#0:WindowsOnlyMethod<BrowserOnlyType>()|}; // should flag for WindowsOnlyMethod method and BrowserOnlyType parameter
{|#1:GenericMethod<WindowsOnlyType, BrowserOnlyType>()|}; // should flag for WindowsOnlyType and BrowserOnlyType parameters
}
}
}
";
await VerifyAnalyzerAsyncCs(csSource, s_msBuildPlatforms,
#pragma warning disable RS0030 // Do not used banned APIs
VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(24, 13).WithArguments("BrowserOnlyType", "'browser'"),
#pragma warning restore RS0030 // Do not used banned APIs
#pragma warning disable RS0030 // Do not used banned APIs
VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(25, 13).WithArguments("WindowsOnlyType", "'windows'"));
#pragma warning restore RS0030 // Do not used banned APIs
VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(0).WithArguments("BrowserOnlyType", "'browser'"),
VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(0).WithArguments("Test.WindowsOnlyMethod<BrowserOnlyType>()", "'windows'"),
VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(1).WithArguments("WindowsOnlyType", "'windows'"),
VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(1).WithArguments("BrowserOnlyType", "'browser'"));
}

[Fact]
Expand Down Expand Up @@ -163,6 +161,103 @@ void Api() { }
await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms);
}

[Fact, WorkItem(4932, "https://github.com/dotnet/roslyn-analyzers/issues/4932")]
public async Task GuardMethodWith3VersionPartsEquavalentTo4PartsWithLeading0()
{
var source = @"
using System.Runtime.Versioning;
using System;
public class MSAL
{
[SupportedOSPlatform(""windows10.0.17763.0"")]
public static void UseWAMWithLeading0() { }
[SupportedOSPlatform(""windows10.0.17763"")]
public static void UseWAMNoLeading0() { }
static void Test()
{
if (OperatingSystem.IsWindowsVersionAtLeast(10, 0, 17763, 0))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}
if (OperatingSystem.IsWindowsVersionAtLeast(10, 0, 17763))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}
}
}";

await VerifyAnalyzerAsyncCs(source);
}

[Fact]
public async Task GuardMethodWith1VersionPartsEquavalentTo2PartsWithLeading0()
{
var source = @"
using System.Runtime.Versioning;
using System;
public class MSAL
{
[SupportedOSPlatform(""windows10.0"")]
public static void UseWAMWithLeading0() { }
static void Test()
{
if (OperatingSystem.IsWindowsVersionAtLeast(10))
{
UseWAMWithLeading0();
}
if (OperatingSystem.IsWindowsVersionAtLeast(10, 0))
{
UseWAMWithLeading0();
}
}
}";

await VerifyAnalyzerAsyncCs(source);
}

[Fact]
public async Task GuardMethodWith2VersionPartsEquavalentTo3PartsWithLeading0()
{
var source = @"
using System.Runtime.Versioning;
using System;
public class MSAL
{
[SupportedOSPlatform(""windows10.1.0"")]
public static void UseWAMWithLeading0() { }
[SupportedOSPlatform(""windows10.1"")]
public static void UseWAMNoLeading0() { }
static void Test()
{
if (OperatingSystem.IsWindowsVersionAtLeast(10, 1))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}
if (OperatingSystem.IsWindowsVersionAtLeast(10, 1, 0))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}
}
}";

await VerifyAnalyzerAsyncCs(source);
}

[Fact]
public async Task GuardsAroundSupported_SimpleIfElse()
{
Expand Down
1 change: 1 addition & 0 deletions src/Utilities/Compiler/Analyzer.Utilities.projitems
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
<Compile Include="$(MSBuildThisFileDirectory)Extensions\OperationBlockAnalysisContextExtension.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Extensions\SourceTextExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Extensions\StringCompatExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Extensions\VersionExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Extensions\WellKnownDiagnosticTagsExtensions.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Index.cs" />
<Compile Include="$(MSBuildThisFileDirectory)IsExternalInit.cs" />
Expand Down
45 changes: 45 additions & 0 deletions src/Utilities/Compiler/Extensions/VersionExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;

namespace Analyzer.Utilities.Extensions
{
internal static class VersionExtension
{
public static bool IsGreaterThanOrEqualTo(this Version? current, Version? compare)
{
if (current == null)
{
return compare == null;
}

if (compare == null)
{
return true;
}

if (current.Major != compare.Major)
{
return current.Major > compare.Major;
}

if (current.Minor != compare.Minor)
{
return current.Minor > compare.Minor;
}

// For build or revision value of 0 equals to -1
if (current.Build != compare.Build && (current.Build > 0 || compare.Build > 0))
{
return current.Build > compare.Build;
}

if (current.Revision != compare.Revision && (current.Revision > 0 || compare.Revision > 0))
{
return current.Revision > compare.Revision;
}

return true;
}
}
}

0 comments on commit 6628ba6

Please sign in to comment.