Skip to content

Coerce empty string to null #381

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Coerce empty string to null #381

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
65f58f3
coerce empty string to null
shmax Mar 11, 2017
62f37a8
style
shmax Mar 11, 2017
9ccdc11
add explicit tests
shmax Mar 11, 2017
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
19 changes: 19 additions & 0 deletions src/JsonSchema/Constraints/TypeConstraint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ protected function validateType(&$value, $type)
}

if ('null' === $type) {
if ($coerce) {
$value = $this->toNull($value);
}
return is_null($value);
Copy link
Contributor

@erayd erayd Mar 11, 2017

Choose a reason for hiding this comment

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

Need a newline above return is_null($value); to make the style checker happy.

}

Expand All @@ -234,6 +237,22 @@ protected function toBoolean($value)
return $value;
}

/**
* Converts a value to null. For example, "" becomes null.
*
* @param $value The value to convert to null
*
* @return null|mixed
*/
protected function toNull($value)
{
if ($value === '') {
return null;
}

return $value;
}

/**
* Converts a numeric string to a number. For example, "4" becomes 4.
*
Expand Down
11 changes: 11 additions & 0 deletions tests/Constraints/BasicTypesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ public function getInvalidTests()
"additionalProperties":false
}'
),
array(
'{
"null":""
}',
'{
"type":"object",
"properties": {
"null":{"type":"null"}
}
}'
),
array(
'{
"null":1
Expand Down
2 changes: 1 addition & 1 deletion tests/Constraints/CoerciveTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public function getValidCoerceTests()
"boolean":"true",
"object":{},
"array":[],
"null":null,
"null":"",
Copy link
Contributor

@erayd erayd Mar 11, 2017

Choose a reason for hiding this comment

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

I notice that you have lots of explicit tests in testValidCoerceCases, in addition to simply requiring the schema to validate, but that null coercion hasn't been added as one of them. Is there a reason for that?

Copy link
Collaborator Author

@shmax shmax Mar 11, 2017

Choose a reason for hiding this comment

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

I don't understand. Can you provide an example?

I believe I originally created this file by duplicating BasicTypesTest.

Copy link
Collaborator Author

@shmax shmax Mar 11, 2017

Choose a reason for hiding this comment

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

Ohhh, I think I get what you mean. One sec...

Copy link
Collaborator Author

@shmax shmax Mar 11, 2017

Choose a reason for hiding this comment

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

Added a post-validate test for null.

I also removed what looked like a duplicated block of explicit tests in the same section of code. Not sure if there was any purpose for it...

"any": "string",
"allOf": "1",
"multitype1": "false",
Expand Down