TypeConstraint: validateType fails with stringable objects.

[kb05]

Branch: 5.2.10
File : /src/JsonSchema/Constraints/TypeConstraint.php
Line : 203

Hi.

First of all this problem seems to be solved in master but not in the published versions of packgist.

I have a situation like this:

$schema = [
    'type'                 => 'object',
    'additionalProperties' => false,
    'patternProperties'    => [
        '^.*$' => [
            'type' => 'string',
        ],
    ],
];

$params = [
    'options'   => [
        'app_oid'     => 3,
    ],
];

I have a problem when I use the validator in coerce mode, the validator doesn't cast the "app_oid" into a integer, instead gives an error, this is caused by this line:

In "master" branch this problem has been resolved in this way:

What is strange to me is that the problem was solved two years ago in the master branch, but the branch 5.2.10 it was published on 2020-05-27 (six month ago) and does not have this change.

[erayd]

This decision was deliberate, but I don't recall the exact reasoning. I think we were trying not to change something that would affect the result of validation on existing schemas, or possibly for parity with another validator.

@shmax Do you remember why we made this decision?

[erayd]

@kb05 For clarity - the 5.x.x branch is the current stable branch, and is what the releases are based on. It no longer has feature updates, but does have buxfixes backported from the master branch where that can be done without changing API behavior.

The master branch is intended for v6, but has not yet been released. It is, however, generally stable, so if there's something in it that you need, which isn't in v5, you're welcome to go ahead and use it. We don't merge anything into master that isn't tested.

[erayd]

#368 (Backports for 5.2.0) is when this was considered, and deliberately not backported.

[kb05]

Hi @erayd.

Thank you for your quick response.

I understand then that the PR 384 will be rejected to be merged in 5.x.x branches because it has some "api changes" like "early coerce mode", my doubt is, if someone would separate the "value to string coerce problem" (that could be considered a bug) in a separate pull request, you would consider including it in a 5.x.x version?

[erayd]

I would consider merging a PR against the 5.x.x branch that fixed just this bug, provided it also included a new unit test to validate the behavior. The substance of that test could likely just be pulled straight out of the PR which resolved this in master.

The one reservation I have about this, is I recall having an extensive discussion with @shmax regarding the coercion matrix, ajv's coercion behavior, and a concern about breaking some expected validation results if we changed some aspects of the coercion behavior in a minor version. I'd quite like to wait a bit for his input before proceeding with a fix.