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

Change parseJson to perform type validation #168

Merged
merged 1 commit into from
Apr 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2760,24 +2760,28 @@
)(s);
});

//# parseJson :: String -> Maybe Any
//# parseJson :: TypeRep a -> String -> Maybe a
//.
//. Takes a string which may or may not be valid JSON, and returns Just
//. the result of applying `JSON.parse` to the string if valid; Nothing
//. otherwise.
//. Takes a [type representative](#type-representatives) and a string which
//. may or may not be valid JSON, and returns Just the result of applying
//. `JSON.parse` to the string *if* the result is of the specified type
Copy link
Member Author

Choose a reason for hiding this comment

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

What do you think about the cursive if? I thought it's a nice way to break up the long sentence, but I'm not sure about it.

Copy link
Member

Choose a reason for hiding this comment

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

It works for me, but the one thing that's not clear from this description is the handling of invalid strings.

What do you think of this phrasing?

… and returns Just x if the given string is a JSON representation of x, a value of the specified type (according to is); Nothing otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered it, but I think only specifying the latter is good enough, because it implies the first. That combined with the code example makes it pretty clear. I'm worried going into the specifics take away from the ease of reading and clarity.

Copy link
Member

Choose a reason for hiding this comment

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

Okay. I'm happy with your current wording also.

//. (according to [`is`](#is)); Nothing otherwise.
//.
//. ```javascript
//. > S.parseJson('["foo","bar","baz"]')
//. > S.parseJson(Array, '["foo","bar","baz"]')
//. Just(['foo', 'bar', 'baz'])
//.
//. > S.parseJson('[')
//. > S.parseJson(Array, '[')
//. Nothing()
//.
//. > S.parseJson(Object, '["foo","bar","baz"]')
//. Nothing()
//. ```
S.parseJson =
def('parseJson',
{},
[$.String, $Maybe($.Any)],
encase(JSON.parse));
[TypeRep, $.String, $Maybe(a)],
function(type, s) { return filter(is(type), encase(JSON.parse, s)); });

//. ### RegExp

Expand Down
32 changes: 24 additions & 8 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5125,31 +5125,47 @@ describe('parse', function() {

describe('parseJson', function() {

it('is a unary function', function() {
it('is a binary function', function() {
eq(typeof S.parseJson, 'function');
eq(S.parseJson.length, 1);
eq(S.parseJson.length, 2);
});

it('type checks its arguments', function() {
throws(function() { S.parseJson([1, 2, 3]); },
throws(function() { S.parseJson('String'); },
errorEq(TypeError,
'Invalid value\n' +
'\n' +
'parseJson :: String -> Maybe Any\n' +
' ^^^^^^\n' +
' 1\n' +
'parseJson :: TypeRep -> String -> Maybe a\n' +
' ^^^^^^^\n' +
' 1\n' +
'\n' +
'1) "String" :: String\n' +
'\n' +
'The value at position 1 is not a member of ‘TypeRep’.\n'));

throws(function() { S.parseJson(Array, [1, 2, 3]); },
Copy link
Member

Choose a reason for hiding this comment

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

Mind adding an empty line between these blocks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

errorEq(TypeError,
'Invalid value\n' +
'\n' +
'parseJson :: TypeRep -> String -> Maybe a\n' +
' ^^^^^^\n' +
' 1\n' +
'\n' +
'1) [1, 2, 3] :: Array Number, Array FiniteNumber, Array NonZeroFiniteNumber, Array Integer, Array ValidNumber\n' +
'\n' +
'The value at position 1 is not a member of ‘String’.\n'));
});

it('returns a Just when applied to a valid JSON string', function() {
eq(S.parseJson('["foo","bar"]'), S.Just(['foo', 'bar']));
eq(S.parseJson(Array, '["foo","bar"]'), S.Just(['foo', 'bar']));
});

it('returns a Nothing when applied to an invalid JSON string', function() {
eq(S.parseJson('[Invalid JSON]'), S.Nothing());
eq(S.parseJson(Object, '[Invalid JSON]'), S.Nothing());
});

it('returns a Nothing when the parsed result is not a member of the given type', function() {
eq(S.parseJson(Array, '{"foo":"bar"}'), S.Nothing());
});

});
Expand Down