Skip to content

Revert the PRs that made DBNull.Value and NullString.Value treated as $null #11584

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
adityapatwardhan merged 5 commits into from
Jan 15, 2020
Merged

Revert the PRs that made DBNull.Value and NullString.Value treated as $null #11584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e035a28
Revert "Use more effective evaluation order in LanguagePrimitives.IsN…
daxian-dbw Jan 14, 2020
21a8a3f
Revert "Consider `DBNull.Value` and `NullString.Value` the same as `$…
daxian-dbw Jan 14, 2020
ce64abb
Update Coalescing and Null-conditional operators to use 'IsNull' instead
daxian-dbw Jan 14, 2020
9923f3b
Fix 2 test failures
daxian-dbw Jan 15, 2020
8c8cb32
Fix the spelling issue in 'preview.md'
daxian-dbw Jan 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions CHANGELOG/preview.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
- Update `ConciseView` to use `TargetObject` if applicable (#11075)
- Fix `NullReferenceException` in `CompletionCompleters` public methods (#11274)
- Fix apartment thread state check on non-Windows platforms (#11301)
- Update setting PSModulePath to concatenate the process and machine environment variables (#11276)
- Update setting `PSModulePath` to concatenate the process and machine environment variables (#11276)
- Bump `.NET Core` to `3.1.0` (#11260)
- Fix detection of `$PSHOME` in front of `$env:PATH` (#11141)

Expand All @@ -27,8 +27,8 @@
- Update calculation of char width to respect `CJK` chars correctly (#11262)
- Add `Unblock-File` for macOS (#11137)
- Fix regression in `Get-PSCallStack` (#11210) (Thanks @iSazonov!)
- Remove autoloading of the `ScheduledJob` module when using Job cmdlets (#11194)
- Add `OutputType` to `Get-Error` cmdlet and preserve original typenames (#10856)
- Avoid automatically loading the `ScheduledJob` module when using Job cmdlets (#11194)
- Add `OutputType` to `Get-Error` cmdlet and preserve original `TypeNames` (#10856)
- Fix null reference in `SupportsVirtualTerminal` property (#11105)

### Code Cleanup
Expand Down
149 changes: 72 additions & 77 deletions src/System.Management.Automation/engine/LanguagePrimitives.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,9 @@ public static PSDataCollection<PSObject> GetPSDataCollection(object inputValue)
/// <param name="second">Object to compare first to.</param>
/// <returns>True if first is equal to the second.</returns>
public static new bool Equals(object first, object second)
=> Equals(first, second, false, CultureInfo.InvariantCulture);
{
return Equals(first, second, false, CultureInfo.InvariantCulture);
}

/// <summary>
/// Used to compare two objects for equality converting the second to the type of the first, if required.
Expand All @@ -604,7 +606,9 @@ public static PSDataCollection<PSObject> GetPSDataCollection(object inputValue)
/// to specify the type of string comparison </param>
/// <returns>True if first is equal to the second.</returns>
public static bool Equals(object first, object second, bool ignoreCase)
=> Equals(first, second, ignoreCase, CultureInfo.InvariantCulture);
{
return Equals(first, second, ignoreCase, CultureInfo.InvariantCulture);
}

/// <summary>
/// Used to compare two objects for equality converting the second to the type of the first, if required.
Expand Down Expand Up @@ -642,28 +646,25 @@ public static bool Equals(object first, object second, bool ignoreCase, IFormatP

if (first == null)
{
return IsNullLike(second);
if (second == null) return true;
return false;
}

if (second == null)
{
return IsNullLike(first);
return false; // first is not null
}

string firstString = first as string;
string secondString;
if (first is string firstString)
if (firstString != null)
{
secondString = second as string ?? (string)LanguagePrimitives.ConvertTo(second, typeof(string), culture);
return culture.CompareInfo.Compare(
firstString,
secondString,
ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None) == 0;
return (culture.CompareInfo.Compare(firstString, secondString,
ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None) == 0);
}

if (first.Equals(second))
{
return true;
}
if (first.Equals(second)) return true;

Type firstType = first.GetType();
Type secondType = second.GetType();
Expand Down Expand Up @@ -707,24 +708,24 @@ public static bool Equals(object first, object second, bool ignoreCase, IFormatP
/// Helper method for [Try]Compare to determine object ordering with null.
/// </summary>
/// <param name="value">The numeric value to compare to null.</param>
/// <param name="numberIsRightHandSide">True if the number to compare is on the right hand side in the comparison.</param>
/// <param name="numberIsRightHandSide">True if the number to compare is on the right hand side if the comparison.</param>
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...
return value switch
{
Int16 i16 => Math.Sign(i16) < 0 ? -i : i,
Int32 i32 => Math.Sign(i32) < 0 ? -i : i,
Int64 i64 => Math.Sign(i64) < 0 ? -i : i,
sbyte s => Math.Sign(s) < 0 ? -i : i,
float f => Math.Sign(f) < 0 ? -i : i,
double d => Math.Sign(d) < 0 ? -i : i,
decimal m => Math.Sign(m) < 0 ? -i : i,
_ => IsNullLike(value) ? 0 : i
};
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;
}
}

/// <summary>
Expand All @@ -740,7 +741,9 @@ private static int CompareObjectToNull(object value, bool numberIsRightHandSide)
/// to the type of <paramref name="first"/>.
/// </exception>
public static int Compare(object first, object second)
=> LanguagePrimitives.Compare(first, second, false, CultureInfo.InvariantCulture);
{
return LanguagePrimitives.Compare(first, second, false, CultureInfo.InvariantCulture);
}

/// <summary>
/// Compare first and second, converting second to the
Expand All @@ -756,7 +759,9 @@ public static int Compare(object first, object second)
/// to the type of <paramref name="first"/>.
/// </exception>
public static int Compare(object first, object second, bool ignoreCase)
=> LanguagePrimitives.Compare(first, second, ignoreCase, CultureInfo.InvariantCulture);
{
return LanguagePrimitives.Compare(first, second, ignoreCase, CultureInfo.InvariantCulture);
}

/// <summary>
/// Compare first and second, converting second to the
Expand All @@ -774,20 +779,23 @@ public static int Compare(object first, object second, bool ignoreCase)
/// </exception>
public static int Compare(object first, object second, bool ignoreCase, IFormatProvider formatProvider)
{
formatProvider ??= CultureInfo.InvariantCulture;
if (formatProvider == null)
{
formatProvider = CultureInfo.InvariantCulture;
}

var culture = formatProvider as CultureInfo;
if (culture == null)
{
throw PSTraceSource.NewArgumentException(nameof(formatProvider));
throw PSTraceSource.NewArgumentException("formatProvider");
}

first = PSObject.Base(first);
second = PSObject.Base(second);

if (first == null)
{
return CompareObjectToNull(second, true);
return second == null ? 0 : CompareObjectToNull(second, true);
}

if (second == null)
Expand All @@ -797,7 +805,7 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP

if (first is string firstString)
{
var secondString = second as string;
string secondString = second as string;
if (secondString == null)
{
try
Expand All @@ -806,26 +814,19 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP
}
catch (PSInvalidCastException e)
{
throw PSTraceSource.NewArgumentException(
nameof(second),
ExtendedTypeSystem.ComparisonFailure,
first.ToString(),
second.ToString(),
e.Message);
throw PSTraceSource.NewArgumentException("second", ExtendedTypeSystem.ComparisonFailure,
first.ToString(), second.ToString(), e.Message);
}
}

return culture.CompareInfo.Compare(
firstString,
secondString,
ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None);
return culture.CompareInfo.Compare(firstString, secondString,
ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None);
}

Type firstType = first.GetType();
Type secondType = second.GetType();
int firstIndex = LanguagePrimitives.TypeTableIndex(firstType);
int secondIndex = LanguagePrimitives.TypeTableIndex(secondType);

if ((firstIndex != -1) && (secondIndex != -1))
{
return LanguagePrimitives.NumericCompare(first, second, firstIndex, secondIndex);
Expand All @@ -838,12 +839,8 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP
}
catch (PSInvalidCastException e)
{
throw PSTraceSource.NewArgumentException(
nameof(second),
ExtendedTypeSystem.ComparisonFailure,
first.ToString(),
second.ToString(),
e.Message);
throw PSTraceSource.NewArgumentException("second", ExtendedTypeSystem.ComparisonFailure,
first.ToString(), second.ToString(), e.Message);
}

if (first is IComparable firstComparable)
Expand All @@ -858,7 +855,7 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP

// 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 throw an exception.
throw PSTraceSource.NewArgumentException(nameof(first), ExtendedTypeSystem.NotIcomparable, first.ToString());
throw PSTraceSource.NewArgumentException("first", ExtendedTypeSystem.NotIcomparable, first.ToString());
}

/// <summary>
Expand All @@ -871,7 +868,9 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP
/// zero if it is greater or zero if they are the same.</param>
/// <returns>True if the comparison was successful, false otherwise.</returns>
public static bool TryCompare(object first, object second, out int result)
=> TryCompare(first, second, ignoreCase: false, CultureInfo.InvariantCulture, out result);
{
return TryCompare(first, second, ignoreCase: false, CultureInfo.InvariantCulture, out result);
}

/// <summary>
/// Tries to compare first and second, converting second to the type of the first, if necessary.
Expand All @@ -883,7 +882,9 @@ public static bool TryCompare(object first, object second, out int result)
/// <param name="result">Less than zero if first is smaller than second, more than zero if it is greater or zero if they are the same.</param>
/// <returns>True if the comparison was successful, false otherwise.</returns>
public static bool TryCompare(object first, object second, bool ignoreCase, out int result)
=> TryCompare(first, second, ignoreCase, CultureInfo.InvariantCulture, out result);
{
return TryCompare(first, second, ignoreCase, CultureInfo.InvariantCulture, out result);
}

/// <summary>
/// Tries to compare first and second, converting second to the type of the first, if necessary.
Expand All @@ -899,7 +900,10 @@ public static bool TryCompare(object first, object second, bool ignoreCase, out
public static bool TryCompare(object first, object second, bool ignoreCase, IFormatProvider formatProvider, out int result)
{
result = 0;
formatProvider ??= CultureInfo.InvariantCulture;
if (formatProvider == null)
{
formatProvider = CultureInfo.InvariantCulture;
}

if (!(formatProvider is CultureInfo culture))
{
Expand Down Expand Up @@ -984,10 +988,8 @@ public static bool TryCompare(object first, object second, bool ignoreCase, IFor
public static bool IsTrue(object obj)
{
// null is a valid argument - it converts to false...
if (IsNullLike(obj))
{
if (obj == null || obj == AutomationNull.Value)
return false;
}

obj = PSObject.Base(obj);

Expand All @@ -1013,7 +1015,8 @@ public static bool IsTrue(object obj)
if (objType == typeof(SwitchParameter))
return ((SwitchParameter)obj).ToBool();

if (obj is IList objectArray)
IList objectArray = obj as IList;
if (objectArray != null)
{
return IsTrue(objectArray);
}
Expand Down Expand Up @@ -1058,20 +1061,15 @@ internal static bool IsTrue(IList objectArray)
}
}

/// <summary>
/// Internal routine that determines if an object meets any of our criteria for true null.
/// </summary>
/// <param name="obj">The object to test.</param>
/// <returns>True if the object is null.</returns>
public static bool IsNull(object obj) => obj == null || obj == AutomationNull.Value;

/// <summary>
/// Internal routine that determines if an object meets any of our criteria for null.
/// This method additionally checks for <see cref="NullString.Value"/> and <see cref="DBNull.Value"/>
/// </summary>
/// <param name="obj">The object to test.</param>
/// <returns>True if the object is null.</returns>
public static bool IsNullLike(object obj) => IsNull(obj) || obj == DBNull.Value || obj == NullString.Value;
internal static bool IsNull(object obj)
{
return (obj == null || obj == AutomationNull.Value);
}

/// <summary>
/// Auxiliary for the cases where we want a new PSObject or null.
Expand Down Expand Up @@ -3086,17 +3084,15 @@ private static object ConvertToVoid(object valueToConvert,
return AutomationNull.Value;
}

private static bool ConvertClassToBool(
object valueToConvert,
Type resultType,
bool recursion,
PSObject originalValueToConvert,
IFormatProvider formatProvider,
TypeTable backupTable)
private static bool ConvertClassToBool(object valueToConvert,
Type resultType,
bool recursion,
PSObject originalValueToConvert,
IFormatProvider formatProvider,
TypeTable backupTable)
{
typeConversion.WriteLine("Converting ref to boolean.");
// Both NullString and DBNull should be treated the same as true nulls for the purposes of this conversion.
return !IsNullLike(valueToConvert);
return valueToConvert != null;
}

private static bool ConvertValueToBool(object valueToConvert,
Expand Down Expand Up @@ -4712,11 +4708,10 @@ internal static IConversionData FigureConversion(object valueToConvert, Type res
{
PSObject valueAsPsObj;
Type originalType;

if (IsNull(valueToConvert))
if (valueToConvert == null || valueToConvert == AutomationNull.Value)
{
originalType = typeof(Null);
valueAsPsObj = null;
originalType = typeof(Null);
}
else
{
Expand Down
10 changes: 5 additions & 5 deletions src/System.Management.Automation/engine/parser/Compiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ internal static class CachedReflectionInfo

internal static readonly MethodInfo LanguagePrimitives_GetInvalidCastMessages =
typeof(LanguagePrimitives).GetMethod(nameof(LanguagePrimitives.GetInvalidCastMessages), StaticFlags);
internal static readonly MethodInfo LanguagePrimitives_IsNullLike =
typeof(LanguagePrimitives).GetMethod(nameof(LanguagePrimitives.IsNullLike), StaticPublicFlags);
internal static readonly MethodInfo LanguagePrimitives_IsNull =
typeof(LanguagePrimitives).GetMethod(nameof(LanguagePrimitives.IsNull), StaticFlags);
internal static readonly MethodInfo LanguagePrimitives_ThrowInvalidCastException =
typeof(LanguagePrimitives).GetMethod(nameof(LanguagePrimitives.ThrowInvalidCastException), StaticFlags);

Expand Down Expand Up @@ -982,7 +982,7 @@ private static Expression Coalesce(Expression left, Expression right)
{
return left;
}
else if(leftType == typeof(DBNull) || leftType == typeof(NullString) || leftType == typeof(AutomationNull))
else if(leftType == typeof(AutomationNull))
{
return right;
}
Expand All @@ -992,7 +992,7 @@ private static Expression Coalesce(Expression left, Expression right)
Expression rhs = right.Cast(typeof(object));

return Expression.Condition(
Expression.Call(CachedReflectionInfo.LanguagePrimitives_IsNullLike, lhs),
Expression.Call(CachedReflectionInfo.LanguagePrimitives_IsNull, lhs),
rhs,
lhs);
}
Expand Down Expand Up @@ -6547,7 +6547,7 @@ public object VisitIndexExpression(IndexExpressionAst indexExpressionAst)
private static Expression GetNullConditionalWrappedExpression(Expression targetExpr, Expression memberAccessExpression)
{
return Expression.Condition(
Expression.Call(CachedReflectionInfo.LanguagePrimitives_IsNullLike, targetExpr.Cast(typeof(object))),
Expression.Call(CachedReflectionInfo.LanguagePrimitives_IsNull, targetExpr.Cast(typeof(object))),
ExpressionCache.NullConstant,
memberAccessExpression);
}
Expand Down
Loading