Powershell: Unexpected argument conversion $null to String.Empty

[iSazonov]

See conversation in Core repo https://github.com/dotnet/runtime/pull/1057#discussion_r363868514

Perhaps it is the same as #4616 but if it is by design for comparisons I don't agree that it is right for passing arguments.

Steps to reproduce

[System.Net.NetworkInformation.PhysicalAddress]::Parse("")
[System.Net.NetworkInformation.PhysicalAddress]::Parse($null)

Expected behavior

Second (Parse($null)) should return PhysicalAddress.None without exception

Actual behavior

PS C:\> [System.Net.NetworkInformation.PhysicalAddress]::Parse("")
MethodInvocationException: Exception calling "Parse" with "1" argument(s): "An invalid physical address was specified: ''."
PS C:\> [System.Net.NetworkInformation.PhysicalAddress]::Parse($null)
MethodInvocationException: Exception calling "Parse" with "1" argument(s): "An invalid physical address was specified: ''."

Environment data

Name                           Value
----                           -----
PSVersion                      7.0.0-preview.6
PSEdition                      Core
GitCommitId                    7.0.0-preview.6-84-g9d14bc6959c0f0f055ba7a2a01512290f3349ed8
OS                             Microsoft Windows 10.0.17763
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[iSazonov]

/cc @alx9r @lzybkr @mklement0 @bergmeister @SteveL-MSFT @daxian-dbw

[daxian-dbw]

This is why PowerShell has [NullString]::Value. It should be used if you want to pass null to a string type parameter of a method. I admit it's not intuitive, but not sure what it would break to change it.

[SeeminglyScience]

but not sure what it would break to change it.

Yeah, I've definitely run into methods that throw ArgumentNullException on null but accept empty strings (even a few PowerShell API's iirc).

For as long as this has been a thing in PowerShell, it comes up very rarely. Also I can only think of one or two occasions where I've actually had to use NullString tbh.

[iSazonov]

I very wonder that (@bergmeister's comment from #4616)

If you simply remove the [string] type then it will work as expected:
we have a workaround for scripts and not for C# methods.

Could we enhance the Core adapter to pass null-s as-is?

[SeeminglyScience]

I very wonder that (@bergmeister's comment from #4616)

If you simply remove the [string] type then it will work as expected:
we have a workaround for scripts and not for C# methods.

Removing [string] just types it as [object], which works only because it's no longer trying to convert to [string].

Could we enhance the Core adapter to pass null-s as-is?

It's not a bug or limitation though, it's by design (sorry if you already understand that, double checking because of the word choice "enhance").

Not sure if it was the right choice or not, but at this point it'd be a very dangerous change. Here's an example of the pattern I mentioned in PowerShell itself:

$ExecutionContext.SessionState.Path.CurrentProviderPath($null)
# Path
# ----
# C:\

$ExecutionContext.SessionState.Path.CurrentProviderPath([NullString]::Value)
# MethodInvocationException: Exception calling "CurrentProviderLocation" with "1" argument(s): "Cannot process argument because the value of argument "namespaceID" is null. Change the value of argument "namespaceID" to a non-null value."

PowerShell users would have no idea that these methods didn't accept $null until they start throwing after an upgrade.

[iSazonov]

@SeeminglyScience In my previous comment I ask about Core adapter where we bind C# methods.

[SeeminglyScience]

@SeeminglyScience In my previous comment I ask about Core adapter where we bind C# methods.

I think we're talking about the same thing. Did my example show something other than what you're referring to?

[mattpwhite]

PowerShell users would have no idea that these methods didn't accept $null until they start throwing after an upgrade.

This is a fair point, but it seems at least as likely that they'll be bitten by silent conversions of null to an empty string at some point. When that inevitably does happen, it will be much less obvious what is happening or how to avoid it. Empty string and null are things that virtually everyone knows and hopefully understands. Reasonably documented libraries typically indicate whether or not null is valid input. Very few people know what NullString is or why they might need to use it.

PowerShell is pretty rotten with gotchas and POLA violations like this. In aggregate, they impose a significant mental load on users and make it really hard to write anything robust and reliable. Given the huge compatibility break with the move to Core anyway, I wish there was a greater appetite for taking breaking changes that made the language behave in a more reasonable way.

Lastly, at least speaking from personal experience, assignment to properties and parameter coercion cause even more NullString problems than method arguments. Especially properties of objects that are going to be serialized as part of some REST call, where a null would cause the property to be left out completely and an empty string means that the property ends up being explicitly specified as an empty string.

PS C:\> class foo {[string] $bar}
>> $foo = [foo]::new()
>> $null -eq $foo.bar
>> $foo.bar = $null
>> $null -eq $foo.bar
True
False
PS C:\> function foo {param([string]$bar) $null -eq $bar}
PS C:\> foo $null
False

[SeeminglyScience]

Very few people know what NullString is or why they might need to use it.

Most don't need to really. Only time I concretely remember running into it personally was a funky p/invoke signature, and the only super impactful scenario I know of is the one you bring up later: serialization edge cases.

I'm guessing it was initially added to to cut down on the amount of if ($null -eq $string) { '' } else { $string } that tends to happen in string-centric code in other languages. Do I agree with it? Ehhhh not really, I wish it wasn't there; but it is and we've all depended on it whether we know it or not.

PowerShell is pretty rotten with gotchas and POLA violations like this. In aggregate, they impose a significant mental load on users and make it really hard to write anything robust and reliable.

No disagreement there, with an emphasis on robust and reliable. PowerShell does a ton for you that you don't have to think about, and that's great. With all of those abstractions comes the cost of complexity though. It's easy to make a thing, but the more sophisticated that thing becomes, and the more resilient it needs to be, the more the difficulty curve starts to tip. That's why it's fantastic for automation, but kinda funky to try to make a product with.

Given the huge compatibility break with the move to Core anyway, I wish there was a greater appetite for taking breaking changes that made the language behave in a more reasonable way.

Eh it wasn't that huge of a break. I mean initially yeah it was tremendous, but a lot less these days. That said, if this was changed, fixing the scripts that it breaks would be very difficult. Especially for those who are completely unaware of this behavior or really any of the automatic conversions.

Something as simple as param([string] $s) $s.Trim() would now throw if passed $null. Trying to explain to all the folks that this would break: "yeah you need to check for null now, yes it used to do that for you but sometimes it made serialization inconsistent or was confusing when using certain APIs" is just not gonna fly.

The PS team isn't completely resistant to breaking changes, but this one is a hard Bucket 1.