Skip to content

Why does parse_to_value() return a Result of an Option? #52

@polarathene

Description

@polarathene

pub fn parse_to_value<'a>(text: &'a str, options: &ParseOptions) -> Result<Option<JsonValue<'a>>, ParseError> {

When originally implemented in May 2020 (as-is still currently the case), the AST value is acquired as an Option which then transforms the inner value via a .map() call + separate match method:

Ok(value.map(handle_value))

Is there a valid use-case scenario where one would expect this method to return None? I can see that it is coming from under the hood from parsing the AST with related tests for this expectation:

#[test]
fn it_should_not_include_tokens_by_default() {
let result = parse_to_ast("{}", &Default::default(), &Default::default()).unwrap();
assert!(result.tokens.is_none());
}
#[test]
fn it_should_include_tokens_when_specified() {
let result = parse_to_ast(
"{}",
&CollectOptions {
tokens: true,
..Default::default()
},
&Default::default(),
)
.unwrap();
let tokens = result.tokens.unwrap();
assert_eq!(tokens.len(), 2);
}

However for these other parsing methods like parse_to_value() there is no CollectOptions config which probably doesn't make much sense to have as a value of JsonValue is expected, otherwise an error?

So when would None be relevant in this context to warrant the result wrapping JsonValue in an option? Or was this an accident in hindsight? (similar to the related parse_to_serde_value() that was implemented in April 2021)


I only ask as most other format parsing crates I've come across so far (at least those returning a serde value) return a result with the value only. I understand it'd be a breaking change to resolve for jsonc-parser, just curious if this decision is intentional and serves a purpose.

#[test]
fn it_should_parse_empty() {
let value = parse_to_value("", &Default::default()).unwrap();
assert!(value.is_none());
}


For reference, the equivalent test for serde-json (tested methods uses from_str() and from_value()) parses "" into an empty String (and also it's associated Value variant).

EDIT: The serde-json test referenced is not the equivalent input("" vs "\"\""), my mistake. Parsing an empty string "" with serde-json instead emits an error similar to this failure test.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions