Repository navigation
Empty key name converting to PSObject - #2820
Kitt Holland (kittholland) wants to merge 2 commits into
Conversation
In issue PowerShell#1755 we determined that you could not create a PSObject with a key name of an empty string. This is supported in JSON and in Hashtables. Simple change was to lower evaluations of name field from 'IsNullOrEmpty' to '== null'. Simple test to confirm is: [pscustomobject]@{'' = 'Test'} Simple JSON to test which results in ths same error: { "": "Test" } After these changes, both of these cases work and I am able to succesfully Convert project.lock.json from JSON and back to JSON with no data loss.
This no longer checks for empty names.
|
Hi Kitt Holland (@kittholland), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
|
I wonder if it makes more sense to "reserve" a property name for the JSON cmdlets to use instead of allowing the empty string as a property name. |
|
I'm not sure how people feel about the valid hashtable @{'' = 'Test'} failing to convert to psobject as well. Not sure if that is a bug or intended. If we add a reserved property for JSON would we want to handle the hashtable conversion case as well? |
Jason Shirk (lzybkr)
added
the
Review - Committee
|
@PowerShell/powershell-committee discussed this and agree we should support this for JSON (and investigate if YAML will be an issue when we support YAML), but not something we should support in the language for objects as there are situations where we would error today that would start working due to unintended circumstances. Recommendation is see how JSON.Net is handling this case and use as a precedent |
Steve Lee (SteveL-MSFT)
added
Committee-Reviewed
|
I experimented with Newtonsoft.Json and they don't have this problem as deserializing to a specific type would require that object to have a property with empty name or they deserialize to a Newtonsoft.Json.Linq.JProperty. It seems that for PowerShell to support this and not introduce the concept of an empty property name to the language, we may need to introduce a json type. |
|
Can I close this PR? I don't think we will be implementing a solution with this type of approach. |
|
Because of the potential language issues raised, e.g. |
5 participants
In issue #1755 we determined that you could not create a PSObject with a
key name of an empty string. This is supported in JSON and in
Hashtables.
Simple change was to lower evaluations of name field from
'IsNullOrEmpty' to '== null'.
Simple test to confirm is:
[pscustomobject]@{'' = 'Test'}
Simple JSON to test which results in ths same error:
'{"": "Test"}' | ConvertFrom-Json
After these changes, both of these cases work and I am able to
succesfully Convert project.lock.json from JSON and back to JSON with no
data loss.
There are many references to the areas of code I have updated and I do not believe I can foresee all consequences however I have ran the full test suite locally and have not found any indication these changes are harmful, while I have confirmed that it addresses the issue we are seeing.
I would also at least like to submit the above two cases as tests for this issue, but I am uncertain where those tests should go.