Implicitly convert XML rval to string - #2678
Conversation
There was a problem hiding this comment.
Calling ToString directly avoids any magic PowerShell might apply when converting a value to string.
You could just call LanguagePrimitives.ConvertTo<string>(setValue) - this will properly call into a CodeMethod or ScriptMethod named ToString if the object has one.
There was a problem hiding this comment.
You should also test that we properly invoke ToString implemented as a CodeMethod and ScriptMethod. System.Type has a CodeMethod implementation of ToString, you can see the difference:
#57 PS> [System.Int32].ToString()
System.Int32
#58 PS> [string][System.Int32]
int
There was a problem hiding this comment.
Done.
Only I could not create ToString ScriptMethod because ToString already exists.
d6f72d2 to
3ffe8fb
Compare
|
Jason Shirk (@lzybkr) could you continue the code review? |
There was a problem hiding this comment.
I think this condition is unreachable now.
We can convert everything to string (including $null which turns into the empty string).
The only failure condition now is if the conversion to string fails with an exception, e.g. if ToString() throws an exception. This should be rare, but if it happens, I think we'd want to see that exception and not the SetValueException that is being thrown here.
In other words, I think this test and subsequent throw can be safely deleted.
There was a problem hiding this comment.
I would add tests for:
- $null
- array of strings
- array of integers
- psobject wrapping each of the previous values
There was a problem hiding this comment.
Fixed.
But I have doubts about [PSObject]::AsPSObject($null) and [PSObject]::AsPSObject(1)
There was a problem hiding this comment.
We don't always call ToString.
PS> $x = 1,2
PS> [System.Management.Automation.LanguagePrimitives]::ConvertTo($x, [string])
1 2
PS> $x.ToString()
System.Object[]1. Implicitly convert XML rval to string 2. Add test
a0c98ba to
a1a94ba
Compare
|
Jason Shirk (@lzybkr) Do you have any other comments or concerns regarding this PR? |
|
Jason Shirk (@lzybkr) Thanks for code review and great comments! |
…ll#2678) Using LanguagePrimitives.ConvertTo (to emulate what would happen if the property were a regular .Net property typed as string), we now convert the value assigned to an XML property to string.
Close #2459