Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[JsonPath] Make the component RFC compliant#61132

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
alexandre-daubois wants to merge2 commits intosymfony:7.3
base:7.3
Choose a base branch
Loading
fromalexandre-daubois:jp-compliance

Conversation

alexandre-daubois
Copy link
Member

@alexandre-dauboisalexandre-daubois commentedJul 16, 2025
edited
Loading

QA
Branch?7.3
Bug fix?yes
New feature?no
Deprecations?no
Issues-
LicenseMIT

This PR makes the JsonPath component RFC compliant by removing all skipped tests of the compliance test suite.

*/
private function isJsonArrayStructure(mixed $value): bool
{
return \is_array($value) && array_is_list($value) && (1 !== \count($value) || !isset($value[0]));
Copy link
Member

Choose a reason for hiding this comment

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

why wouldn't['foo'] be a JSON array structure ?

Copy link
MemberAuthor

@alexandre-dauboisalexandre-dauboisJul 17, 2025
edited
Loading

Choose a reason for hiding this comment

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

This method handles an edge case: when a json like[{"0": 5}] is decoded to PHP, accessing@[0] on the inner object should return nothing, because numeric index selectors only work on arrays, not objects per the RFC. So just callingarray_is_list() would incorrectly allow it. The|| part of the condition specifically excludes single-element arrays with index 0 set to handle this distinction.

Does this answer your question?

Copy link
Member

Choose a reason for hiding this comment

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

but this ignores 2 cases:

  • AFAICT, it breaks the case of accessing single-element arrays
  • you could totally have JSON objects with multiple numeric keys ({"0": 5, "1": 12}), which would be considered array structure in your method.

Copy link
MemberAuthor

@alexandre-dauboisalexandre-dauboisJul 17, 2025
edited
Loading

Choose a reason for hiding this comment

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

It seems pretty complex to fix... Just in case,@mtarld, is there something in JsonStreamer that could also help here? We would need to "easily" differentiate an array from an object with numeric indices. MaybeRead/Lexer.php?

Copy link
Member

Choose a reason for hiding this comment

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

For the case of strings usingjson_decode, we might decode objects intostdClass to distinguish objects from arrays. But this suffers from other limitations as this won't support object keys that are not valid names of PHP properties.

Copy link
MemberAuthor

@alexandre-dauboisalexandre-dauboisJul 17, 2025
edited
Loading

Choose a reason for hiding this comment

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

Tried this approach in54281cb and succeeded on making the CI green. It makes things a bit more complicated but we can easily differentiate objects and arrays now. Also updated the compliance test suite because the specific cases we were dealing with suffered from the same problem when being decoded fromcts.json.

this suffers from other limitations as this won't support object keys that are not valid names of PHP properties

Do you have an example? It seems to work pretty well, and as far as I know, stdClass are actually "allowed" to use wrong property names likehttps://3v4l.org/ede7M#vnull.

@alexandre-dauboisalexandre-dauboisforce-pushed thejp-compliance branch 2 times, most recently fromd1364da to99ff096CompareJuly 17, 2025 10:00
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stloydstloydstloyd left review comments

@stofstofstof left review comments

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

4 participants
@alexandre-daubois@stloyd@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp