Skip to content
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

Remove implicit null skip from Hash to JSON serialization #7053

Conversation

makenowjust
Copy link
Contributor

@makenowjust makenowjust commented Nov 9, 2018

Fixed #7052

I think Hash(String, String).from_str(%{{"foo": null}}) should be failed because String does not contain nil and JSON null is just nil. In fact Rust (serde) and Swift does not allow this.

Rust:

#[macro_use]
extern crate serde_derive;

extern crate serde;
extern crate serde_json;

use std::collections::HashMap;

fn main() {
    let _: HashMap<String, String> = serde_json::from_str(r#"{"foo": null}"#).unwrap();
}
// Error("invalid type: null, expected a string", line: 1, column: 12)

Swift:

import Foundation

let decoder: JSONDecoder = JSONDecoder()
let _: [String: String] = try! decoder.decode(Dictionary<String, String>.self, from: """
  {"foo": null}
""".data(using: .utf8)!)
// Swift.DecodingError.valueNotFound(Swift.String, Swift.DecodingError.Context(codingPath: [_DictionaryCodingKey(stringValue: "foo", intValue: nil)], debugDescription: "Expected String but found null value instead.", underlyingError: nil))

@straight-shoota
Copy link
Member

Why not leave the spec expecting an exception?

@makenowjust
Copy link
Contributor Author

@straight-shoota Good! Added.

@bcardiff bcardiff added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib labels Nov 9, 2018
Copy link
Member

@sdogruyol sdogruyol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @makenowjust 👍

@sdogruyol sdogruyol merged commit 3df580d into crystal-lang:master Nov 9, 2018
@sdogruyol sdogruyol added this to the 0.27.1 milestone Nov 9, 2018
@makenowjust makenowjust deleted the fix/7052-hash-from-json-not-skip-null branch November 9, 2018 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:collection
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants