From e12c29ce0485f1cdedc9fd8032b1ddcec35b8346 Mon Sep 17 00:00:00 2001 From: Buyaa Date: Mon, 1 Feb 2021 12:10:25 -0800 Subject: [PATCH] CA1416 handle call-site empty sets causing warnings when TFM attributes applied (#4740) * Handle call site empty sets causing warnings when TFM attributes applied --- .../PlatformCompatibilityAnalyzer.Data.cs | 19 +- .../PlatformCompatibilityAnalyzer.cs | 395 +++++++++--------- .../PlatformCompatibilityAnalyzerTests.cs | 242 ++++++++++- src/Utilities/Compiler/SmallDictionary.cs | 112 ++++- 4 files changed, 562 insertions(+), 206 deletions(-) diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Data.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Data.cs index 582c27b551..8c1674230d 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Data.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.Data.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; +using Analyzer.Utilities; namespace Microsoft.NetCore.Analyzers.InteropServices { @@ -22,14 +23,28 @@ public sealed partial class PlatformCompatibilityAnalyzer /// - UnsupportedFirst - keeps the lowest version of [UnsupportedOSPlatform] attribute found /// - UnsupportedSecond - keeps the second lowest version of [UnsupportedOSPlatform] attribute found /// - private class PlatformAttributes + private class Versions { public Version? SupportedFirst { get; set; } public Version? SupportedSecond { get; set; } public Version? UnsupportedFirst { get; set; } public Version? UnsupportedSecond { get; set; } - public bool HasAttribute() => SupportedFirst != null || UnsupportedFirst != null || + public bool IsSet() => SupportedFirst != null || UnsupportedFirst != null || SupportedSecond != null || UnsupportedSecond != null; } + + private sealed class PlatformAttributes + { + public PlatformAttributes() { } + + public PlatformAttributes(Callsite callsite, SmallDictionary platforms) + { + Callsite = callsite; + Platforms = platforms; + } + + public SmallDictionary? Platforms { get; set; } + public Callsite Callsite { get; set; } + } } } diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs index bfe558fda3..2b8522793b 100644 --- a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs +++ b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzer.cs @@ -159,7 +159,7 @@ public override void Initialize(AnalysisContext context) m.Parameters[0].Type.Equals(osPlatformType)); var guardMethods = GetOperatingSystemGuardMethods(runtimeIsOSPlatformMethod, operatingSystemType); - var platformSpecificMembers = new ConcurrentDictionary?>(); + var platformSpecificMembers = new ConcurrentDictionary(); var osPlatformCreateMethod = osPlatformType?.GetMembers("Create").OfType().FirstOrDefault(m => m.IsStatic && m.ReturnType.Equals(osPlatformType) && @@ -223,7 +223,7 @@ private void AnalyzeOperationBlock( IMethodSymbol? osPlatformCreateMethod, INamedTypeSymbol? osPlatformType, INamedTypeSymbol stringType, - ConcurrentDictionary?> platformSpecificMembers, + ConcurrentDictionary platformSpecificMembers, ImmutableArray msBuildPlatforms, ITypeSymbol? notSupportedExceptionType) { @@ -233,8 +233,8 @@ private void AnalyzeOperationBlock( } var osPlatformTypeArray = osPlatformType != null ? ImmutableArray.Create(osPlatformType) : ImmutableArray.Empty; - var platformSpecificOperations = PooledConcurrentDictionary, (SmallDictionary attributes, - SmallDictionary? csAttributes)>.GetInstance(); + var platformSpecificOperations = PooledConcurrentDictionary, + (SmallDictionary attributes, SmallDictionary? csAttributes)>.GetInstance(); context.RegisterOperationAction(context => { @@ -251,12 +251,9 @@ private void AnalyzeOperationBlock( { try { - if (platformSpecificOperations.IsEmpty) - { - return; - } - - if (guardMethods.IsEmpty || !(context.OperationBlocks.GetControlFlowGraph() is { } cfg)) + if (platformSpecificOperations.IsEmpty || + guardMethods.IsEmpty || + !(context.OperationBlocks.GetControlFlowGraph() is { } cfg)) { return; } @@ -293,12 +290,7 @@ private void AnalyzeOperationBlock( } finally { - // Workaround for https://github.com/dotnet/roslyn/issues/46859 - // Do not free in presence of cancellation. - if (!context.CancellationToken.IsCancellationRequested) - { - platformSpecificOperations.Free(context.CancellationToken); - } + platformSpecificOperations.Free(context.CancellationToken); } return; @@ -324,8 +316,8 @@ argument.ConstantValue.Value is string platformName && }); } - private static bool HasGuardedLambdaOrLocalFunctionResult(IOperation platformSpecificOperation, SmallDictionary attributes, - ref SmallDictionary? csAttributes, DataFlowAnalysisResult analysisResult) + private static bool HasGuardedLambdaOrLocalFunctionResult(IOperation platformSpecificOperation, SmallDictionary attributes, + ref SmallDictionary? csAttributes, DataFlowAnalysisResult analysisResult) { if (!platformSpecificOperation.IsWithinLambdaOrLocalFunction(out var containingLambdaOrLocalFunctionOperation)) { @@ -392,15 +384,15 @@ invocation.Arguments[0].Value is IPropertyReferenceOperation propertyReference & return false; } - private static bool IsKnownValueGuarded(SmallDictionary attributes, - ref SmallDictionary? csAttributes, GlobalFlowStateAnalysisValueSet value) + private static bool IsKnownValueGuarded(SmallDictionary attributes, + ref SmallDictionary? csAttributes, GlobalFlowStateAnalysisValueSet value) { using var capturedVersions = PooledDictionary.GetInstance(StringComparer.OrdinalIgnoreCase); return IsKnownValueGuarded(attributes, ref csAttributes, value, capturedVersions); static bool IsKnownValueGuarded( - SmallDictionary attributes, - ref SmallDictionary? csAttributes, + SmallDictionary attributes, + ref SmallDictionary? csAttributes, GlobalFlowStateAnalysisValueSet value, PooledDictionary capturedVersions) { @@ -413,26 +405,22 @@ static bool IsKnownValueGuarded( { if (info.Negated) { - if (attribute.UnsupportedFirst != null) + if (attribute.UnsupportedFirst != null && + attribute.UnsupportedFirst >= info.Version) { - if (attribute.UnsupportedFirst >= info.Version) + if (DenyList(attribute)) { - if (DenyList(attribute)) - { - attribute.SupportedFirst = null; - attribute.SupportedSecond = null; - attribute.UnsupportedSecond = null; - } - attribute.UnsupportedFirst = null; + attribute.SupportedFirst = null; + attribute.SupportedSecond = null; + attribute.UnsupportedSecond = null; } + attribute.UnsupportedFirst = null; } - if (attribute.UnsupportedSecond != null) + if (attribute.UnsupportedSecond != null && + attribute.UnsupportedSecond <= info.Version) { - if (attribute.UnsupportedSecond <= info.Version) - { - attribute.UnsupportedSecond = null; - } + attribute.UnsupportedSecond = null; } if (!IsEmptyVersion(info.Version)) @@ -479,14 +467,11 @@ static bool IsKnownValueGuarded( RemoveUnsupportsOnDifferentPlatforms(attributes, info.PlatformName); } } - else + else if (!info.Negated) { - if (!info.Negated) - { - // it is checking one exact platform, other unsupported should be suppressed - RemoveUnsupportsOnDifferentPlatforms(attributes, info.PlatformName); - csAttributes = SetAsCallSiteSupportedAttribute(csAttributes, info); - } + // it is checking one exact platform, other unsupported should be suppressed + RemoveUnsupportsOnDifferentPlatforms(attributes, info.PlatformName); + csAttributes = SetAsCallSiteSupportedAttribute(csAttributes, info); } } } @@ -496,7 +481,7 @@ static bool IsKnownValueGuarded( foreach (var attribute in attributes) { // if any of the attributes is not suppressed - if (attribute.Value.HasAttribute()) + if (attribute.Value.IsSet()) { return false; } @@ -523,11 +508,11 @@ static bool IsKnownValueGuarded( return true; } - static SmallDictionary SetAsCallSiteSupportedAttribute(SmallDictionary? csAttributes, PlatformMethodValue info) + static SmallDictionary SetAsCallSiteSupportedAttribute(SmallDictionary? csAttributes, PlatformMethodValue info) { if (csAttributes == null) { - csAttributes = new SmallDictionary(); + csAttributes = new SmallDictionary(StringComparer.OrdinalIgnoreCase); } if (csAttributes.TryGetValue(info.PlatformName, out var attributes)) { @@ -542,12 +527,12 @@ static SmallDictionary SetAsCallSiteSupportedAttribu } else { - csAttributes.Add(info.PlatformName, new PlatformAttributes() { SupportedFirst = info.Version }); + csAttributes.Add(info.PlatformName, new Versions() { SupportedFirst = info.Version }); } return csAttributes; } - static void RemoveUnsupportsOnDifferentPlatforms(SmallDictionary attributes, string platformName) + static void RemoveUnsupportsOnDifferentPlatforms(SmallDictionary attributes, string platformName) { foreach (var (name, attribute) in attributes) { @@ -562,7 +547,7 @@ static void RemoveUnsupportsOnDifferentPlatforms(SmallDictionary attributes, string platformName) + static void RemoveOtherSupportsOnDifferentPlatforms(SmallDictionary attributes, string platformName) { foreach (var (name, attribute) in attributes) { @@ -586,14 +571,14 @@ static void RemoveOtherSupportsOnDifferentPlatforms(SmallDictionary version.Major == 0 && version.Minor == 0; - private static void ReportDiagnostics(KeyValuePair operationToSymbol, SmallDictionary attributes, - SmallDictionary? csAttributes, OperationBlockAnalysisContext context, - ConcurrentDictionary?> platformSpecificMembers) + private static void ReportDiagnostics(KeyValuePair operationToSymbol, SmallDictionary attributes, + SmallDictionary? csAttributes, OperationBlockAnalysisContext context, + ConcurrentDictionary platformSpecificMembers) { var symbol = operationToSymbol.Value is IMethodSymbol method && method.IsConstructor() ? operationToSymbol.Value.ContainingType : operationToSymbol.Value; var operationName = symbol.ToDisplayString(GetLanguageSpecificFormat(operationToSymbol.Key)); - var originalAttributes = platformSpecificMembers[symbol] ?? attributes; + var originalAttributes = platformSpecificMembers[symbol].Platforms ?? attributes; foreach (var attribute in originalAttributes.Values) { @@ -609,7 +594,7 @@ private static void ReportDiagnostics(KeyValuePair operatio } static void ReportSupportedDiagnostic(IOperation operation, OperationBlockAnalysisContext context, string operationName, - SmallDictionary attributes, SmallDictionary? callsiteAttributes) + SmallDictionary attributes, SmallDictionary? callsiteAttributes) { var supportedRule = GetSupportedPlatforms(attributes, callsiteAttributes, out var platformNames); var callSitePlatforms = GetCallsitePlatforms(attributes, callsiteAttributes, out var callsite, supported: supportedRule); @@ -631,10 +616,10 @@ static DiagnosticDescriptor SwitchSupportedRule(Callsite callsite) _ => throw new NotImplementedException() }; - static bool IsDenyList(SmallDictionary? callsiteAttributes) => + static bool IsDenyList(SmallDictionary? callsiteAttributes) => callsiteAttributes != null && callsiteAttributes.Any(csa => DenyList(csa.Value)); - static bool GetSupportedPlatforms(SmallDictionary attributes, SmallDictionary? csAttributes, out List platformNames) + static bool GetSupportedPlatforms(SmallDictionary attributes, SmallDictionary? csAttributes, out List platformNames) { var supportedRule = true; platformNames = new List(); @@ -716,13 +701,13 @@ static DiagnosticDescriptor SwitchRule(Callsite callsite, bool unsupported) } static void ReportUnsupportedDiagnostic(IOperation operation, OperationBlockAnalysisContext context, string operationName, - SmallDictionary attributes, SmallDictionary? callsiteAttributes) + SmallDictionary attributes, SmallDictionary? callsiteAttributes) { var unsupportedRule = GetPlatformNames(attributes, callsiteAttributes, out var platformNames); var csPlatformNames = JoinNames(GetCallsitePlatforms(attributes, callsiteAttributes, out var callsite, supported: !unsupportedRule)); context.ReportDiagnostic(operation.CreateDiagnostic(SwitchRule(callsite, unsupportedRule), operationName, JoinNames(platformNames), csPlatformNames)); - static bool GetPlatformNames(SmallDictionary attributes, SmallDictionary? csAttributes, out List platformNames) + static bool GetPlatformNames(SmallDictionary attributes, SmallDictionary? csAttributes, out List platformNames) { platformNames = new List(); var unsupportedRule = true; @@ -795,8 +780,8 @@ static bool GetPlatformNames(SmallDictionary attribu } } - static List GetCallsitePlatforms(SmallDictionary attributes, - SmallDictionary? callsiteAttributes, out Callsite callsite, bool supported) + static List GetCallsitePlatforms(SmallDictionary attributes, + SmallDictionary? callsiteAttributes, out Callsite callsite, bool supported) { callsite = Callsite.AllPlatforms; var platformNames = new List(); @@ -892,7 +877,7 @@ static List GetCallsitePlatforms(SmallDictionary operation.Language == LanguageNames.CSharp ? SymbolDisplayFormat.CSharpShortErrorMessageFormat : SymbolDisplayFormat.VisualBasicShortErrorMessageFormat; - static bool HasSameVersionedPlatformSupport(SmallDictionary attributes, string pName, bool checkSupport) + static bool HasSameVersionedPlatformSupport(SmallDictionary attributes, string pName, bool checkSupport) { if (attributes.TryGetValue(pName, out var attribute)) { @@ -930,7 +915,8 @@ private enum Callsite { AllPlatforms, Reachable, - Unreachable + Unreachable, + Empty } private static ISymbol? GetOperationSymbol(IOperation operation) @@ -981,8 +967,8 @@ private static ISymbol GetEventAccessor(IEventSymbol iEvent, IOperation operatio } private static void AnalyzeOperation(IOperation operation, OperationAnalysisContext context, PooledConcurrentDictionary, - (SmallDictionary attributes, SmallDictionary? csAttributes)> platformSpecificOperations, - ConcurrentDictionary?> platformSpecificMembers, ImmutableArray msBuildPlatforms, + (SmallDictionary attributes, SmallDictionary? csAttributes)> platformSpecificOperations, + ConcurrentDictionary platformSpecificMembers, ImmutableArray msBuildPlatforms, ITypeSymbol? notSupportedExceptionType) { if (operation.Parent is IArgumentOperation argumentOperation && UsedInCreatingNotSupportedException(argumentOperation, notSupportedExceptionType)) @@ -1029,18 +1015,18 @@ private static void AnalyzeOperation(IOperation operation, OperationAnalysisCont } static void CheckTypeArguments(ImmutableArray typeArguments, IOperation operation, OperationAnalysisContext context, - PooledConcurrentDictionary, (SmallDictionary attributes, - SmallDictionary? csAttributes)> platformSpecificOperations, - ConcurrentDictionary?> platformSpecificMembers, ImmutableArray msBuildPlatforms) + PooledConcurrentDictionary, (SmallDictionary attributes, + SmallDictionary? csAttributes)> platformSpecificOperations, + ConcurrentDictionary platformSpecificMembers, ImmutableArray msBuildPlatforms) { using var workingSet = PooledHashSet.GetInstance(); CheckTypeArgumentsCore(typeArguments, operation, context, platformSpecificOperations, platformSpecificMembers, msBuildPlatforms, workingSet); } static void CheckTypeArgumentsCore(ImmutableArray typeArguments, IOperation operation, OperationAnalysisContext context, - PooledConcurrentDictionary, (SmallDictionary attributes, - SmallDictionary? csAttributes)> platformSpecificOperations, ConcurrentDictionary?> platformSpecificMembers, ImmutableArray msBuildPlatforms, PooledHashSet workingSet) + PooledConcurrentDictionary, (SmallDictionary attributes, + SmallDictionary? csAttributes)> platformSpecificOperations, ConcurrentDictionary platformSpecificMembers, ImmutableArray msBuildPlatforms, PooledHashSet workingSet) { foreach (var typeArgument in typeArguments) { @@ -1061,8 +1047,8 @@ static void CheckTypeArgumentsCore(ImmutableArray typeArguments, IO } static void CheckOperationAttributes(IOperation operation, OperationAnalysisContext context, PooledConcurrentDictionary, - (SmallDictionary attributes, SmallDictionary? csAttributes)> platformSpecificOperations, - ConcurrentDictionary?> platformSpecificMembers, ImmutableArray msBuildPlatforms, ISymbol symbol, bool checkParents) + (SmallDictionary attributes, SmallDictionary? csAttributes)> platformSpecificOperations, + ConcurrentDictionary platformSpecificMembers, ImmutableArray msBuildPlatforms, ISymbol symbol, bool checkParents) { if (TryGetOrCreatePlatformAttributes(symbol, checkParents, platformSpecificMembers, out var operationAttributes)) { @@ -1074,17 +1060,15 @@ static void CheckOperationAttributes(IOperation operation, OperationAnalysisCont if (TryGetOrCreatePlatformAttributes(containingSymbol, true, platformSpecificMembers, out var callSiteAttributes)) { - if (IsNotSuppressedByCallSite(operationAttributes, callSiteAttributes, msBuildPlatforms, out var notSuppressedAttributes)) + if (callSiteAttributes.Callsite != Callsite.Empty && + IsNotSuppressedByCallSite(operationAttributes.Platforms!, callSiteAttributes.Platforms!, msBuildPlatforms, out var notSuppressedAttributes)) { - platformSpecificOperations.TryAdd(new KeyValuePair(operation, symbol), (notSuppressedAttributes, callSiteAttributes)); + platformSpecificOperations.TryAdd(new KeyValuePair(operation, symbol), (notSuppressedAttributes, callSiteAttributes.Platforms)); } } - else + else if (TryCopyAttributesNotSuppressedByMsBuild(operationAttributes.Platforms!, msBuildPlatforms, out var copiedAttributes)) { - if (TryCopyAttributesNotSuppressedByMsBuild(operationAttributes, msBuildPlatforms, out var copiedAttributes)) - { - platformSpecificOperations.TryAdd(new KeyValuePair(operation, symbol), (copiedAttributes, null)); - } + platformSpecificOperations.TryAdd(new KeyValuePair(operation, symbol), (copiedAttributes, null)); } } } @@ -1102,27 +1086,40 @@ private static bool UsedInCreatingNotSupportedException(IArgumentOperation opera return false; } - private static bool TryCopyAttributesNotSuppressedByMsBuild(SmallDictionary operationAttributes, - ImmutableArray msBuildPlatforms, out SmallDictionary copiedAttributes) + private static bool TryCopyAttributesNotSuppressedByMsBuild(SmallDictionary operationAttributes, + ImmutableArray msBuildPlatforms, out SmallDictionary copiedAttributes) { - copiedAttributes = new SmallDictionary(StringComparer.OrdinalIgnoreCase); + copiedAttributes = new SmallDictionary(StringComparer.OrdinalIgnoreCase); foreach (var (platformName, attributes) in operationAttributes) { if (AllowList(attributes) || msBuildPlatforms.Contains(platformName, StringComparer.OrdinalIgnoreCase)) { - copiedAttributes.Add(platformName, CopyAllAttributes(new PlatformAttributes(), attributes)); + copiedAttributes.Add(platformName, CopyAllAttributes(new Versions(), attributes)); } } - return copiedAttributes.Any(); + return !copiedAttributes.IsEmpty; + } + + private static PlatformAttributes CopyAttributes(PlatformAttributes copyAttributes) + { + var copy = new PlatformAttributes(copyAttributes.Callsite, new SmallDictionary(StringComparer.OrdinalIgnoreCase)); + + foreach (var (platformName, attributes) in copyAttributes.Platforms!) + { + copy.Platforms!.Add(platformName, CopyAllAttributes(new Versions(), attributes)); + } + + return copy; } - private static SmallDictionary CopyAttributes(SmallDictionary copyAttributes) + private static SmallDictionary CopyAttributes(SmallDictionary copyAttributes) { - var copy = new SmallDictionary(StringComparer.OrdinalIgnoreCase); - foreach (var (platformName, attributes) in copyAttributes) + var copy = new SmallDictionary(StringComparer.OrdinalIgnoreCase); + + foreach (var (platformName, attributes) in copyAttributes!) { - copy.Add(platformName, CopyAllAttributes(new PlatformAttributes(), attributes)); + copy.Add(platformName, CopyAllAttributes(new Versions(), attributes)); } return copy; @@ -1147,16 +1144,16 @@ private static SmallDictionary CopyAttributes(SmallD /// Out parameter will include all attributes not suppressed by call site /// true if all attributes applied to the operation is suppressed, false otherwise - private static bool IsNotSuppressedByCallSite(SmallDictionary operationAttributes, - SmallDictionary callSiteAttributes, ImmutableArray msBuildPlatforms, - out SmallDictionary notSuppressedAttributes) + private static bool IsNotSuppressedByCallSite(SmallDictionary operationAttributes, + SmallDictionary callSiteAttributes, ImmutableArray msBuildPlatforms, + out SmallDictionary notSuppressedAttributes) { - notSuppressedAttributes = new SmallDictionary(StringComparer.OrdinalIgnoreCase); + notSuppressedAttributes = new SmallDictionary(StringComparer.OrdinalIgnoreCase); bool? mandatorySupportFound = null; using var supportedOnlyPlatforms = PooledHashSet.GetInstance(StringComparer.OrdinalIgnoreCase); foreach (var (platformName, attribute) in operationAttributes) { - var diagnosticAttribute = new PlatformAttributes(); + var diagnosticAttribute = new Versions(); if (attribute.SupportedFirst != null) { @@ -1175,12 +1172,13 @@ private static bool IsNotSuppressedByCallSite(SmallDictionary DenyList(ca.Value))) - { - diagnosticAttribute.SupportedFirst = (Version)attribute.SupportedFirst.Clone(); - } + { + diagnosticAttribute.SupportedFirst = (Version)attribute.SupportedFirst.Clone(); } } } - else + else if (attribute.UnsupportedFirst != null) // Unsupported for this but supported all other { - if (attribute.UnsupportedFirst != null) // Unsupported for this but supported all other + if (callSiteAttributes.TryGetValue(platformName, out var callSiteAttribute)) { - if (callSiteAttributes.TryGetValue(platformName, out var callSiteAttribute)) + if (callSiteAttribute.SupportedFirst != null) { - if (callSiteAttribute.SupportedFirst != null) + if (callSiteAttribute.UnsupportedFirst != null) { - if (callSiteAttribute.UnsupportedFirst != null) + if (!SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst)) { - if (!SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst)) - { - diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); - } - else if (DenyList(callSiteAttribute)) - { - diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); - } + diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); } - else + else if (DenyList(callSiteAttribute)) { diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); } } else { - if (msBuildPlatforms.Contains(platformName, StringComparer.OrdinalIgnoreCase)) - { - if (!SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst)) - { - diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); - } - } + diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); } } else if (msBuildPlatforms.Contains(platformName, StringComparer.OrdinalIgnoreCase) && - !callSiteAttributes.Values.Any(v => v.SupportedFirst != null)) + !SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst)) { - // if MsBuild list contain the platform and call site has no any other supported attribute it means global, so need to warn diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); } } + else if (msBuildPlatforms.Contains(platformName, StringComparer.OrdinalIgnoreCase) && + !callSiteAttributes.Values.Any(v => v.SupportedFirst != null)) + { + // if MsBuild list contain the platform and call site has no any other supported attribute it means global, so need to warn + diagnosticAttribute.UnsupportedFirst = (Version)attribute.UnsupportedFirst.Clone(); + } } - if (diagnosticAttribute.HasAttribute()) + if (diagnosticAttribute.IsSet()) { notSuppressedAttributes[platformName] = diagnosticAttribute; } @@ -1294,12 +1278,10 @@ private static bool IsNotSuppressedByCallSite(SmallDictionary notSuppressedAttributes, string name) + static void AddOrUpdatedDiagnostic(Versions operationAttributes, + SmallDictionary notSuppressedAttributes, string name) { if (operationAttributes.SupportedFirst != null) { if (!notSuppressedAttributes.TryGetValue(name, out var diagnosticAttribute)) { - diagnosticAttribute = new PlatformAttributes(); + diagnosticAttribute = new Versions(); } diagnosticAttribute.SupportedFirst = (Version)operationAttributes.SupportedFirst.Clone(); notSuppressedAttributes[name] = diagnosticAttribute; } } - static bool UnsupportedSecondSuppressed(PlatformAttributes attribute, PlatformAttributes callSiteAttribute) => + static bool UnsupportedSecondSuppressed(Versions attribute, Versions callSiteAttribute) => SuppressedByCallSiteSupported(attribute, callSiteAttribute.SupportedFirst) || SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedSecond!); - static bool SuppressedByCallSiteUnsupported(PlatformAttributes callSiteAttribute, Version unsupporteAttribute) => + static bool SuppressedByCallSiteUnsupported(Versions callSiteAttribute, Version unsupporteAttribute) => DenyList(callSiteAttribute) && callSiteAttribute.SupportedFirst != null ? callSiteAttribute.UnsupportedSecond != null && unsupporteAttribute >= callSiteAttribute.UnsupportedSecond : callSiteAttribute.UnsupportedFirst != null && unsupporteAttribute >= callSiteAttribute.UnsupportedFirst; - static bool SuppressedByCallSiteSupported(PlatformAttributes attribute, Version? callSiteSupportedFirst) => + static bool SuppressedByCallSiteSupported(Versions attribute, Version? callSiteSupportedFirst) => callSiteSupportedFirst != null && callSiteSupportedFirst >= attribute.SupportedFirst! && attribute.SupportedSecond != null && callSiteSupportedFirst >= attribute.SupportedSecond; - static bool UnsupportedFirstSuppressed(PlatformAttributes attribute, PlatformAttributes callSiteAttribute) => + static bool UnsupportedFirstSuppressed(Versions attribute, Versions callSiteAttribute) => callSiteAttribute.SupportedFirst != null && callSiteAttribute.SupportedFirst >= attribute.SupportedFirst || SuppressedByCallSiteUnsupported(callSiteAttribute, attribute.UnsupportedFirst!); // As optional if call site supports that platform, their versions should match - static bool OptionalOsSupportSuppressed(PlatformAttributes callSiteAttribute, PlatformAttributes attribute) => + static bool OptionalOsSupportSuppressed(Versions callSiteAttribute, Versions attribute) => (callSiteAttribute.SupportedFirst == null || attribute.SupportedFirst <= callSiteAttribute.SupportedFirst) && (callSiteAttribute.SupportedSecond == null || attribute.SupportedFirst <= callSiteAttribute.SupportedSecond); - static bool MandatoryOsVersionsSuppressed(PlatformAttributes callSitePlatforms, Version checkingVersion) => + static bool MandatoryOsVersionsSuppressed(Versions callSitePlatforms, Version checkingVersion) => callSitePlatforms.SupportedFirst != null && checkingVersion <= callSitePlatforms.SupportedFirst || callSitePlatforms.SupportedSecond != null && checkingVersion <= callSitePlatforms.SupportedSecond; } - private static PlatformAttributes CopyAllAttributes(PlatformAttributes copyTo, PlatformAttributes copyFrom) + private static Versions CopyAllAttributes(Versions copyTo, Versions copyFrom) { copyTo.SupportedFirst = (Version?)copyFrom.SupportedFirst?.Clone(); copyTo.SupportedSecond = (Version?)copyFrom.SupportedSecond?.Clone(); @@ -1384,8 +1367,8 @@ pOperation.Parent is IBinaryOperation bo && private static bool TryGetOrCreatePlatformAttributes( ISymbol symbol, bool checkParents, - ConcurrentDictionary?> platformSpecificMembers, - [NotNullWhen(true)] out SmallDictionary? attributes) + ConcurrentDictionary platformSpecificMembers, + out PlatformAttributes attributes) { if (!platformSpecificMembers.TryGetValue(symbol, out attributes)) { @@ -1406,15 +1389,17 @@ private static bool TryGetOrCreatePlatformAttributes( } } + attributes ??= new PlatformAttributes(); MergePlatformAttributes(symbol.GetAttributes(), ref attributes); attributes = platformSpecificMembers.GetOrAdd(symbol, attributes); } - return attributes != null; + return attributes.Platforms != null; - static void MergePlatformAttributes(ImmutableArray immediateAttributes, ref SmallDictionary? parentAttributes) + static void MergePlatformAttributes(ImmutableArray immediateAttributes, + ref PlatformAttributes parentAttributes) { - SmallDictionary? childAttributes = null; + SmallDictionary? childAttributes = null; foreach (AttributeData attribute in immediateAttributes) { if (s_osPlatformAttributes.Contains(attribute.AttributeClass.Name)) @@ -1428,18 +1413,21 @@ static void MergePlatformAttributes(ImmutableArray immediateAttri return; } - if (parentAttributes != null && parentAttributes.Any()) + var pAttributes = parentAttributes.Platforms; + if (pAttributes != null && !pAttributes.IsEmpty) { - foreach (var (platform, attributes) in parentAttributes) + var notFoundPlatforms = PooledHashSet.GetInstance(); + bool supportFound = false; + foreach (var (platform, attributes) in pAttributes) { if (DenyList(attributes) && - !parentAttributes.Any(ca => AllowList(ca.Value))) + !pAttributes.Any(ca => AllowList(ca.Value))) { // if all are deny list then we can add the child attributes foreach (var (name, childAttribute) in childAttributes) { NormalizeAttribute(childAttribute); - if (parentAttributes.TryGetValue(name, out var existing)) + if (pAttributes.TryGetValue(name, out var existing)) { if (childAttribute.UnsupportedFirst != null) { @@ -1468,7 +1456,7 @@ static void MergePlatformAttributes(ImmutableArray immediateAttri } else { - parentAttributes[name] = childAttribute; + pAttributes[name] = childAttribute; } } // merged all attributes, no need to continue looping @@ -1481,16 +1469,18 @@ static void MergePlatformAttributes(ImmutableArray immediateAttri { childAttribute = NormalizeAttribute(childAttribute); // only later versions could narrow, other versions ignored - if (childAttribute.SupportedFirst > attributes.SupportedFirst && + if (childAttribute.SupportedFirst >= attributes.SupportedFirst && (attributes.SupportedSecond == null || attributes.SupportedSecond < childAttribute.SupportedFirst)) { attributes.SupportedSecond = childAttribute.SupportedFirst; + supportFound = true; } if (childAttribute.UnsupportedFirst != null) { if (childAttribute.UnsupportedFirst <= attributes.SupportedFirst) { + parentAttributes.Callsite = Callsite.Empty; attributes.SupportedFirst = childAttribute.SupportedFirst > attributes.SupportedFirst ? childAttribute.SupportedFirst : null; attributes.UnsupportedFirst = childAttribute.UnsupportedFirst; } @@ -1509,43 +1499,63 @@ static void MergePlatformAttributes(ImmutableArray immediateAttri } } } - // other platform attributes are ignored as the list couldn't be extended + else + { + // not existing parent platforms might need to be removed + notFoundPlatforms.Add(platform); + } + } + } + + if (notFoundPlatforms.Count > 0) + { + // For allow list if child narrowing supported platforms by having less platforms support than parent, + // not existing parent platforms should be removed + if (supportFound) + { + foreach (var platform in notFoundPlatforms) + { + parentAttributes.Platforms!.Remove(platform); + } + } + else + { + parentAttributes.Callsite = Callsite.Empty; } } } else { - parentAttributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); + pAttributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); foreach (var (platform, attributes) in childAttributes) { - parentAttributes[platform] = NormalizeAttribute(attributes); + pAttributes[platform] = NormalizeAttribute(attributes); } + parentAttributes.Platforms = pAttributes; } + return; - static PlatformAttributes NormalizeAttribute(PlatformAttributes attributes) + static Versions NormalizeAttribute(Versions attributes) { if (AllowList(attributes)) { // For Allow list UnsupportedSecond should not be used. attributes.UnsupportedSecond = null; } - else - { - // For deny list UnsupportedSecond should only set if there is SupportedFirst verison between UnsupportedSecond and UnsupportedFirst - if (attributes.SupportedFirst == null || + // For deny list UnsupportedSecond should only set if there is SupportedFirst verison between UnsupportedSecond and UnsupportedFirst + else if (attributes.SupportedFirst == null || (attributes.UnsupportedSecond != null && attributes.SupportedFirst > attributes.UnsupportedSecond)) - { - attributes.UnsupportedSecond = null; - } + { + attributes.UnsupportedSecond = null; } return attributes; } } } - private static bool TryAddValidAttribute([NotNullWhen(true)] ref SmallDictionary? attributes, AttributeData attribute) + private static bool TryAddValidAttribute([NotNullWhen(true)] ref SmallDictionary? attributes, AttributeData attribute) { if (!attribute.ConstructorArguments.IsEmpty && attribute.ConstructorArguments[0] is { } argument && @@ -1555,10 +1565,10 @@ attribute.ConstructorArguments[0] is { } argument && !argument.Value.Equals(string.Empty) && TryParsePlatformNameAndVersion(argument.Value.ToString(), out string platformName, out Version? version)) { - attributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); + attributes ??= new SmallDictionary(StringComparer.OrdinalIgnoreCase); if (!attributes.TryGetValue(platformName, out var _)) { - attributes[platformName] = new PlatformAttributes(); + attributes[platformName] = new Versions(); } AddAttribute(attribute.AttributeClass.Name, version, attributes[platformName]); @@ -1592,7 +1602,7 @@ private static bool TryParsePlatformNameAndVersion(string osString, out string o return true; } - private static void AddAttribute(string name, Version version, PlatformAttributes attributes) + private static void AddAttribute(string name, Version version, Versions attributes) { if (name == SupportedOSPlatformAttribute) { @@ -1604,7 +1614,7 @@ private static void AddAttribute(string name, Version version, PlatformAttribute AddOrUpdateUnsupportedAttribute(attributes, version); } - static void AddOrUpdateUnsupportedAttribute(PlatformAttributes attributes, Version version) + static void AddOrUpdateUnsupportedAttribute(Versions attributes, Version version) { if (attributes.UnsupportedFirst != null) { @@ -1613,13 +1623,10 @@ static void AddOrUpdateUnsupportedAttribute(PlatformAttributes attributes, Versi attributes.UnsupportedSecond = attributes.UnsupportedFirst; attributes.UnsupportedFirst = version; } - else - { - if (attributes.UnsupportedSecond == null || + else if (attributes.UnsupportedSecond == null || attributes.UnsupportedSecond > version) - { - attributes.UnsupportedSecond = version; - } + { + attributes.UnsupportedSecond = version; } } else @@ -1628,7 +1635,7 @@ static void AddOrUpdateUnsupportedAttribute(PlatformAttributes attributes, Versi } } - static void AddOrUpdateSupportedAttribute(PlatformAttributes attributes, Version version) + static void AddOrUpdateSupportedAttribute(Versions attributes, Version version) { if (attributes.SupportedFirst != null) { @@ -1650,7 +1657,7 @@ static void AddOrUpdateSupportedAttribute(PlatformAttributes attributes, Version /// /// PlatformAttributes being checked /// true if it is allow list - private static bool AllowList(PlatformAttributes attributes) => + private static bool AllowList(Versions attributes) => attributes.SupportedFirst != null && (attributes.UnsupportedFirst == null || attributes.SupportedFirst <= attributes.UnsupportedFirst); @@ -1659,7 +1666,7 @@ private static bool AllowList(PlatformAttributes attributes) => /// /// PlatformAttributes being checked /// true if it is deny list - private static bool DenyList(PlatformAttributes attributes) => + private static bool DenyList(Versions attributes) => attributes.UnsupportedFirst != null && (attributes.SupportedFirst == null || attributes.UnsupportedFirst < attributes.SupportedFirst); } diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs index 569687d741..c3eb4c65f4 100644 --- a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs +++ b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/InteropServices/PlatformCompatibilityAnalyzerTests.cs @@ -171,32 +171,32 @@ public class Test { void ThrowWithStringArgument() { - [|SR.Message|] = ""Warns not reachable on Browser""; + SR.Message = ""This does not warn because this code is not reachable on any""; throw new PlatformNotSupportedException(SR.Message); } void ThrowNotSupportedWithStringArgument() { - [|SR.Message|] = ""Warns not reachable on Browser""; + SR.Message = ""This does not warn because this code is not reachable on any""; throw new NotSupportedException(SR.Message); } void ThrowWithNoArgument() { - [|SR.Message|] = ""Warns not reachable on Browser""; + SR.Message = ""This does not warn because this code is not reachable on any""; throw new PlatformNotSupportedException(); } void ThrowWithStringAndExceptionConstructor() { - [|SR.Message|] = ""Warns not reachable on Browser""; + SR.Message = ""This does not warn because this code is not reachable on any""; throw new PlatformNotSupportedException(SR.Message, new Exception()); } void ThrowWithAnotherExceptionUsingResourceString() { - [|SR.Message|] = ""Warns not reachable on Browser""; - throw new PlatformNotSupportedException(SR.Message, new Exception([|SR.Message|])); + SR.Message = ""This does not warn because this code is not reachable on any""; + throw new PlatformNotSupportedException(SR.Message, new Exception(SR.Message)); } }"; await VerifyAnalyzerAsyncCs(csSource); @@ -278,14 +278,14 @@ public class Test [UnsupportedOSPlatform(""browser"")] public static Exception GetNotSupportedWithStringArgument() { - [|s_message|] = ""Warns not reachable on Browser""; + s_message = ""Warns not reachable on Browser""; return new NotSupportedException(s_message); } [UnsupportedOSPlatform(""browser"")] public static Exception ThrowPnseWithStringWarnsForInnerException() { - return new PlatformNotSupportedException(s_message, new Exception([|s_message|])); + return new PlatformNotSupportedException(s_message, new Exception(s_message)); } [UnsupportedOSPlatform(""browser"")] @@ -3198,6 +3198,232 @@ await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms, Join(GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityFromVersionToVersion, "windows", "10.0", "12.0"), GetFormattedString(MicrosoftNetCoreAnalyzersResources.PlatformCompatibilityFromVersionToVersion, "ios", "9.0", "10.0")))); } + [Fact] + public async Task EmptyCallsiteReferencesLocalFunctionNotWarns() + { + var source = @" +using System.Runtime.Versioning; +[SupportedOSPlatform(""Browser"")] +public class Test +{ + [UnsupportedOSPlatform(""browser"")] + public static string CurrentProgram + { + get + { + return EnsureInitialized(); + string EnsureInitialized() { return string.Empty; } + } + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task NarrowedCallsiteReferencesLocalFunctionNotWarns() + { + var source = @" +using System.Runtime.Versioning; +[SupportedOSPlatform(""Browser"")] +public class Test +{ + [UnsupportedOSPlatform(""browser2.0"")] + public static string CurrentProgram + { + get + { + return EnsureInitialized(); + string EnsureInitialized() { return string.Empty; } + } + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task EmptyCallsiteReferencesApiWithinSameAssemblyAttributeNotWarn() + { + var source = @" +using System.Runtime.Versioning; + +[assembly:SupportedOSPlatform(""Browser"")] +public class Test +{ + private string program; + + [UnsupportedOSPlatform(""browser"")] // unsupports the only supported platform of parent producing empty call site, not warning + public string CurrentProgram + { + get + { + return program; // should not warn + } + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task EmptyCallsiteReferencesApiWithImmediateAttributeNotWarn() + { + var source = @" +using System.Runtime.Versioning; + +[assembly:SupportedOSPlatform(""Browser"")] +public class Test +{ + [SupportedOSPlatform(""Browser"")] + private static string s_program; + + [UnsupportedOSPlatform(""browser"")] + public static string CurrentProgram + { + get + { + return s_program; // should not warn + } + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task EmptyCallsiteReferencesPlatformSpecificLibrariesApisNotWarn() + { + var source = @" +using System; +using System.Runtime.Versioning; + +[assembly:SupportedOSPlatform(""Browser"")] +public class Test +{ + [UnsupportedOSPlatform(""browser"")] // Overrides only support parent had, no valid call sites, not warm for anything + public void M1() + { + Console.Beep(); // Unsupported on browser API, should not warn + Console.Beep(10, 20); // Windows only API, should not warn + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task Empty_NonCallsiteReferencesVersionedApiWithImmediateAttribute() + { + var source = @" +using System; +using System.Runtime.Versioning; + +[SupportedOSPlatform(""windows7.0"")] +public class Test +{ + [UnsupportedOSPlatform(""windows9.0"")] + void StillSupportedOnWindows7and8() + { // This call site is reachable on: 'windows' from version 7.0 to 9.0. 'Test.SupportedOnWindows10()' is only supported on: 'windows' 10.0 and later. + [|SupportedOnWindows10()|]; // Warning; calling from Windows 7-8 is still supported but it will fail + } + [UnsupportedOSPlatform(""windows6.0"")] + void NotSupportedAnywhere() + { + SupportedOnWindows10(); // No warning; no valid call sites + } + [SupportedOSPlatform(""windows10.0"")] + void SupportedOnWindows10() { } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task ChildCanNarrowUpperLevelSupportedPlatforms() + { + var source = @" +using System.Runtime.Versioning; + +[assembly:SupportedOSPlatform(""windows"")] +[assembly:SupportedOSPlatform(""ios"")] +public class Test +{ + [SupportedOSPlatform(""windows"")] + private void WindowsOnly() { } + + [SupportedOSPlatform(""ios"")] + private void IosOnly () { } + + private void NoAttribute () { } + + public void M1 () + { + [|WindowsOnly()|]; + [|IosOnly()|]; + NoAttribute(); + } + [SupportedOSPlatform(""Windows"")] + public void M2 () + { + WindowsOnly(); + [|IosOnly()|]; + NoAttribute(); + } + [SupportedOSPlatform(""iOS"")] + public void M3 () + { + [|WindowsOnly()|]; + IosOnly(); + NoAttribute(); + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + + [Fact] + public async Task EmptyCallsiteReferencesSupportedUnsupportedApisNotWarns() + { + var source = @" +using System; +using System.Runtime.Versioning; + +[SupportedOSPlatform(""browser"")] +[SupportedOSPlatform(""windows"")] +[SupportedOSPlatform(""macos"")] +class Test +{ + [UnsupportedOSPlatform(""browser"")] + private static void ApiUnsupportedOnBrowser () { } + + [SupportedOSPlatform(""macos"")] + class TestMacOs // Inside only mac accessable + { + [SupportedOSPlatform(""windows"")] + void WindowsOnly() // The method attribute causes not valid call sites; no warnings no matter what + { + Test.ApiUnsupportedOnBrowser(); + Console.Beep(10, 20); // Windows only API + } + } + + void MethodHasNoAttribute() + { + [|ApiUnsupportedOnBrowser()|]; // This call site is reachable on: 'macos', 'browser', 'windows'. 'Test.ApiUnsupportedOnBrowser()' is unsupported on: 'browser'. + [|Console.Beep(10, 20)|]; // This call site is reachable on: 'macos', 'browser', 'windows'. 'Console.Beep(int, int)' is only supported on: 'windows'. + } + + [SupportedOSPlatform(""WINDOWS"")] + void WindowsOnly() // Nothing should warn + { + ApiUnsupportedOnBrowser(); + Console.Beep(10, 20); + } + + [SupportedOSPlatform(""Linux"")] // Not valid because Linux is not in parent list of platforms + void LinuxOnly() // => not valid call site; no warnings no matter what + { + ApiUnsupportedOnBrowser(); + Console.Beep(10, 20); + } +}"; + await VerifyAnalyzerAsyncCs(source, s_msBuildPlatforms); + } + private string GetFormattedString(string resource, params string[] args) => string.Format(CultureInfo.InvariantCulture, resource, args); diff --git a/src/Utilities/Compiler/SmallDictionary.cs b/src/Utilities/Compiler/SmallDictionary.cs index c0394ec536..73568162c9 100644 --- a/src/Utilities/Compiler/SmallDictionary.cs +++ b/src/Utilities/Compiler/SmallDictionary.cs @@ -68,6 +68,114 @@ private int GetHashCode(K k) return Comparer.GetHashCode(k); } + public bool IsEmpty => _root == null; + + public void Remove(K key) + { + _root = Remove(_root, GetHashCode(key)); + } + + private AvlNode? Remove(AvlNode? currentNode, int hashCode) + { + if (currentNode == null) + { + return null; + } + + var hc = currentNode.HashCode; + + if (hc > hashCode) + { + currentNode.Left = Remove(currentNode.Left, hashCode); + } + else if (hc < hashCode) + { + currentNode.Right = Remove(currentNode.Right, hashCode); + } + else + { + // node with only one child or no child + if ((currentNode.Left == null) || (currentNode.Right == null)) + { + AvlNode? temp = null; + if (temp == currentNode.Left) + temp = currentNode.Right; + else + temp = currentNode.Left; + + // No child case + if (temp == null) + { + currentNode = null; + } + else // One child case + { + currentNode = temp; + } + } + else + { + // node with two children; get the smallest in the right subtree + AvlNode temp = MinValueNode(currentNode.Right); + + // Copy the smallest in the right successor's data to this node + currentNode.HashCode = temp.HashCode; + currentNode.Value = temp.Value; + currentNode.Key = temp.Key; + + // Delete the smallest in the right successor + currentNode.Right = Remove(currentNode.Right, temp.HashCode); + } + } + + if (currentNode == null) + return null; + + currentNode.Balance = (sbyte)(Height(currentNode.Left) - Height(currentNode.Right)); + + AvlNode rotated; + var balance = currentNode.Balance; + + if (balance == -2) + { + rotated = currentNode.Right!.Balance < 0 ? + LeftSimple(currentNode) : + LeftComplex(currentNode); + } + else if (balance == 2) + { + rotated = currentNode.Left!.Balance > 0 ? + RightSimple(currentNode) : + RightComplex(currentNode); + } + else + { + return currentNode; + } + + return rotated; + } + + private static AvlNode MinValueNode(AvlNode node) + { + AvlNode current = node; + + while (current.Left != null) + current = current.Left; + + return current; + } + + private static int Height(AvlNode? node) + { + if (node == null) return 0; + + int a = Height(node.Left); + int b = Height(node.Right); + + return 1 + Math.Max(a, b); + } + public bool TryGetValue(K key, [MaybeNullWhen(returnValue: false)] out V value) { if (_root != null) @@ -116,7 +224,7 @@ internal void AssertBalanced() #pragma warning restore CA1822 private abstract class Node { - public readonly K Key; + public K Key; public V Value; protected Node(K key, V value) @@ -159,7 +267,7 @@ public AvlNodeHead(int hashCode, K key, V value, Node next) // Balance is also here for better packing of AvlNode on 64bit private abstract class HashedNode : Node { - public readonly int HashCode; + public int HashCode; public sbyte Balance; protected HashedNode(int hashCode, K key, V value)