diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ObjectCommandComparer.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ObjectCommandComparer.cs index 127ee190073..7b68d4c7064 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ObjectCommandComparer.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/ObjectCommandComparer.cs @@ -79,31 +79,30 @@ public override bool Equals(Object inputObject) { ObjectCommandPropertyValue objectCommandPropertyValueObject = inputObject as ObjectCommandPropertyValue; if (objectCommandPropertyValueObject == null) + { return false; + } object baseObject = PSObject.Base(PropertyValue); object inComingbaseObjectPropertyValue = PSObject.Base(objectCommandPropertyValueObject.PropertyValue); - IComparable baseObjectComparable = baseObject as IComparable; + if (baseObject is IComparable) + { + var success = LanguagePrimitives.TryCompare(baseObject, inComingbaseObjectPropertyValue, CaseSensitive, Culture, out int result); + return success && result == 0; + } - if (baseObjectComparable != null) + if (baseObject == null && inComingbaseObjectPropertyValue == null) { - return (LanguagePrimitives.Compare(baseObject, inComingbaseObjectPropertyValue, CaseSensitive, Culture) == 0); + return true; } - else + if (baseObject != null && inComingbaseObjectPropertyValue != null) { - if (baseObject == null && inComingbaseObjectPropertyValue == null) - { - return true; - } - if (baseObject != null && inComingbaseObjectPropertyValue != null) - { - return baseObject.ToString().Equals(inComingbaseObjectPropertyValue.ToString(), StringComparison.OrdinalIgnoreCase); - } - - // One of the property values being compared is null. - return false; + return baseObject.ToString().Equals(inComingbaseObjectPropertyValue.ToString(), StringComparison.OrdinalIgnoreCase); } + + // One of the property values being compared is null. + return false; } /// @@ -190,35 +189,30 @@ public int Compare(object first, object second) { // This method will never throw exceptions, two null // objects are considered the same - if (IsValueNull(first) && IsValueNull(second)) return 0; + if (IsValueNull(first) && IsValueNull(second)) + { + return 0; + } - PSObject firstMsh = first as PSObject; - if (firstMsh != null) + if (first is PSObject firstMsh) { first = firstMsh.BaseObject; } - PSObject secondMsh = second as PSObject; - if (secondMsh != null) + if (second is PSObject secondMsh) { second = secondMsh.BaseObject; } - try - { - return LanguagePrimitives.Compare(first, second, !_caseSensitive, _cultureInfo) * (_ascendingOrder ? 1 : -1); - } - catch (InvalidCastException) - { - } - catch (ArgumentException) + if (LanguagePrimitives.TryCompare(first, second, !_caseSensitive, _cultureInfo, out int result)) { - // Note that this will occur if the objects do not support - // IComparable. We fall back to comparing as strings. + return result * (_ascendingOrder ? 1 : -1); } + // Note that this will occur if the objects do not support + // IComparable. We fall back to comparing as strings. + // being here means the first object doesn't support ICompare - // or an Exception was raised win Compare string firstString = PSObject.AsPSObject(first).ToString(); string secondString = PSObject.AsPSObject(second).ToString(); diff --git a/src/System.Management.Automation/FormatAndOutput/common/FormatGroupManager.cs b/src/System.Management.Automation/FormatAndOutput/common/FormatGroupManager.cs index e4cf1d7f70f..1185a151dce 100644 --- a/src/System.Management.Automation/FormatAndOutput/common/FormatGroupManager.cs +++ b/src/System.Management.Automation/FormatAndOutput/common/FormatGroupManager.cs @@ -83,19 +83,14 @@ internal bool UpdateGroupingKeyValue(PSObject so) private static bool IsEqual(object first, object second) { - try + if (LanguagePrimitives.TryCompare(first, second, true, CultureInfo.CurrentCulture, out int result)) { - return LanguagePrimitives.Compare(first, second, true, CultureInfo.CurrentCulture) == 0; - } - catch (InvalidCastException) - { - } - catch (ArgumentException) - { - // Note that this will occur if the objects do not support - // IComparable. We fall back to comparing as strings. + return result == 0; } + // Note that this will occur if the objects do not support + // IComparable. We fall back to comparing as strings. + // being here means the first object doesn't support ICompare // or an Exception was raised win Compare string firstString = PSObject.AsPSObject(first).ToString(); diff --git a/src/System.Management.Automation/engine/LanguagePrimitives.cs b/src/System.Management.Automation/engine/LanguagePrimitives.cs index f087dc7038e..69b0a9d62bb 100644 --- a/src/System.Management.Automation/engine/LanguagePrimitives.cs +++ b/src/System.Management.Automation/engine/LanguagePrimitives.cs @@ -695,6 +695,30 @@ public static bool Equals(object first, object second, bool ignoreCase, IFormatP return false; } + /// + /// Helper method for [Try]Compare to determine object ordering with null. + /// + /// the numeric value to compare to null + /// True if the number to compare is on the right hand side if the comparison. + private static int CompareObjectToNull(object value, bool numberIsRightHandSide) + { + var i = numberIsRightHandSide ? -1 : 1; + + // If it's a positive number, including 0, it's greater than null + // for everything else it's less than zero... + switch (value) + { + case Int16 i16: return Math.Sign(i16) < 0 ? -i : i; + case Int32 i32: return Math.Sign(i32) < 0 ? -i : i; + case Int64 i64: return Math.Sign(i64) < 0 ? -i : i; + case sbyte sby: return Math.Sign(sby) < 0 ? -i : i; + case float f: return Math.Sign(f) < 0 ? -i : i; + case double d: return Math.Sign(d) < 0 ? -i : i; + case decimal de: return Math.Sign(de) < 0 ? -i : i; + default: return i; + } + } + /// /// Compare first and second, converting second to the /// type of the first, if necessary. @@ -762,43 +786,12 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP if (first == null) { - if (second == null) - { - return 0; - } - else - { - // If it's a positive number, including 0, it's greater than null - // for everything else it's less than zero... - switch (LanguagePrimitives.GetTypeCode(second.GetType())) - { - case TypeCode.Int16: return System.Math.Sign((Int16)second) < 0 ? 1 : -1; - case TypeCode.Int32: return System.Math.Sign((Int32)second) < 0 ? 1 : -1; - case TypeCode.Int64: return System.Math.Sign((Int64)second) < 0 ? 1 : -1; - case TypeCode.SByte: return System.Math.Sign((sbyte)second) < 0 ? 1 : -1; - case TypeCode.Single: return System.Math.Sign((System.Single)second) < 0 ? 1 : -1; - case TypeCode.Double: return System.Math.Sign((System.Double)second) < 0 ? 1 : -1; - case TypeCode.Decimal: return System.Math.Sign((System.Decimal)second) < 0 ? 1 : -1; - default: return -1; - } - } + return second == null ? 0 : CompareObjectToNull(second, true); } if (second == null) { - // If it's a positive number, including 0, it's greater than null - // for everything else it's less than zero... - switch (LanguagePrimitives.GetTypeCode(first.GetType())) - { - case TypeCode.Int16: return System.Math.Sign((Int16)first) < 0 ? -1 : 1; - case TypeCode.Int32: return System.Math.Sign((Int32)first) < 0 ? -1 : 1; - case TypeCode.Int64: return System.Math.Sign((Int64)first) < 0 ? -1 : 1; - case TypeCode.SByte: return System.Math.Sign((sbyte)first) < 0 ? -1 : 1; - case TypeCode.Single: return System.Math.Sign((System.Single)first) < 0 ? -1 : 1; - case TypeCode.Double: return System.Math.Sign((System.Double)first) < 0 ? -1 : 1; - case TypeCode.Decimal: return System.Math.Sign((System.Decimal)first) < 0 ? -1 : 1; - default: return 1; - } + return CompareObjectToNull(first, false); } if (first is string firstString) @@ -854,6 +847,127 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP throw PSTraceSource.NewArgumentException("first", ExtendedTypeSystem.NotIcomparable, first.ToString()); } + /// + /// Tries to compare first and second, converting second to the type of the first, if necessary. + /// If a conversion is needed but fails, false is return. + /// + /// First comparison value. + /// Second comparison value. + /// Less than zero if first is smaller than second, more than + /// zero if it is greater or zero if they are the same. + /// True if the comparison was successful, false otherwise. + public static bool TryCompare(object first, object second, out int result) + { + return TryCompare(first, second, ignoreCase: false, CultureInfo.InvariantCulture, out result); + } + + /// + /// Tries to compare first and second, converting second to the type of the first, if necessary. + /// If a conversion is needed but fails, false is return. + /// + /// First comparison value. + /// Second comparison value. + /// Used if both values are strings. + /// Less than zero if first is smaller than second, more than zero if it is greater or zero if they are the same. + /// True if the comparison was successful, false otherwise. + public static bool TryCompare(object first, object second, bool ignoreCase, out int result) + { + return TryCompare(first, second, ignoreCase, CultureInfo.InvariantCulture, out result); + } + + /// + /// Tries to compare first and second, converting second to the type of the first, if necessary. + /// If a conversion is needed but fails, false is return. + /// + /// First comparison value. + /// Second comparison value. + /// Used if both values are strings. + /// Used in type conversions and if both values are strings. + /// Less than zero if first is smaller than second, more than zero if it is greater or zero if they are the same. + /// True if the comparison was successful, false otherwise. + /// The parameter is not a . + public static bool TryCompare(object first, object second, bool ignoreCase, IFormatProvider formatProvider, out int result) + { + result = 0; + if (formatProvider == null) + { + formatProvider = CultureInfo.InvariantCulture; + } + + if (!(formatProvider is CultureInfo culture)) + { + throw PSTraceSource.NewArgumentException("formatProvider"); + } + + first = PSObject.Base(first); + second = PSObject.Base(second); + + if (first == null && second == null) + { + result = 0; + return true; + } + + if (first == null) + { + result = CompareObjectToNull(second, true); + return true; + } + + if (second == null) + { + // If it's a positive number, including 0, it's greater than null + // for everything else it's less than zero... + result = CompareObjectToNull(first, false); + return true; + } + + if (first is string firstString) + { + if (!(second is string secondString)) + { + if (!TryConvertTo(second, culture, out secondString)) + { + return false; + } + } + + result = culture.CompareInfo.Compare(firstString, secondString, ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None); + return true; + } + + Type firstType = first.GetType(); + Type secondType = second.GetType(); + int firstIndex = TypeTableIndex(firstType); + int secondIndex = TypeTableIndex(secondType); + if (firstIndex != -1 && secondIndex != -1) + { + result = NumericCompare(first, second, firstIndex, secondIndex); + return true; + } + + if (!TryConvertTo(second, firstType, culture, out object secondConverted)) + { + return false; + } + + if (first is IComparable firstComparable) + { + result = firstComparable.CompareTo(secondConverted); + return true; + } + + if (first.Equals(second)) + { + result = 0; + return true; + } + + // At this point, we know that they aren't equal but we have no way of + // knowing which should compare greater than the other so we return false. + return false; + } + /// /// Returns true if the language considers obj to be true /// diff --git a/test/powershell/engine/Api/LanguagePrimitive.Tests.ps1 b/test/powershell/engine/Api/LanguagePrimitive.Tests.ps1 index 7b34e9174c9..f4c3a5975cc 100644 --- a/test/powershell/engine/Api/LanguagePrimitive.Tests.ps1 +++ b/test/powershell/engine/Api/LanguagePrimitive.Tests.ps1 @@ -29,7 +29,7 @@ Describe "Language Primitive Tests" -Tags "CI" { } It "Casting recursive array to bool should not cause crash" { - $a[0] = $a = [PSObject](,1) + $a[0] = $a = [PSObject](, 1) [System.Management.Automation.LanguagePrimitives]::IsTrue($a) | Should -BeTrue } @@ -46,4 +46,60 @@ Describe "Language Primitive Tests" -Tags "CI" { [System.Management.Automation.LanguagePrimitives]::GetEnumerable($dt) | ForEach-Object { $count++ } $count | Should -Be 2 } + + It "TryCompare should succeed on int and string" { + $result = $null + [System.Management.Automation.LanguagePrimitives]::TryCompare(1, "1", [ref] $result) | Should -BeTrue + $result | Should -Be 0 + } + + It "TryCompare should fail on int and datetime" { + $result = $null + [System.Management.Automation.LanguagePrimitives]::TryCompare(1, [datetime]::Now, [ref] $result) | Should -BeFalse + } + + It "TryCompare should succeed on int and int and compare correctly smaller" { + $result = $null + [System.Management.Automation.LanguagePrimitives]::TryCompare(1, 2, [ref] $result) | Should -BeTrue + $result | Should -BeExactly -1 + } + + It "TryCompare should succeed on string and string and compare correctly greater" { + $result = $null + [System.Management.Automation.LanguagePrimitives]::TryCompare("bbb", "aaa", [ref] $result) | Should -BeTrue + $result | Should -BeExactly 1 + } + + It "TryCompare should succeed on string and string and compare case insensitive correctly" { + $result = $null + [System.Management.Automation.LanguagePrimitives]::TryCompare("AAA", "aaa", $true, [ref] $result) | Should -BeTrue + $result | Should -BeExactly 0 + } + + It "TryCompare with cultureInfo is culture sensitive" { + $result = $null + $swedish = [cultureinfo] 'sv-SE' + # in Swedish, åäö appears at the end of the alphabet, and should compare greater than o + $val = [System.Management.Automation.LanguagePrimitives]::TryCompare("ooo", "ååå", $false, $swedish, [ref] $result) + $val | Should -BeTrue + $result | Should -BeExactly -1 + } + + It "TryCompare compares greater than null as Compare" { + $result = $null + + $compareResult = [System.Management.Automation.LanguagePrimitives]::Compare($null, 10) + $val = [System.Management.Automation.LanguagePrimitives]::TryCompare($null, 10, [ref] $result) + $val | Should -BeTrue + $result | Should -BeExactly $compareResult + } + + It "TryCompare compares less than null as Compare" { + $result = $null + + $compareResult = [System.Management.Automation.LanguagePrimitives]::Compare(10, $null) + $val = [System.Management.Automation.LanguagePrimitives]::TryCompare(10, $null, [ref] $result) + $val | Should -BeTrue + $result | Should -BeExactly $compareResult + } }