Correctly cast object to boolean, added more tests

[nickdnk]

Hello

After my issue (#81) I worked out this. I am unsure if the reason you accept strings that are clearly not numbers is simply because you want to mimic PHPs odd handling of numeric strings (i.e. 144 === (int)"144 sf_sdfsdf").

I also fixed a bug related to objects incorrectly converted to boolean (I don't know why anyone would use this cast though). It was inverted based on the documentation.

None of this solves the problem with the data not actually being cast.

[msarca]

Thanks for the PR @nickdnk! There seems to be a problem with the object to boolean conversion, but that's a different issue, please open a separate PR if you want to submit a fix.

Also, the title of this PR is misleading. What you define as being "correct" might not be de same for me. This goes the other way around too. There are some issues with your PR:

• filter_* functions are part of an extension. Since the extension is enabled by default I consider this to be a small issue.
• filter_* functions are configuration dependant. This is a major issue. In some configurations "1.234" will be interpreted as one point two three four while in others will be interpreted as one thousand two hundred thirty-four.

[nickdnk]

Okay, the filter behaving differently does make sense. I did not consider that. But then how do we do proper validation? Is it intended that the string "5notanumber" is supposed to successfully convert to the integer value 5? That seems a little crazy to me.

I'll separate the boolean-object conversion into a separate PR.

Also, can you explain the casts feature (#81), aside from what you already did here? Is it not supposed to cast the values automatically? I mean, is that not at all the goal? If no, then perhaps this should be explicitly stated in the documentation. In https://github.com/justinrainbow/json-schema, which I replaced with opis since it's oudated/unmaintained, type coercion actually converts the values in the input, so I (perhaps incorrectly) assumed that this was the case here as well. I had to manually cast all the values in the places they're being used now. And that's not a big deal, but it was unclear that I needed to do this.

[nickdnk]

When it comes to float conversion, I think the existing "cast everything to float" solution already suffers from the localization-issues you mentioned:

// The first example here could also potentially be wrong; the input could be 1.322 or 1,322, meaning
// two different things depending on your localization. Neither are just 1?
(float)"1,322"; => 1  
(float)"1.322"; => 1.322
(float)1,322; => 1322 // Same problem here, but currently we assume comma is always thousand-separator - again language specific
(float)1.322; => 1.322
(float)"1notafloat"; => 1 // crazy-town
(float)"notafloat"; => 0

// Instead of just assuming the first example here is just 1, we can't validate it as a float at all:
filter_var("1,322", FILTER_VALIDATE_FLOAT); => false
filter_var("1.322", FILTER_VALIDATE_FLOAT); => 1.322
// In computer-world a float never has a comma un-stringed, so this is rightfully false:
// Edit: Obviously this can't even be done, as we're now just passing 322 as the filter parameter and the filter as the options parameter. Scratch this one. Sorry.
// filter_var(1,322, FILTER_VALIDATE_FLOAT); => false 
filter_var(1.322, FILTER_VALIDATE_FLOAT); => 1.322
filter_var("1notafloat", FILTER_VALIDATE_FLOAT); => false
filter_var("notafloat", FILTER_VALIDATE_FLOAT); => false

[msarca]

Okay, the filter behaving differently does make sense. I did not consider that. But then how do we do proper validation? Is it intended that the string "5notanumber" is supposed to successfully convert to the integer value 5? That seems a little crazy to me.

This is not an issue of validation, this is an issue of convention. The term numeric string is just a convention, there are no multiple types of strings. There is only string. If you want to interpret the content of a string, you must establish first a convention. We chose to interpret the string following the PHP rules. You have an issue with that, and I understand why. But you must also understand that what you are proposing is not a better solution, it's just another convention. If I merge this, one month from now, another issue will be opened and someone will complain that instead of following the PHP rules, we made our own and they will ask us to follow a different convention that is more appropriate. This could easily escalate into a never-ending debate.

When it comes to float conversion, I think the existing "cast everything to float" solution already suffers from the localization-issues you mentioned:

No, it does not. There is only one way to write a float in PHP: using the dot notation. Trying to write a number using a comma will result in a fatal syntax error, no matter the configuration.

// Syntax error
$a = 1,234;

[nickdnk]

Hey

What you posted is rightfully a syntax error, but:

(float)"1,322"; => 1  
(float)1,322; => 1322
(float)"1.322"; => 1.322

The second one never comes into play based on the fact that nothing can be assigned as a variable this way (why it does not give a syntax error I don't know), just like in your example, but the first case is clearly wrong, regardless of localization, and the third follows the same pattern as the default configuration for filter_var. The point I was making here was in response to when you wrote:

filter_* functions are configuration dependant. This is a major issue. In some configurations "1.234" will be interpreted as one point two three four while in others will be interpreted as one thousand two hundred thirty-four.

Changing the default behavior of filter_var should probably be at your own peril as a developer. I understand that you want to follow PHP convention, I was just wondering if these things were intentional or by mistake, as I am sure you can see that some of the examples are totally nonsensical. And yes, I know that that is PHP's fault and not yours. I guess I had hoped that validation libraries would be a little stricter in their tolerances than what PHP normally allows. I am also well aware of the complications of changing the convention.

I will extract the bug fix for the object cast and the tests I added that pass using the old validation code.

Cheers.

[nickdnk]

Squashed to only contain object bool-cast fix, more tests and renamed the PR appropriately.