From c61e3f44d459e2a7fb7c35afce0e40d8de7558ba Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 29 Apr 2021 13:40:37 -0700 Subject: [PATCH 1/6] Support custom guard with attributes --- ...mCompatibilityAnalyzer.OperationVisitor.cs | 75 +++- .../PlatformCompatibilityAnalyzer.Value.cs | 46 +- .../PlatformCompatibilityAnalyzer.cs | 87 ++-- ...tibilityAnalyzerTests.GuardedCallsTests.cs | 396 ++++++++++++++++++ .../PlatformCompatibilityAnalyzerTests.cs | 16 +- 5 files changed, 563 insertions(+), 57 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs index 32ef050ccd..8c806bd5a8 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs @@ -1,6 +1,7 @@ // 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.Collections.Immutable; +using System.Linq; using Analyzer.Utilities.PooledObjects; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.GlobalFlowStateAnalysis; @@ -25,6 +26,32 @@ public OperationVisitor( _osPlatformType = osPlatformType; } + internal static bool TryParseGuardAttrbiutes(ISymbol symbol, ref GlobalFlowStateAnalysisValueSet value) + { + if (HasAnyGuardAttribute(symbol)) + { + using var infosBuilder = ArrayBuilder.GetInstance(); + if (PlatformMethodValue.TryDecode(symbol.GetAttributes(), infosBuilder)) + { + for (var i = 0; i < infosBuilder.Count; i++) + { + var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); + value = i == 0 ? newValue : infosBuilder[i].Negated ? value.WithAdditionalAnalysisValues(newValue, false) : + GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); + } + + return true; + } + + value = GlobalFlowStateAnalysisValueSet.Unknown; + } + + return false; + + static bool HasAnyGuardAttribute(ISymbol symbol) => + symbol.GetAttributes().Any(a => a.AttributeClass.Name is SupportedOSPlatformGuardAttribute or UnsupportedOSPlatformGuardAttribute); + } + public override GlobalFlowStateAnalysisValueSet VisitInvocation_NonLambdaOrDelegateOrLocalFunction( IMethodSymbol method, IOperation? visitedInstance, @@ -35,25 +62,57 @@ public override GlobalFlowStateAnalysisValueSet VisitInvocation_NonLambdaOrDeleg { var value = base.VisitInvocation_NonLambdaOrDelegateOrLocalFunction(method, visitedInstance, visitedArguments, invokedAsDelegate, originalOperation, defaultValue); - if (_platformCheckMethods.Contains(method.OriginalDefinition)) + if (method.ReturnType.SpecialType == SpecialType.System_Boolean) { - using var infosBuilder = ArrayBuilder.GetInstance(); - if (PlatformMethodValue.TryDecode(method, visitedArguments, DataFlowAnalysisContext.ValueContentAnalysisResult, _osPlatformType, infosBuilder)) + if (_platformCheckMethods.Contains(method.OriginalDefinition)) { - for (var i = 0; i < infosBuilder.Count; i++) + using var infosBuilder = ArrayBuilder.GetInstance(); + if (PlatformMethodValue.TryDecode(method, visitedArguments, DataFlowAnalysisContext.ValueContentAnalysisResult, _osPlatformType, infosBuilder)) { - var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); - value = i == 0 ? newValue : GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); + for (var i = 0; i < infosBuilder.Count; i++) + { + var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); + value = i == 0 ? newValue : GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); + } + + return value; } + return GlobalFlowStateAnalysisValueSet.Unknown; + } + else if (TryParseGuardAttrbiutes(method, ref value)) + { return value; } - - return GlobalFlowStateAnalysisValueSet.Unknown; } return value; } + + public override GlobalFlowStateAnalysisValueSet VisitFieldReference(IFieldReferenceOperation operation, object? argument) + { + var value = base.VisitFieldReference(operation, argument); + + if (operation.Type.SpecialType == SpecialType.System_Boolean && + TryParseGuardAttrbiutes(operation.Field, ref value)) + { + return value; + } + + return ComputeAnalysisValueForReferenceOperation(operation, value); + } + + public override GlobalFlowStateAnalysisValueSet VisitPropertyReference(IPropertyReferenceOperation operation, object? argument) + { + var value = base.VisitPropertyReference(operation, argument); + if (operation.Type.SpecialType == SpecialType.System_Boolean && + TryParseGuardAttrbiutes(operation.Property, ref value)) + { + return value; + } + + return ComputeAnalysisValueForReferenceOperation(operation, value); + } } } } diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs index b4a53a7523..f895d1c07b 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs @@ -22,21 +22,19 @@ public sealed partial class PlatformCompatibilityAnalyzer { private readonly struct PlatformMethodValue : IAbstractAnalysisValue, IEquatable { - private PlatformMethodValue(string invokedPlatformCheckMethodName, string platformPropertyName, Version version, bool negated) + internal PlatformMethodValue(string platformPropertyName, Version version, bool negated) { - InvokedMethodName = invokedPlatformCheckMethodName ?? throw new ArgumentNullException(nameof(invokedPlatformCheckMethodName)); PlatformName = platformPropertyName ?? throw new ArgumentNullException(nameof(platformPropertyName)); Version = version ?? throw new ArgumentNullException(nameof(version)); Negated = negated; } - public string InvokedMethodName { get; } public string PlatformName { get; } public Version Version { get; } public bool Negated { get; } public IAbstractAnalysisValue GetNegatedValue() - => new PlatformMethodValue(InvokedMethodName, PlatformName, Version, !Negated); + => new PlatformMethodValue(PlatformName, Version, !Negated); public static bool TryDecode( IMethodSymbol invokedPlatformCheckMethod, @@ -50,7 +48,7 @@ public static bool TryDecode( { if (TryExtractPlatformName(invokedPlatformCheckMethod.Name, out var platformName)) { - var info = new PlatformMethodValue(invokedPlatformCheckMethod.Name, platformName, new Version(0, 0), negated: false); + var info = new PlatformMethodValue(platformName, EmptyVersion, negated: false); infosBuilder.Add(info); return true; } @@ -63,7 +61,7 @@ public static bool TryDecode( Debug.Assert(osPlatformNamesBuilder.Count > 0); for (var i = 0; i < osPlatformNamesBuilder.Count; i++) { - var info = new PlatformMethodValue(invokedPlatformCheckMethod.Name, osPlatformNamesBuilder[i], new Version(0, 0), negated: false); + var info = new PlatformMethodValue(osPlatformNamesBuilder[i], EmptyVersion, negated: false); infosBuilder.Add(info); } @@ -79,7 +77,7 @@ public static bool TryDecode( if (invokedPlatformCheckMethod.Name == IsOSPlatform && TryParsePlatformNameAndVersion(literal.ConstantValue.Value.ToString(), out string platformName, out Version? version)) { - var info = new PlatformMethodValue(invokedPlatformCheckMethod.Name, platformName, version, negated: false); + var info = new PlatformMethodValue(platformName, version, negated: false); infosBuilder.Add(info); return true; } @@ -87,7 +85,7 @@ public static bool TryDecode( { // OperatingSystem.IsOSPlatformVersionAtLeast(string platform, int major, int minor = 0, int build = 0, int revision = 0) Debug.Assert(invokedPlatformCheckMethod.Name == "IsOSPlatformVersionAtLeast"); - var info = new PlatformMethodValue(invokedPlatformCheckMethod.Name, literal.ConstantValue.Value.ToString(), version, negated: false); + var info = new PlatformMethodValue(literal.ConstantValue.Value.ToString(), version, negated: false); infosBuilder.Add(info); return true; } @@ -98,7 +96,7 @@ public static bool TryDecode( if (TryExtractPlatformName(invokedPlatformCheckMethod.Name, out var platformName) && TryDecodeOSVersion(arguments, valueContentAnalysisResult, out var version)) { - var info = new PlatformMethodValue(invokedPlatformCheckMethod.Name, platformName, version, negated: false); + var info = new PlatformMethodValue(platformName, version, negated: false); infosBuilder.Add(info); return true; } @@ -109,6 +107,26 @@ public static bool TryDecode( return false; } + public static bool TryDecode(ImmutableArray attributes, ArrayBuilder infosBuilder) + { + foreach (var attribute in attributes) + { + if (attribute.AttributeClass.Name is SupportedOSPlatformGuardAttribute or UnsupportedOSPlatformGuardAttribute && + HasNonEmptyStringArgument(attribute, out var argument) && + TryParsePlatformNameAndVersion(argument, out string platformName, out Version? version)) + { + if (platformName.Equals(OSX, StringComparison.OrdinalIgnoreCase)) + { + platformName = macOS; + } + + var info = new PlatformMethodValue(platformName, version, negated: attribute.AttributeClass.Name == UnsupportedOSPlatformGuardAttribute); + infosBuilder.Add(info); + } + } + return infosBuilder.Any(); + } + private static bool TryExtractPlatformName(string methodName, [NotNullWhen(true)] out string? platformName) { if (!methodName.StartsWith(IsPrefix, StringComparison.Ordinal)) @@ -225,7 +243,7 @@ static Version CreateVersion(ArrayBuilder versionBuilder) public override string ToString() { - var result = $"{InvokedMethodName};{PlatformName};{Version}"; + var result = $"{PlatformName};{Version}"; if (Negated) { result = $"!{result}"; @@ -235,10 +253,9 @@ public override string ToString() } public bool Equals(PlatformMethodValue other) - => InvokedMethodName.Equals(other.InvokedMethodName, StringComparison.OrdinalIgnoreCase) && - PlatformName.Equals(other.PlatformName, StringComparison.OrdinalIgnoreCase) && - Version.Equals(other.Version) && - Negated == other.Negated; + => PlatformName.Equals(other.PlatformName, StringComparison.OrdinalIgnoreCase) && + Version.Equals(other.Version) && + Negated == other.Negated; public override bool Equals(object obj) => obj is PlatformMethodValue otherInfo && Equals(otherInfo); @@ -246,7 +263,6 @@ public override bool Equals(object obj) public override int GetHashCode() { return RoslynHashCode.Combine( - InvokedMethodName.GetHashCode(), PlatformName.GetHashCode(), Version.GetHashCode(), Negated.GetHashCode()); diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs index 335b2c5499..49484c882a 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs @@ -49,6 +49,8 @@ public sealed partial class PlatformCompatibilityAnalyzer : DiagnosticAnalyzer // version of internal attribute type which will cause ambiguity, to avoid that we are comparing the attributes by their name private const string SupportedOSPlatformAttribute = nameof(SupportedOSPlatformAttribute); private const string UnsupportedOSPlatformAttribute = nameof(UnsupportedOSPlatformAttribute); + private const string UnsupportedOSPlatformGuardAttribute = nameof(UnsupportedOSPlatformGuardAttribute); + private const string SupportedOSPlatformGuardAttribute = nameof(SupportedOSPlatformGuardAttribute); // Platform guard method name, prefix, suffix private const string IsOSPlatform = nameof(IsOSPlatform); @@ -56,7 +58,9 @@ public sealed partial class PlatformCompatibilityAnalyzer : DiagnosticAnalyzer private const string OptionalSuffix = "VersionAtLeast"; private const string Net = "net"; private const string macOS = nameof(macOS); + private const string OSX = nameof(OSX); private const string MacSlashOSX = "macOS/OSX"; + private static readonly Version EmptyVersion = new(0, 0); internal static DiagnosticDescriptor OnlySupportedCsReachable = DiagnosticDescriptorHelper.Create(RuleId, s_localizableTitle, @@ -418,9 +422,13 @@ static bool IsKnownValueGuarded( } attribute.UnsupportedFirst = null; } + else if (value.AnalysisValues.Contains(new PlatformMethodValue(info.PlatformName, EmptyVersion, false))) + { + csAttributes = SetCallSiteUnsupportedAttribute(csAttributes, info); + } if (attribute.UnsupportedSecond != null && - info.Version.IsGreaterThanOrEqualTo(attribute.UnsupportedSecond)) + attribute.UnsupportedSecond.IsGreaterThanOrEqualTo(info.Version)) { attribute.UnsupportedSecond = null; } @@ -456,16 +464,12 @@ static bool IsKnownValueGuarded( RemoveUnsupportedWithLessVersion(info.Version, attribute); RemoveOtherSupportsOnDifferentPlatforms(attributes, info.PlatformName); } - - if (attribute.SupportedSecond != null && - info.Version.IsGreaterThanOrEqualTo(attribute.SupportedSecond)) + else { - attribute.SupportedSecond = null; - RemoveUnsupportedWithLessVersion(info.Version, attribute); - RemoveOtherSupportsOnDifferentPlatforms(attributes, info.PlatformName); + capturedVersions.TryGetValue(info.PlatformName, out var unsupportedVersion); + csAttributes = SetCallSiteSupportedAttribute(csAttributes, info, unsupportedVersion); } - csAttributes = SetAsCallSiteSupportedAttribute(csAttributes, info); RemoveUnsupportsOnDifferentPlatforms(attributes, info.PlatformName); } } @@ -473,7 +477,7 @@ static bool IsKnownValueGuarded( { // it is checking one exact platform, other unsupported should be suppressed RemoveUnsupportsOnDifferentPlatforms(attributes, info.PlatformName); - csAttributes = SetAsCallSiteSupportedAttribute(csAttributes, info); + csAttributes = SetCallSiteSupportedAttribute(csAttributes, info, null); } } } @@ -510,12 +514,11 @@ static bool IsKnownValueGuarded( return true; } - static SmallDictionary SetAsCallSiteSupportedAttribute(SmallDictionary? csAttributes, PlatformMethodValue info) + static SmallDictionary SetCallSiteSupportedAttribute(SmallDictionary? csAttributes, + PlatformMethodValue info, Version? unsupportedVersion) { - if (csAttributes == null) - { - csAttributes = new SmallDictionary(StringComparer.OrdinalIgnoreCase); - } + csAttributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); + if (csAttributes.TryGetValue(info.PlatformName, out var attributes)) { if (attributes.SupportedFirst == null) @@ -526,11 +529,36 @@ static SmallDictionary SetAsCallSiteSupportedAttribute(SmallDi { attributes.SupportedSecond = info.Version; } + attributes.UnsupportedFirst = unsupportedVersion; } else { - csAttributes.Add(info.PlatformName, new Versions() { SupportedFirst = info.Version }); + csAttributes.Add(info.PlatformName, new Versions() { SupportedFirst = info.Version, UnsupportedFirst = unsupportedVersion }); } + + return csAttributes; + } + + static SmallDictionary SetCallSiteUnsupportedAttribute(SmallDictionary? csAttributes, PlatformMethodValue info) + { + csAttributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); + + if (csAttributes.TryGetValue(info.PlatformName, out var attributes)) + { + if (attributes.UnsupportedFirst == null) + { + attributes.UnsupportedFirst = info.Version; + } + else + { + attributes.UnsupportedSecond = info.Version; + } + } + else + { + csAttributes.Add(info.PlatformName, new Versions() { UnsupportedFirst = info.Version }); + } + return csAttributes; } @@ -1590,16 +1618,11 @@ static Versions NormalizeAttribute(Versions attributes) private static bool TryAddValidAttribute([NotNullWhen(true)] ref SmallDictionary? attributes, AttributeData attribute) { - if (!attribute.ConstructorArguments.IsEmpty && - attribute.ConstructorArguments[0] is { } argument && - argument.Kind == TypedConstantKind.Primitive && - argument.Type.SpecialType == SpecialType.System_String && - !argument.IsNull && - !argument.Value.Equals(string.Empty) && - TryParsePlatformNameAndVersion(argument.Value.ToString(), out string platformName, out Version? version)) + if (HasNonEmptyStringArgument(attribute, out var argument) && + TryParsePlatformNameAndVersion(argument, out var platformName, out var version)) { attributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); - if (platformName.Equals("OSX", StringComparison.OrdinalIgnoreCase)) + if (platformName.Equals(OSX, StringComparison.OrdinalIgnoreCase)) { platformName = macOS; } @@ -1616,6 +1639,22 @@ attribute.ConstructorArguments[0] is { } argument && return false; } + private static bool HasNonEmptyStringArgument(AttributeData attribute, [NotNullWhen(true)] out string? stringArgument) + { + if (!attribute.ConstructorArguments.IsEmpty && + attribute.ConstructorArguments[0] is { } argument && + argument.Type.SpecialType == SpecialType.System_String && + !argument.IsNull && + !argument.Value.Equals(string.Empty)) + { + stringArgument = argument.Value.ToString(); + return true; + } + + stringArgument = null; + return false; + } + private static bool TryParsePlatformNameAndVersion(string osString, out string osPlatformName, [NotNullWhen(true)] out Version? version) { version = null; @@ -1636,7 +1675,7 @@ private static bool TryParsePlatformNameAndVersion(string osString, out string o } osPlatformName = osString; - version = new Version(0, 0); + version = EmptyVersion; return true; } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs index bcd737c54c..30f549c034 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs @@ -3945,6 +3945,402 @@ bool IsWindows11OrLater() await VerifyAnalyzerAsyncCs(source, "dotnet_code_quality.interprocedural_analysis_kind = ContextSensitive"); } + [Fact] + public async Task GuardMembersWithSupportedGuardAttributes() + { + var source = @" +using System; +using System.Diagnostics; +using System.Runtime.Versioning; + +class Test +{ + [SupportedOSPlatformGuard(""linux"")] + private bool IsLunuxSupported() => true; + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""Windows"")] + internal bool LinuxAndWindowsSupported { get; set; } + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""macOS"")] + [SupportedOSPlatformGuard(""Windows"")] + private readonly bool _http3Enabled; + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""macOS"")] + [SupportedOSPlatformGuard(""Windows"")] + [SupportedOSPlatformGuard(""Android"")] + private bool IsHttp3PlusAndroidEnabled() => false; + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""macOS"")] + [SupportedOSPlatformGuard(""Windows"")] + [UnsupportedOSPlatformGuard(""Android"")] + private readonly bool _http3EnabledNotAndroid; + + void M1() + { + if (IsLunuxSupported()) // one of the support guarded, so no warning + { + SupportedOnWindowsLinuxOsx(); + SupportedOnLinux(); + } + + if (_http3Enabled) + { + //SupportedOnWindowsLinuxOsx(); + [|SupportedOnLinux()|]; // only supported on linux but call site is reachable on 3, linus, windows, macos + } + else + { + [|SupportedOnWindowsLinuxOsx()|]; // This call site is reachable on all platforms. 'Test.SupportedOnWindowsLinuxOsx()' is only supported on: 'macOS/OSX', 'Linux', 'windows'. + {|#0:SupportedOnLinux()|}; // This call site is reachable on all platforms. 'Test.SupportedOnLinux()' is only supported on: 'linux'. + } + + if (IsHttp3PlusAndroidEnabled()) // Android is not in the support list + { + [|SupportedOnWindowsLinuxOsx()|]; // This call site is reachable on: 'Android'. 'Test.M2()' is only supported on: 'windows', 'macOS/OSX', 'Linux'. + } + + if (_http3EnabledNotAndroid) + { + SupportedOnWindowsLinuxOsx(); + [|SupportedOnLinux()|]; + } + + [|SupportedOnWindowsLinuxOsx()|]; + Debug.Assert(LinuxAndWindowsSupported); + SupportedOnWindowsLinuxOsx(); + [|SupportedOnLinux()|]; + } + + [SupportedOSPlatform(""windows"")] + [SupportedOSPlatform(""Linux"")] + [SupportedOSPlatform(""OSX"")] + void SupportedOnWindowsLinuxOsx() { } + + [SupportedOSPlatform(""linux"")] + void SupportedOnLinux() { } +}" + MockAttributesCsSource; + + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms, + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(0). + WithArguments("Test.SupportedOnLinux()", "'linux'")); + } + + [Fact] + public async Task GuardMembersWithUnsupportedGuardAttributes() + { + var source = @" +using System; +using System.Diagnostics; +using System.Runtime.Versioning; + +class Test +{ + [UnsupportedOSPlatformGuard(""linux"")] + private readonly bool _linuxNotSupported; + + [UnsupportedOSPlatformGuard(""linux"")] + [UnsupportedOSPlatformGuard(""Windows"")] + public bool LinuxAndWindowsNotSupported { get; } + + [UnsupportedOSPlatformGuard(""linux"")] + [UnsupportedOSPlatformGuard(""ios"")] + [UnsupportedOSPlatformGuard(""Windows"")] + private bool IsLinuxWindowsIosNotSupported() => true; + + [UnsupportedOSPlatformGuard(""linux"")] + [UnsupportedOSPlatformGuard(""ios"")] + [UnsupportedOSPlatformGuard(""Windows"")] + [UnsupportedOSPlatformGuard(""Android"")] + private readonly bool _linuxWindowsIosAndroidNotSupported; + + void M1() + { + if (_linuxNotSupported) + { + UnsupportedOnLinux(); + [|UnsupportedOnLinuxWindowsIos()|]; // This call site is reachable on all platforms. 'Test.UnsupportedOnLinuxWindowsIos()' is unsupported on: 'windows', 'ios'. + } + + if (LinuxAndWindowsNotSupported) + { + UnsupportedOnLinux(); + {|#0:UnsupportedOnLinuxWindowsIos()|}; // This call site is reachable on all platforms. 'Test.UnsupportedOnLinuxWindowsIos()' is unsupported on: 'ios'. + } + + if (_linuxWindowsIosAndroidNotSupported) + { + UnsupportedOnLinux(); + UnsupportedOnLinuxWindowsIos(); + } + else + { + [|UnsupportedOnLinux()|]; // This call site is reachable on: 'Windows', 'linux', 'Android'. 'Test.UnsupportedOnLinux()' is unsupported on: 'Linux'. + [|UnsupportedOnLinuxWindowsIos()|]; // This call site is reachable on: 'Android', 'Windows'. 'Test.UnsupportedOnLinuxWindowsIos()' is unsupported on: 'Linux', 'ios', 'windows'. + } + + Debug.Assert(IsLinuxWindowsIosNotSupported()); + UnsupportedOnLinux(); + UnsupportedOnLinuxWindowsIos(); + } + + + [UnsupportedOSPlatform(""Linux"")] + void UnsupportedOnLinux() { } + + [UnsupportedOSPlatform(""windows"")] + [UnsupportedOSPlatform(""Linux"")] + [UnsupportedOSPlatform(""ios"")] + void UnsupportedOnLinuxWindowsIos() { } +}" + MockAttributesCsSource; + + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms, + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsAllPlatforms).WithLocation(0). + WithArguments("Test.UnsupportedOnLinuxWindowsIos()", "'ios'")); + } + + [Fact] + public async Task GuardMembersWithVersionedSupportedUnsupportedGuardAttributes() + { + var source = @" +using System; +using System.Runtime.Versioning; + +class Test +{ + [SupportedOSPlatformGuard(""Windows10.0"")] + private bool IsWindow10Supported() => true; + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""Windows10.0"")] + [SupportedOSPlatformGuard(""Osx14.1"")] + private readonly bool _linuxAndWindows10MacOS14Supported; + + [UnsupportedOSPlatformGuard(""linux"")] + [UnsupportedOSPlatformGuard(""ios9.0"")] + [UnsupportedOSPlatformGuard(""Windows8.0"")] + private bool LinuxWindows8Ios9NotSupported { get; } + + void M1() + { + if (IsWindow10Supported()) + { + {|#0:UnsupportedOnLinuxWindows10Ios91()|}; // This call site is reachable on: 'Windows' 10.0 and later. 'Test.UnsupportedOnLinuxWindows10Ios91()' is unsupported on: 'windows' 10.0 and later. + SupportedOnWindows10LinuxMacOS14(); + SupportedOnWindows8(); + } + + if (_linuxAndWindows10MacOS14Supported) + { + [|UnsupportedOnLinuxWindows10Ios91()|]; // This call site is reachable on: 'Windows' 10.0 and later. 'Test.UnsupportedOnLinuxWindows10Ios91()' is unsupported on: 'windows' 10.0 and later, 'Linux', 'ios' 9.1 and later. + SupportedOnWindows10LinuxMacOS14(); + [|SupportedOnWindows8()|]; // This call site is reachable on: 'linux'. 'Test.SupportedOnWindows8()' is only supported on: 'windows' 8.0 and later. + } + + if (LinuxWindows8Ios9NotSupported) + { + UnsupportedOnLinuxWindows10Ios91(); + [|SupportedOnWindows10LinuxMacOS14()|]; // This call site is reachable on all platforms. 'Test.SupportedOnWindows10LinuxMacOS14()' is only supported on: 'windows' 10.0 and later, 'Linux', 'macOS' 14.0 and later. + {|#1:SupportedOnWindows8()|}; // This call site is reachable on all platforms. 'Test.SupportedOnWindows8()' is only supported on: 'windows' 8.0 and later. + } + } + + [UnsupportedOSPlatform(""windows10.0"")] + [UnsupportedOSPlatform(""Linux"")] + [UnsupportedOSPlatform(""ios9.1"")] + void UnsupportedOnLinuxWindows10Ios91() { } + + [SupportedOSPlatform(""windows10.0"")] + [SupportedOSPlatform(""Linux"")] + [SupportedOSPlatform(""macOS14.0"")] + void SupportedOnWindows10LinuxMacOS14() { } + + [SupportedOSPlatform(""windows8.0"")] + void SupportedOnWindows8() { } +}" + MockAttributesCsSource; + + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms, + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsReachable).WithLocation(0). + WithArguments("Test.UnsupportedOnLinuxWindows10Ios91()", GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityVersionAndLater, "windows", "10.0"), + GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityVersionAndLater, "Windows", "10.0")), + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsAllPlatforms).WithLocation(1).WithArguments("Test.SupportedOnWindows8()", + GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityVersionAndLater, "windows", "8.0"))); + } + + [Fact] + public async Task GuardMembersWithSupportedUnsupportedVersionRangeGuardAttributes() + { + var source = @" +using System; +using System.Runtime.Versioning; + +class Test +{ + [SupportedOSPlatformGuard(""Windows"")] + [UnsupportedOSPlatformGuard(""Windows10.0"")] + public bool SupportedUntilWindow10 => true; + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""Windows"")] + [UnsupportedOSPlatformGuard(""Windows10.0"")] + private bool IsSupportedUntilWindow10AndLinux() => false; + + [UnsupportedOSPlatformGuard(""ios"")] + [SupportedOSPlatformGuard(""ios14.0"")] + [UnsupportedOSPlatformGuard(""ios18.0"")] + [UnsupportedOSPlatformGuard(""Windows8.0"")] + private readonly bool _windows8IosNotSupportedSupportedIos14_18; + + void M1() + { + if (SupportedUntilWindow10) + { + {|#0:UnsupportedOnWindows8IosSupportsIos14_19()|}; // This call site is reachable on: 'Windows' 10.0 and before. 'Test.UnsupportedOnWindows8IosSupportsIos14_19()' is unsupported on: 'Windows' 8.0 and later. + SupportedOnWindowsUntil10AndLinux(); + SupportedOnWindowsUntil10(); + } + + if (IsSupportedUntilWindow10AndLinux()) + { + [|UnsupportedOnWindows8IosSupportsIos14_19()|]; // This call site is reachable on: 'linux', 'Windows' 10.0 and before. 'Test.UnsupportedOnWindows8IosSupportsIos14_19()' is supported on: 'ios' from version 14.0 to 19.0, 'Windows' 8.0 and later. + SupportedOnWindowsUntil10AndLinux(); + {|#1:SupportedOnWindowsUntil10()|}; // This call site is reachable on: 'linux'. 'Test.SupportedOnWindowsUntil10()' is only supported on: 'Windows'. + } + else + { + [|UnsupportedOnWindows8IosSupportsIos14_19()|]; // This call site is reachable on all platforms. 'Test.UnsupportedOnWindows8IosSupportsIos14_19()' is supported on: 'ios' from version 14.0 to 19.0, 'Windows' 8.0 and later. + [|SupportedOnWindowsUntil10AndLinux()|]; // This call site is reachable on all platforms. 'Test.SupportedOnWindowsUntil10AndLinux()' is only supported on: 'linux', 'Windows' 10.0 and before. + [|SupportedOnWindowsUntil10()|]; // This call site is reachable on all platforms. 'Test.SupportedOnWindowsUntil10()' is only supported on: 'Windows' 10.0 and before. + } + + if (_windows8IosNotSupportedSupportedIos14_18) + { + UnsupportedOnWindows8IosSupportsIos14_19(); + [|SupportedOnWindowsUntil10AndLinux()|]; // This call site is reachable on: 'ios' 14.0 and later. 'Test.SupportedOnWindowsUntil10AndLinux()' is only supported on: 'linux', 'Windows'. + [|SupportedOnWindowsUntil10()|]; // This call site is reachable on: 'ios' 14.0 and later. 'Test.SupportedOnWindowsUntil10()' is only supported on: 'Windows'. + } + } + + [UnsupportedOSPlatform(""ios"")] + [SupportedOSPlatform(""ios14.0"")] + [UnsupportedOSPlatform(""ios19.0"")] + [UnsupportedOSPlatform(""Windows8.0"")] + void UnsupportedOnWindows8IosSupportsIos14_19() { } + + [SupportedOSPlatform(""linux"")] + [SupportedOSPlatform(""Windows"")] + [UnsupportedOSPlatform(""Windows10.0"")] + void SupportedOnWindowsUntil10AndLinux() { } + + [SupportedOSPlatform(""Windows"")] + [UnsupportedOSPlatform(""Windows10.0"")] + void SupportedOnWindowsUntil10() { } +}" + MockAttributesCsSource; + + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms, + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsReachable).WithLocation(0).WithArguments("Test.UnsupportedOnWindows8IosSupportsIos14_19()", + GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityVersionAndLater, "Windows", "8.0"), + GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityVersionAndBefore, "Windows", "10.0")), + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.OnlySupportedCsReachable).WithLocation(1).WithArguments("Test.SupportedOnWindowsUntil10()", "'Windows'", "'linux'")); + } + + [Fact] + public async Task GuardAttributesFalsePositives() + { + var source = @" +using System; +using System.Diagnostics; +using System.Runtime.Versioning; + +class Test +{ + [SupportedOSPlatformGuard(""Windows10.0"")] + private string IsWindow10Supported() => ""true""; + + [SupportedOSPlatformGuard(""linux"")] + [SupportedOSPlatformGuard(""Windows10.0"")] + private bool _linuxAndWindows10Supported; + + [UnsupportedOSPlatformGuard(""linux"")] + [UnsupportedOSPlatformGuard(""ios9.0"")] + private bool LinuxIos9NotSupported { get; set;} + + void M1() + { + if (IsWindow10Supported() == ""true"") // return type string, no effect + { + [|SupportedOnWindows10Linux()|]; + [|SupportedOnWindows8()|]; + } + _linuxAndWindows10Supported = true; // normal statements no effect + IsWindow10Supported(); + [|SupportedOnWindows10Linux()|]; + var value = _linuxAndWindows10Supported; + if (value) + { + IsWindow10Supported(); + [|SupportedOnWindows8()|]; + SupportedOnWindows10Linux(); + } + if (_linuxAndWindows10Supported == true) + { + [|SupportedOnWindows8()|]; + [|SupportedOnWindows10Linux()|]; // 39 + } + var result = LinuxIos9NotSupported == true; // not a guarding a block + [|UnsupportedOnLinuxIos91()|]; + + Debug.Assert(LinuxIos9NotSupported == false); // no effect when there is expression + [|UnsupportedOnLinuxIos91()|]; + + Debug.Assert(LinuxIos9NotSupported); // Assert should work + UnsupportedOnLinuxIos91(); + } + + [UnsupportedOSPlatform(""Linux"")] + [UnsupportedOSPlatform(""ios9.1"")] + void UnsupportedOnLinuxIos91() { } + + [SupportedOSPlatform(""windows10.0"")] + [SupportedOSPlatform(""Linux"")] + void SupportedOnWindows10Linux() { } + + [SupportedOSPlatform(""windows8.0"")] + void SupportedOnWindows8() { } +}" + MockAttributesCsSource; + + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + private readonly string MockAttributesCsSource = @" +namespace System.Runtime.Versioning +{ + [AttributeUsage(AttributeTargets.Class | + AttributeTargets.Method | + AttributeTargets.Property | + AttributeTargets.Field | + AttributeTargets.Struct, + AllowMultiple = true, Inherited = false)] + public sealed class SupportedOSPlatformGuardAttribute : Attribute + { + public SupportedOSPlatformGuardAttribute(string platformName) { } + } + + [AttributeUsage(AttributeTargets.Class | + AttributeTargets.Method | + AttributeTargets.Property | + AttributeTargets.Field | + AttributeTargets.Struct, + AllowMultiple = true, Inherited = false)] + public sealed class UnsupportedOSPlatformGuardAttribute : Attribute + { + public UnsupportedOSPlatformGuardAttribute(string platformName) { } + } +}"; + private readonly string TargetTypesForTest = @" namespace PlatformCompatDemo.SupportedUnupported { diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs index 066d7bc40e..91c434b83b 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs @@ -20,7 +20,7 @@ namespace Microsoft.NetCore.Analyzers.InteropServices.UnitTests { public partial class PlatformCompatabilityAnalyzerTests { - private const string s_msBuildPlatforms = "build_property._SupportedPlatformList=windows,browser,macOS, ios;\nbuild_property.TargetFramework=net5.0"; + private const string s_msBuildPlatforms = "build_property._SupportedPlatformList=windows,browser,macOS, ios, linux;\nbuild_property.TargetFramework=net5.0"; [Fact(Skip = "TODO need to be fixed: Test for for wrong arguments, not sure how to report the Compiler error diagnostic")] public async Task TestOsPlatformAttributesWithNonStringArgument() @@ -1671,14 +1671,14 @@ public static void TestWithMacOsSupported() [UnsupportedOSPlatform(""MacOS"")] public static void TestWithMacOsLinuxSupported() { - Target.UnsupportedOnOSXAndLinux(); + [|Target.UnsupportedOnOSXAndLinux()|]; // This call site is reachable on all platforms. 'Target.UnsupportedOnOSXAndLinux()' is unsupported on: 'linux'. Target.UnsupportedOnOSX14(); } [UnsupportedOSPlatform(""OSX"")] public static void TestWithOsxSupported() { - Target.UnsupportedOnOSXAndLinux(); + [|Target.UnsupportedOnOSXAndLinux()|]; // This call site is reachable on all platforms. 'Target.UnsupportedOnOSXAndLinux()' is unsupported on: 'linux'. Target.UnsupportedOnOSX14(); } @@ -1691,14 +1691,14 @@ public static void TestWithLinuxSupported() public void CrossPlatform() { - {|#1:Target.UnsupportedOnOSXAndLinux()|}; // This call site is reachable on all platforms. 'Target.UnsupportedOnOSXAndLinux()' is unsupported on: 'macOS/OSX'. + [|Target.UnsupportedOnOSXAndLinux()|]; // This call site is reachable on all platforms. 'Target.UnsupportedOnOSXAndLinux()' is unsupported on: 'macOS/OSX'. [|Target.UnsupportedOnOSX14()|]; // This call site is reachable on all platforms. 'Target.UnsupportedOnOSX14()' is unsupported on: 'macOS/OSX' 14.0 and later. } [UnsupportedOSPlatform(""macOs13.0"")] public void TestWithSupportedOnBrowserWarns() { - {|#2:Target.UnsupportedOnOSXAndLinux()|}; // This call site is reachable on: 'macOS/OSX' 13.0 and before. 'Target.UnsupportedOnOSXAndLinux()' is unsupported on: 'macOS/OSX' all versions. + [|Target.UnsupportedOnOSXAndLinux()|]; // This call site is reachable on: 'macOS/OSX' 13.0 and before. 'Target.UnsupportedOnOSXAndLinux()' is unsupported on: 'macOS/OSX' all versions. Target.UnsupportedOnOSX14(); } } @@ -1711,11 +1711,7 @@ public static void UnsupportedOnOSX14() { } } }"; await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms, - VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsReachable).WithLocation(0).WithArguments("Target.UnsupportedOnOSXAndLinux()", "'macOS/OSX'", "'macOS/OSX'"), - VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsAllPlatforms).WithLocation(1).WithArguments("Target.UnsupportedOnOSXAndLinux()", "'macOS/OSX'"), - VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsReachable).WithLocation(2).WithArguments("Target.UnsupportedOnOSXAndLinux()", - GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityAllVersions, "macOS/OSX"), - GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityVersionAndBefore, "macOS/OSX", "13.0"))); + VerifyCS.Diagnostic(PlatformCompatibilityAnalyzer.UnsupportedCsReachable).WithLocation(0).WithArguments("Target.UnsupportedOnOSXAndLinux()", "'macOS/OSX'", "'macOS/OSX'")); } [Fact] From 4664688483d9638bb489ef3511f7f3ef20f7b513 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Thu, 6 May 2021 23:21:06 -0700 Subject: [PATCH 2/6] Do not warn for guard members --- .../PlatformCompatibilityAnalyzer.cs | 5 ++ ...tibilityAnalyzerTests.GuardedCallsTests.cs | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs index 49484c882a..3a6ac9e8c6 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs @@ -1437,6 +1437,11 @@ static void MergePlatformAttributes(ImmutableArray immediateAttri SmallDictionary? childAttributes = null; foreach (AttributeData attribute in immediateAttributes) { + if (attribute.AttributeClass.Name is SupportedOSPlatformGuardAttribute or UnsupportedOSPlatformGuardAttribute) + { + parentAttributes = new PlatformAttributes(); + return; + } if (s_osPlatformAttributes.Contains(attribute.AttributeClass.Name)) { TryAddValidAttribute(ref childAttributes, attribute); diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs index 30f549c034..b5c11164a1 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs @@ -4315,6 +4315,58 @@ void SupportedOnWindows8() { } await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); } + [Fact] + public async Task GuardMemberWithinPlatformSpecificTypeShouldNowWarn() + { + var source = @" +using System; +using System.Runtime.Versioning; + +class Test +{ + void M1() + { + [|UnsupportedBrowserType.M1()|]; + [|UnsupportedBrowserType.M2()|]; + if (UnsupportedBrowserType.IsSupported) + { + UnsupportedBrowserType.M1(); + UnsupportedBrowserType.M2(); + } + var t = [|new UnsupportedBrowserType()|]; + + [|WindowsOnlyType.M1()|]; + [|WindowsOnlyType.M2()|]; + if (WindowsOnlyType.IsSupported) + { + WindowsOnlyType.M1(); + WindowsOnlyType.M2(); + } + var w = [|new WindowsOnlyType()|]; + } +} + +[UnsupportedOSPlatform(""browser"")] +class UnsupportedBrowserType +{ + public static void M1() { } + [UnsupportedOSPlatformGuard(""browser"")] + public static bool IsSupported { get; } + public static void M2() { } +} +[SupportedOSPlatform(""windows"")] +class WindowsOnlyType +{ + public static void M1() { } + [SupportedOSPlatformGuard(""windows"")] + public static bool IsSupported { get; } + public static void M2() { } +} +" + MockAttributesCsSource; + + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + private readonly string MockAttributesCsSource = @" namespace System.Runtime.Versioning { From 51c758672ecae1dcd8c191587846d288e02bb49f Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Fri, 7 May 2021 18:04:25 -0700 Subject: [PATCH 3/6] Apply feedback --- ...formCompatibilityAnalyzer.OperationVisitor.cs | 16 ++++++++-------- .../PlatformCompatibilityAnalyzer.Value.cs | 5 ----- .../PlatformCompatibilityAnalyzer.cs | 11 +++++------ ...mpatibilityAnalyzerTests.GuardedCallsTests.cs | 4 ++-- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs index 8c806bd5a8..81ca4d98d1 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs @@ -26,12 +26,12 @@ public OperationVisitor( _osPlatformType = osPlatformType; } - internal static bool TryParseGuardAttrbiutes(ISymbol symbol, ref GlobalFlowStateAnalysisValueSet value) + internal static bool TryParseGuardAttributes(ImmutableArray attributes, ref GlobalFlowStateAnalysisValueSet value) { - if (HasAnyGuardAttribute(symbol)) + if (HasAnyGuardAttribute(attributes)) { using var infosBuilder = ArrayBuilder.GetInstance(); - if (PlatformMethodValue.TryDecode(symbol.GetAttributes(), infosBuilder)) + if (PlatformMethodValue.TryDecode(attributes, infosBuilder)) { for (var i = 0; i < infosBuilder.Count; i++) { @@ -48,8 +48,8 @@ internal static bool TryParseGuardAttrbiutes(ISymbol symbol, ref GlobalFlowState return false; - static bool HasAnyGuardAttribute(ISymbol symbol) => - symbol.GetAttributes().Any(a => a.AttributeClass.Name is SupportedOSPlatformGuardAttribute or UnsupportedOSPlatformGuardAttribute); + static bool HasAnyGuardAttribute(ImmutableArray attributes) => + attributes.Any(a => a.AttributeClass.Name is SupportedOSPlatformGuardAttribute or UnsupportedOSPlatformGuardAttribute); } public override GlobalFlowStateAnalysisValueSet VisitInvocation_NonLambdaOrDelegateOrLocalFunction( @@ -80,7 +80,7 @@ public override GlobalFlowStateAnalysisValueSet VisitInvocation_NonLambdaOrDeleg return GlobalFlowStateAnalysisValueSet.Unknown; } - else if (TryParseGuardAttrbiutes(method, ref value)) + else if (TryParseGuardAttributes(method.GetAttributes(), ref value)) { return value; } @@ -94,7 +94,7 @@ public override GlobalFlowStateAnalysisValueSet VisitFieldReference(IFieldRefere var value = base.VisitFieldReference(operation, argument); if (operation.Type.SpecialType == SpecialType.System_Boolean && - TryParseGuardAttrbiutes(operation.Field, ref value)) + TryParseGuardAttributes(operation.Field.GetAttributes(), ref value)) { return value; } @@ -106,7 +106,7 @@ public override GlobalFlowStateAnalysisValueSet VisitPropertyReference(IProperty { var value = base.VisitPropertyReference(operation, argument); if (operation.Type.SpecialType == SpecialType.System_Boolean && - TryParseGuardAttrbiutes(operation.Property, ref value)) + TryParseGuardAttributes(operation.Property.GetAttributes(), ref value)) { return value; } diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs index f895d1c07b..506198623f 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs @@ -115,11 +115,6 @@ public static bool TryDecode(ImmutableArray attributes, ArrayBuil HasNonEmptyStringArgument(attribute, out var argument) && TryParsePlatformNameAndVersion(argument, out string platformName, out Version? version)) { - if (platformName.Equals(OSX, StringComparison.OrdinalIgnoreCase)) - { - platformName = macOS; - } - var info = new PlatformMethodValue(platformName, version, negated: attribute.AttributeClass.Name == UnsupportedOSPlatformGuardAttribute); infosBuilder.Add(info); } diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs index 3a6ac9e8c6..627b3a39fb 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs @@ -1627,10 +1627,6 @@ private static bool TryAddValidAttribute([NotNullWhen(true)] ref SmallDictionary TryParsePlatformNameAndVersion(argument, out var platformName, out var version)) { attributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); - if (platformName.Equals(OSX, StringComparison.OrdinalIgnoreCase)) - { - platformName = macOS; - } if (!attributes.TryGetValue(platformName, out var _)) { @@ -1670,7 +1666,7 @@ private static bool TryParsePlatformNameAndVersion(string osString, out string o { if (i > 0 && Version.TryParse(osString[i..], out Version? parsedVersion)) { - osPlatformName = osString.Substring(0, i); + osPlatformName = ConsolidatePlatformName(osString.Substring(0, i)); version = parsedVersion; return true; } @@ -1679,11 +1675,14 @@ private static bool TryParsePlatformNameAndVersion(string osString, out string o } } - osPlatformName = osString; + osPlatformName = ConsolidatePlatformName(osString); version = EmptyVersion; return true; } + private static string ConsolidatePlatformName(string platformName) => + platformName.Equals(OSX, StringComparison.OrdinalIgnoreCase) ? macOS : platformName; + private static void AddAttribute(string name, Version version, Versions attributes) { if (name == SupportedOSPlatformAttribute) diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs index b5c11164a1..9234da7cbc 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.GuardedCallsTests.cs @@ -3989,8 +3989,8 @@ void M1() if (_http3Enabled) { - //SupportedOnWindowsLinuxOsx(); - [|SupportedOnLinux()|]; // only supported on linux but call site is reachable on 3, linus, windows, macos + SupportedOnWindowsLinuxOsx(); + [|SupportedOnLinux()|]; // only supported on linux but call site is reachable on 3, linux, windows, macos } else { From cf63ef6f80699b6d9b6378f2c9cf4cab7a0b520f Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Mon, 10 May 2021 09:57:44 -0700 Subject: [PATCH 4/6] Apply more feedback --- .../PlatformCompatibilityAnalyzer.Value.cs | 3 +-- .../PlatformCompatibilityAnalyzer.cs | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs index 506198623f..313395f1e1 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Value.cs @@ -112,8 +112,7 @@ public static bool TryDecode(ImmutableArray attributes, ArrayBuil foreach (var attribute in attributes) { if (attribute.AttributeClass.Name is SupportedOSPlatformGuardAttribute or UnsupportedOSPlatformGuardAttribute && - HasNonEmptyStringArgument(attribute, out var argument) && - TryParsePlatformNameAndVersion(argument, out string platformName, out Version? version)) + TryParsePlatformNameAndVersion(attribute, out var platformName, out var version)) { var info = new PlatformMethodValue(platformName, version, negated: attribute.AttributeClass.Name == UnsupportedOSPlatformGuardAttribute); infosBuilder.Add(info); diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs index 627b3a39fb..5d2622ccd1 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs @@ -1623,8 +1623,7 @@ static Versions NormalizeAttribute(Versions attributes) private static bool TryAddValidAttribute([NotNullWhen(true)] ref SmallDictionary? attributes, AttributeData attribute) { - if (HasNonEmptyStringArgument(attribute, out var argument) && - TryParsePlatformNameAndVersion(argument, out var platformName, out var version)) + if (TryParsePlatformNameAndVersion(attribute, out var platformName, out var version)) { attributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); @@ -1640,6 +1639,18 @@ private static bool TryAddValidAttribute([NotNullWhen(true)] ref SmallDictionary return false; } + private static bool TryParsePlatformNameAndVersion(AttributeData attribute, out string platformName, [NotNullWhen(true)] out Version? version) + { + if (HasNonEmptyStringArgument(attribute, out var argument)) + { + return TryParsePlatformNameAndVersion(argument, out platformName, out version); + } + + version = null; + platformName = string.Empty; + return false; + } + private static bool HasNonEmptyStringArgument(AttributeData attribute, [NotNullWhen(true)] out string? stringArgument) { if (!attribute.ConstructorArguments.IsEmpty && From 0f38f386851033569fce5c00c657064a2ee48983 Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Tue, 11 May 2021 11:59:08 -0700 Subject: [PATCH 5/6] Apply more feedback --- ...mCompatibilityAnalyzer.OperationVisitor.cs | 67 ++++++++++--------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs index 81ca4d98d1..e0ec80ebdf 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.OperationVisitor.cs @@ -2,6 +2,7 @@ using System.Collections.Immutable; using System.Linq; +using Analyzer.Utilities.Extensions; using Analyzer.Utilities.PooledObjects; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.FlowAnalysis.DataFlow.GlobalFlowStateAnalysis; @@ -26,26 +27,32 @@ public OperationVisitor( _osPlatformType = osPlatformType; } - internal static bool TryParseGuardAttributes(ImmutableArray attributes, ref GlobalFlowStateAnalysisValueSet value) + internal static bool TryParseGuardAttributes(ISymbol symbol, ref GlobalFlowStateAnalysisValueSet value) { - if (HasAnyGuardAttribute(attributes)) + var attributes = symbol.GetAttributes(); + + if (symbol.GetMemberType()!.SpecialType != SpecialType.System_Boolean || + !HasAnyGuardAttribute(attributes)) { - using var infosBuilder = ArrayBuilder.GetInstance(); - if (PlatformMethodValue.TryDecode(attributes, infosBuilder)) - { - for (var i = 0; i < infosBuilder.Count; i++) - { - var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); - value = i == 0 ? newValue : infosBuilder[i].Negated ? value.WithAdditionalAnalysisValues(newValue, false) : - GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); - } + return false; + } - return true; + using var infosBuilder = ArrayBuilder.GetInstance(); + if (PlatformMethodValue.TryDecode(attributes, infosBuilder)) + { + for (var i = 0; i < infosBuilder.Count; i++) + { + var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); + // if the incoming value is negated it should be merged with AND logic, else with OR. + value = i == 0 ? newValue : infosBuilder[i].Negated ? value.WithAdditionalAnalysisValues(newValue, false) : + GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); } - value = GlobalFlowStateAnalysisValueSet.Unknown; + return true; } + value = GlobalFlowStateAnalysisValueSet.Unknown; + return false; static bool HasAnyGuardAttribute(ImmutableArray attributes) => @@ -62,28 +69,25 @@ public override GlobalFlowStateAnalysisValueSet VisitInvocation_NonLambdaOrDeleg { var value = base.VisitInvocation_NonLambdaOrDelegateOrLocalFunction(method, visitedInstance, visitedArguments, invokedAsDelegate, originalOperation, defaultValue); - if (method.ReturnType.SpecialType == SpecialType.System_Boolean) + if (_platformCheckMethods.Contains(method.OriginalDefinition)) { - if (_platformCheckMethods.Contains(method.OriginalDefinition)) + using var infosBuilder = ArrayBuilder.GetInstance(); + if (PlatformMethodValue.TryDecode(method, visitedArguments, DataFlowAnalysisContext.ValueContentAnalysisResult, _osPlatformType, infosBuilder)) { - using var infosBuilder = ArrayBuilder.GetInstance(); - if (PlatformMethodValue.TryDecode(method, visitedArguments, DataFlowAnalysisContext.ValueContentAnalysisResult, _osPlatformType, infosBuilder)) + for (var i = 0; i < infosBuilder.Count; i++) { - for (var i = 0; i < infosBuilder.Count; i++) - { - var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); - value = i == 0 ? newValue : GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); - } - - return value; + var newValue = GlobalFlowStateAnalysisValueSet.Create(infosBuilder[i]); + value = i == 0 ? newValue : GlobalFlowStateAnalysis.GlobalFlowStateAnalysisValueSetDomain.Instance.Merge(value, newValue); } - return GlobalFlowStateAnalysisValueSet.Unknown; - } - else if (TryParseGuardAttributes(method.GetAttributes(), ref value)) - { return value; } + + return GlobalFlowStateAnalysisValueSet.Unknown; + } + else if (TryParseGuardAttributes(method, ref value)) + { + return value; } return value; @@ -93,8 +97,7 @@ public override GlobalFlowStateAnalysisValueSet VisitFieldReference(IFieldRefere { var value = base.VisitFieldReference(operation, argument); - if (operation.Type.SpecialType == SpecialType.System_Boolean && - TryParseGuardAttributes(operation.Field.GetAttributes(), ref value)) + if (TryParseGuardAttributes(operation.Field, ref value)) { return value; } @@ -105,8 +108,8 @@ public override GlobalFlowStateAnalysisValueSet VisitFieldReference(IFieldRefere public override GlobalFlowStateAnalysisValueSet VisitPropertyReference(IPropertyReferenceOperation operation, object? argument) { var value = base.VisitPropertyReference(operation, argument); - if (operation.Type.SpecialType == SpecialType.System_Boolean && - TryParseGuardAttributes(operation.Property.GetAttributes(), ref value)) + + if (TryParseGuardAttributes(operation.Property, ref value)) { return value; } From 9c0a6289ceca6ecd9d1d939d683d27d9cb387c6d Mon Sep 17 00:00:00 2001 From: Buyaa Namnan Date: Wed, 12 May 2021 12:24:37 -0700 Subject: [PATCH 6/6] Update helper method name --- .../InteropServices/PlatformCompatibilityAnalyzer.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs index 5d2622ccd1..adb21c77e6 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs @@ -1677,7 +1677,7 @@ private static bool TryParsePlatformNameAndVersion(string osString, out string o { if (i > 0 && Version.TryParse(osString[i..], out Version? parsedVersion)) { - osPlatformName = ConsolidatePlatformName(osString.Substring(0, i)); + osPlatformName = GetNameAsMacOsWhenOSX(osString.Substring(0, i)); version = parsedVersion; return true; } @@ -1686,12 +1686,12 @@ private static bool TryParsePlatformNameAndVersion(string osString, out string o } } - osPlatformName = ConsolidatePlatformName(osString); + osPlatformName = GetNameAsMacOsWhenOSX(osString); version = EmptyVersion; return true; } - private static string ConsolidatePlatformName(string platformName) => + private static string GetNameAsMacOsWhenOSX(string platformName) => platformName.Equals(OSX, StringComparison.OrdinalIgnoreCase) ? macOS : platformName; private static void AddAttribute(string name, Version version, Versions attributes)