Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CA1416 Fix Version comparison ambiguity #5079

Merged
merged 1 commit into from
Apr 29, 2021
Merged

CA1416 Fix Version comparison ambiguity #5079

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions ...s/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs
View file
Open in desktop
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
53 changes: 31 additions & 22 deletions ...alyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,8 @@ static bool IsKnownValueGuarded(
{
if (info.Negated)
{
if (attribute.UnsupportedFirst != null)
if (attribute.UnsupportedFirst != null &&
attribute.UnsupportedFirst.IsGreaterThanOrEqualTo(info.Version))
{
if (attribute.UnsupportedFirst >= info.Version)
{
Expand All @@ -435,7 +436,8 @@ static bool IsKnownValueGuarded(
}
}

if (attribute.UnsupportedSecond != null)
if (attribute.UnsupportedSecond != null &&
info.Version.IsGreaterThanOrEqualTo(attribute.UnsupportedSecond))
{
if (attribute.UnsupportedSecond <= info.Version)
{
Expand All @@ -454,29 +456,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 @@ -572,8 +574,7 @@ static void RemoveUnsupportsOnDifferentPlatforms(SmallDictionary<string, Platfor

static void RemoveUnsupportedWithLessVersion(Version supportedVersion, PlatformAttributes attribute)
{
if (attribute.UnsupportedFirst != null &&
attribute.UnsupportedFirst <= supportedVersion)
if (supportedVersion.IsGreaterThanOrEqualTo(attribute.UnsupportedFirst))
{
attribute.UnsupportedFirst = null;
}
Expand Down Expand Up @@ -910,11 +911,19 @@ static bool HasSameVersionedPlatformSupport(SmallDictionary<string, PlatformAttr
var supportedVersion = attribute.SupportedSecond ?? attribute.SupportedFirst;
if (supportedVersion != null)
{
version = version == null || supportedVersion >= version ? supportedVersion : version;
version = supportedVersion.IsGreaterThanOrEqualTo(version) ? supportedVersion : version;
}
else
{
version = supportedVersion;
var unsupportedVersion = attribute.UnsupportedSecond ?? attribute.UnsupportedFirst;
if (unsupportedVersion != null)
{
version = unsupportedVersion.IsGreaterThanOrEqualTo(version) ? unsupportedVersion : version;
}
else
{
version = supportedVersion;
}
}
}
if (version != null && !IsEmptyVersion(version))
Expand Down Expand Up @@ -1263,25 +1272,25 @@ static bool UnsupportedSecondSuppressed(PlatformAttributes attribute, PlatformAt
SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedSecond!);

static bool SuppressedByCallSiteUnsupported(PlatformAttributes callSiteAttribute, Version unsupporteAttribute) =>
callSiteAttribute.UnsupportedFirst != null && unsupporteAttribute >= callSiteAttribute.UnsupportedFirst ||
callSiteAttribute.UnsupportedSecond != null && unsupporteAttribute >= callSiteAttribute.UnsupportedSecond;
callSiteAttribute.UnsupportedFirst != null && unsupporteAttribute.IsGreaterThanOrEqualTo(callSiteAttribute.UnsupportedFirst) ||
callSiteAttribute.UnsupportedSecond != null && unsupporteAttribute.IsGreaterThanOrEqualTo(callSiteAttribute.UnsupportedSecond);

static bool SuppressedByCallSiteSupported(PlatformAttributes 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(PlatformAttributes attribute, PlatformAttributes callSiteAttribute) =>
callSiteAttribute.SupportedFirst != null && callSiteAttribute.SupportedFirst >= attribute.SupportedFirst ||
callSiteAttribute.SupportedFirst != null && callSiteAttribute.SupportedFirst.IsGreaterThanOrEqualTo(attribute.SupportedFirst) ||
SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst!);

// As optianal if call site supports that platform, their versions should match
static bool OptionalOsSupportSuppressed(PlatformAttributes callSiteAttribute, PlatformAttributes 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(PlatformAttributes 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 PlatformAttributes CopyAllAttributes(PlatformAttributes copyTo, PlatformAttributes copyFrom)
Expand Down Expand Up @@ -1387,14 +1396,14 @@ static void MergePlatformAttributes(ImmutableArray<AttributeData> immediateAttri
if (childAttributes.TryGetValue(platform, out var childAttribute))
{
// only later versions could narrow, other versions ignored
if (childAttribute.SupportedFirst > attributes.SupportedFirst)
if (childAttribute.SupportedFirst.IsGreaterThanOrEqualTo(attributes.SupportedFirst))
{
attributes.SupportedSecond = childAttribute.SupportedFirst;
}

if (childAttribute.UnsupportedFirst != null)
{
if (childAttribute.UnsupportedFirst <= attributes.SupportedFirst)
if (attributes.SupportedFirst.IsGreaterThanOrEqualTo(childAttribute.UnsupportedFirst))
{
attributes.SupportedFirst = null;
attributes.SupportedSecond = null;
Expand Down Expand Up @@ -1547,7 +1556,7 @@ static void AddOrUpdateSupportedAttribute(PlatformAttributes attributes, Version
/// <returns>true if it is allow list</returns>
private static bool AllowList(PlatformAttributes 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
97 changes: 97 additions & 0 deletions ...NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,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 (OperatingSystemHelper.IsWindowsVersionAtLeast(10, 0, 17763, 0))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}

if (OperatingSystemHelper.IsWindowsVersionAtLeast(10, 0, 17763))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}
}
}" + MockAttributesCsSource + MockOperatingSystemApiSource;

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 (OperatingSystemHelper.IsWindowsVersionAtLeast(10))
{
UseWAMWithLeading0();
}

if (OperatingSystemHelper.IsWindowsVersionAtLeast(10, 0))
{
UseWAMWithLeading0();
}
}
}" + MockAttributesCsSource + MockOperatingSystemApiSource;

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 (OperatingSystemHelper.IsWindowsVersionAtLeast(10, 1))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}

if (OperatingSystemHelper.IsWindowsVersionAtLeast(10, 1, 0))
{
UseWAMWithLeading0();
UseWAMNoLeading0();
}
}
}" + MockAttributesCsSource + MockOperatingSystemApiSource;

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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
<Compile Include="$(MSBuildThisFileDirectory)DisposeMethodKind.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Extensions\OperationBlockAnalysisContextExtension.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)Lightup\ITypeSymbolExtensions.cs" />
Expand Down
45 changes: 45 additions & 0 deletions src/Utilities/Compiler/Extensions/VersionExtensions.cs
View file
Open in desktop
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;
}
}
}