Runtime: Option to support trailing commas with JsonReader

[ahsonkhan]

@JamesNK commented on Mon Oct 29 2018

Comments and trailing commas are the most common differences from the JSON spec found in configuration files.

Consider adding an option to allow JsonReader to support trailing comments in JSON arrays and objects.

{
  "myproperty": true,
}

[
  "first",
  "second",
  "third",
]

---

@danmosemsft commented on Wed Nov 07 2018

Comments above presumably should be commas

[ahsonkhan]

Currently, we have comment handling enum as part of the JsonReaderOption.
The request is to add support for trailing commas as another option given it is invalid according to JSON spec. Json.NET supports reading JSON with trailing commas by default.

```C#
public partial struct JsonReaderOptions
{
public System.Text.Json.JsonCommentHandling CommentHandling { get { throw null; } set { } }

    // Add:
    public bool AllowTrailingCommas { get { throw null; } set { } } // default = false
}

```

Question:

• Keep it as a bool or change it to an enum with two values?
• Are we fine with the name?

~Deferring to future for now since this change could be quite involved (it breaks the error conditions).~

cc @JamesNK, @GrabYourPitchforks, @steveharter, @bartonjs, @KrzysztofCwalina, @eerhardt

[JamesNK]

Two options is fine. I have never had the situation where someone asked for anything beyond turning them on or off. Up to you guys whether it is an enum or boolean.

I think this is an important option for ASP.NET Core's JSON config files. JSON like this is extremely common in config files:

{
  "UseTls": "false",
  //"LogLevel": "Information"
}

@rynowak @davidfowl

[bartonjs]

I can't think of another mode for commas to be in, so boolean seems fine.

[MgSam]

As this introduces no ambiguity in the parsing, I think it should default to on. Pit of success.

[ahsonkhan]

I think it should default to on

Our objective is to strictly follow the JSON RFC by default and only provide such knobs (where the input deviates from it) as explicit options for the caller to opt-in to.

[ahsonkhan]

Fyi, @wli3, this option has been added.