Powershell: Add `.jsonc` support to `ConvertFrom-Json`

[chriskuech]

"JSON with Comments" with the jsonc file extension is a natively supported file format in VS Code. JSONC adds JS-style comments to JSON files. ConvertFrom-Json currently does not support parsing JSONC strings. JSONC support could be added through either a -Comments switch or by default.

Background

Azure Resource Manager PowerShell and Azure Resource Manager templates are a critical component of the Azure ecosystem; many templates provided by Azure are large and complicated JSON files with over 1000 lines. Azure Resource Manager PowerShell cmdlets currently support JSONC templates, but cannot be used in conjunction with custom PowerShell validation due to the lack of JSONC support in ConvertFrom-Json

[iSazonov]

/cc @markekraus

[markekraus]

@chriskuech Can you provide some example JSON, example PowerShell code, what you expect, and what you currently see?

NewtonSoft supports working with JSON comments but has 2 modes: ignore and load. Though, with how the cmdlets currently operate, it may not be possible to make use of that.

[chriskuech]

Working base case

"{`"hello`": `"world`", `n`"a`": 2}" | ConvertFrom-Json

Single-line comment

"{`"hello`": `"world`", // a common example `n`"a`": 2}" | ConvertFrom-Json

ConvertFrom-Json : Invalid object passed in, ':' or '}' expected. (20): {"hello": "world", // a common example
"a": 2}
At line:1 char:60
+ ... llo`": `"world`", // a common example `n`"a`": 2}" | ConvertFrom-Json
+                                                          ~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [ConvertFrom-Json], ArgumentException
    + FullyQualifiedErrorId : System.ArgumentException,Microsoft.PowerShell.Commands.ConvertFromJsonCommand



md5-0a67afc4c6d655569cff2653361c8485

ConvertFrom-Json : Invalid object passed in, ':' or '}' expected. (20): {"hello": "world", /* comment test */
"a": 2}
At line:1 char:59

• ... ello":"world", /* comment test */n"a": 2}" | ConvertFrom-Json
• ~~~~
  
  
  
  • CategoryInfo : NotSpecified: (:) [ConvertFrom-Json], ArgumentException
  
  
  • FullyQualifiedErrorId : System.ArgumentException,Microsoft.PowerShell.Commands.ConvertFromJsonCommand
  
  

### Expectation
Comments are ignored and only the value is returned

hello a

---

world 2
```

[johlju]

This would be a great addition to an already well used cmdlet, able to import a .jsonc file and convert it to an object as usually.

'{
    // Comment
    "property1": "value1"
}' | ConvertFrom-Json

To use ConvertFrom-Json now, the file first needs to loaded and then be stripped from comments.

[markekraus]

One argument against supporting JSON comments is that they are not RFC defined. That makes it difficult to write code that supports something that is not well defined. The general guidance is to not use comments in JSON that are not valid JSON. Instead, adding a metadata object or a "_comment" property is recommended.

If we did support this, we would not be able to include it in the default operation of ConvertFrom-Json until comments for JSON were RFC-backed. It would require a Switch of some kind. -SkipComments or something. That is because we need to reserve the default operation of the cmdlet for RFC compliance. If a later RFC implements comments as beginning with #, for example.

Also, what do we do with the comment data? Nothing? Apply it as a property on the object it comments?

[iSazonov]

Maybe delegate this to Newton.Json? Seem the library is a standard by default.

[johlju]

@markekraus I would say make the use of comments opt-in. Default is today, it will fail parsing the file. If using -AllowComments then it can parse the file (using non-RFC backed way). It would simplify when it is RFC backed. Then the -AllowComments would be obsolete, and the default is to be able to parse both.

I suggest we do nothing with comments, the comments are comments for the jsonc file, not part of the object or data, unless and RFC say differently in the future.

[markekraus]

@iSazonov Delegating to Newtonsoft is a technical implementation, I'm more concerned about design at this point. I think supporting jsonc probably deserves a PowerShell-RFC.

[chriskuech]

For the use case I mentioned, using metadata objects and "_comment" properties would not suffice, as it would break conformity with the external schema.

[markekraus]

@chriskuech I understand. I'm just pointing out that JSONC is not an RFC backed spec and as such the general guidance is to either not use it or to add JSON compliant comments and metadata where possible. And I'm only establishing that as a reason why it should not be included in the default behavior of the cmdlet.

[erictorbenson]

I think @johlju has the right idea about adding an "-AllowComments" switch. That way the cmdlet defaults to RFC-compliant behavior...unless someone wants to write a new RFC? :-)

One very important argument in favor of supporting comments is the Visual Studio "Azure Resource Group" sample project. The PowerShell wrapper that uploads the artifacts to Azure Storage and executes the deployment fails to run when one of the templates or parameters has a comment in it. I stripped out all the comments from the file manually and everything was fine -- with them there, the error that @chriskuech received is what stops the deployment. If I had an "-AllowComments" switch, I could just modify the wrapper script and there would be no problem.

ARM templates can become very confusing and stretch to thousands of lines, and not everyone is 100% dedicated to automated pipeline-based deployment yet. Putting a set of comments in to clearly mark what needs to be modified by a human reduces the chance for error.

[joeyaiello]

Yeah, this seems very useful to me. -AllowComments is probably the way to go (for the reasons @markekraus pointed out).

[TravisEz13]

Comments are allowed by default

'"test" /*test*/' | convertfrom-json

test

'"test" //test' | convertfrom-json

test

Name                           Value
----                           -----
PSVersion                      6.1.3
PSEdition                      Core
GitCommitId                    6.1.3
OS                             Darwin 18.2.0 Darwin Kernel Version 18.2.0: Thu Dec 20 20:46:53 PST 2018; root:xnu-4903.241.1~1/RE...
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[adityapatwardhan]

I have validated on PowerShell Core 6.1.3 on Windows:

Single line comment:

PS> "{`"hello`": `"world`", // a common example `n`"a`": 2}" | ConvertFrom-Json                                                                                                                                                              
hello a
----- -
world 2

Multi-line comment:

PS> "{`"hello`": `"world`", /* comment test */ `n`"a`": 2}" | ConvertFrom-Json                                                                                                                                                               
hello a
----- -
world 2

PS> $PSVersionTable                                                                                                                                                                                                                          
Name                           Value
----                           -----
PSVersion                      6.1.3
PSEdition                      Core
GitCommitId                    6.1.3
OS                             Microsoft Windows 10.0.18845
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[markekraus]

awesome. It has probably been like this since 6.0.0 and we just never validated the premise that they didn't work in Core.

[TravisEz13]

Verified with @chriskuech the question was answered.

[msftrncs]

@SteveL-MSFT, I think the tag 7.0-consider is no longer needed. Seems this is already implemented.