-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
std.json: Adds jsonParse to override parsing behavior #8791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1708,6 +1708,9 @@ fn parseInternal(comptime T: type, token: Token, tokens: *TokenStream, options: | |
} | ||
|
||
pub fn parse(comptime T: type, tokens: *TokenStream, options: ParseOptions) !T { | ||
if (comptime std.meta.trait.hasFn("jsonParse")(T)) { | ||
return T.jsonParse(tokens, options); | ||
} | ||
const token = (try tokens.next()) orelse return error.UnexpectedEndOfJson; | ||
return parseInternal(T, token, tokens, options); | ||
} | ||
|
@@ -2014,12 +2017,12 @@ test "parse into struct with duplicate field" { | |
const ballast = try testing.allocator.alloc(u64, 1); | ||
defer testing.allocator.free(ballast); | ||
|
||
const options_first = ParseOptions{ | ||
const options_first = ParseOptions{ | ||
.allocator = testing.allocator, | ||
.duplicate_field_behavior = .UseFirst | ||
.duplicate_field_behavior = .UseFirst, | ||
}; | ||
|
||
const options_last = ParseOptions{ | ||
const options_last = ParseOptions{ | ||
.allocator = testing.allocator, | ||
.duplicate_field_behavior = .UseLast, | ||
}; | ||
|
@@ -2043,6 +2046,25 @@ test "parse into struct with duplicate field" { | |
try testing.expectError(error.UnexpectedValue, parse(T3, &TokenStream.init(str), options_last)); | ||
} | ||
|
||
test "parse struct with custom parser" { | ||
const T = struct { | ||
const Self = @This(); | ||
a: bool, | ||
b: u64, | ||
|
||
pub fn jsonParse(tokens: *TokenStream, options: ParseOptions) !Self { | ||
return Self{ | ||
.a = true, | ||
.b = try parse(u64, tokens, options), | ||
}; | ||
} | ||
}; | ||
|
||
const r = try parse(T, &TokenStream.init("42"), .{}); | ||
try testing.expect(r.a); | ||
try testing.expectEqual(@as(u64, 42), r.b); | ||
} | ||
|
||
/// A non-stream JSON parser which constructs a tree of Value's. | ||
pub const Parser = struct { | ||
allocator: *Allocator, | ||
|
@@ -2627,6 +2649,9 @@ pub fn stringify( | |
out_stream: anytype, | ||
) @TypeOf(out_stream).Error!void { | ||
const T = @TypeOf(value); | ||
if (comptime std.meta.trait.hasFn("jsonStringify")(T)) { | ||
return value.jsonStringify(options, out_stream); | ||
} | ||
switch (@typeInfo(T)) { | ||
.Float, .ComptimeFloat => { | ||
return std.fmt.formatFloatScientific(value, std.fmt.FormatOptions{}, out_stream); | ||
|
@@ -2648,17 +2673,9 @@ pub fn stringify( | |
} | ||
Bxil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
.Enum => { | ||
if (comptime std.meta.trait.hasFn("jsonStringify")(T)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. only enum/union/struct can have user traits; it makes more sense to keep them here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is exactly why I found it reasonable to move it outside: it stops this check from getting repeated in the code without altering behavior. Since I applied the same reasoning to |
||
return value.jsonStringify(options, out_stream); | ||
} | ||
|
||
@compileError("Unable to stringify enum '" ++ @typeName(T) ++ "'"); | ||
}, | ||
.Union => { | ||
if (comptime std.meta.trait.hasFn("jsonStringify")(T)) { | ||
return value.jsonStringify(options, out_stream); | ||
} | ||
|
||
const info = @typeInfo(T).Union; | ||
if (info.tag_type) |UnionTagType| { | ||
inline for (info.fields) |u_field| { | ||
|
@@ -2671,10 +2688,6 @@ pub fn stringify( | |
} | ||
}, | ||
.Struct => |S| { | ||
if (comptime std.meta.trait.hasFn("jsonStringify")(T)) { | ||
return value.jsonStringify(options, out_stream); | ||
} | ||
|
||
try out_stream.writeByte('{'); | ||
comptime var field_output = false; | ||
var child_options = options; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only work at the top level: the internal recursion uses
parseInternal
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, for all examples I tried I somehow never had one where the root did not have
jsonParse
implemented. What's stopping us from simply keeping a boolean onTokenStream
to know if we need the lookahead or not?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have investigated, and the token parser indeed badly needs an overhaul. I couldn't find a simple enough way to get this properly done without a refactor, but I can't do that right now, so for the time being I will close this PR.