Align array_diff* functions to official PHP behavior#110
Draft
Align array_diff* functions to official PHP behavior#110
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates PH7’s array_diff* family to more closely mirror official PHP behavior, primarily around argument validation/error reporting and diff semantics, and adds PHPT coverage to lock the behavior in.
Changes:
- Add PHPT integration tests asserting PHP-like
ArgumentCountError/TypeErrormessages forarray_diff,array_diff_assoc,array_diff_key, andarray_diff_uassoc. - Add smoke tests for multi-array diff behavior and “return input unchanged” cases.
- Update
src/ph7/hashmap.cimplementations to throw exceptions on invalid arguments and adjust diff logic forarray_diff_assoc/array_diff_uassoc.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/ph7/hashmap.c |
Adds PHP-like argument validation/errors for array diff functions; adjusts diff logic for assoc/uassoc variants. |
tests/ph7/002-integration/function/array_diff/array_diff_missing_args.phpt |
Verifies ArgumentCountError for array_diff() with no args. |
tests/ph7/002-integration/function/array_diff/array_diff_type_error_first.phpt |
Verifies TypeError for non-array arg #1 to array_diff(). |
tests/ph7/002-integration/function/array_diff/array_diff_type_error_second.phpt |
Verifies TypeError for non-array arg #2 to array_diff(). |
tests/ph7/002-integration/function/array_diff_assoc/array_diff_assoc_missing_args.phpt |
Verifies ArgumentCountError for array_diff_assoc() with no args. |
tests/ph7/002-integration/function/array_diff_assoc/array_diff_assoc_type_error_first.phpt |
Verifies TypeError for non-array arg #1 to array_diff_assoc(). |
tests/ph7/002-integration/function/array_diff_assoc/array_diff_assoc_type_error_second.phpt |
Verifies TypeError for non-array arg #2 to array_diff_assoc(). |
tests/ph7/002-integration/function/array_diff_assoc/array_diff_assoc_type_error_third.phpt |
Verifies TypeError for non-array arg #3 to array_diff_assoc(). |
tests/ph7/002-integration/function/array_diff_key/array_diff_key_missing_args.phpt |
Verifies ArgumentCountError for array_diff_key() with no args. |
tests/ph7/002-integration/function/array_diff_key/array_diff_key_type_error_first.phpt |
Verifies TypeError for non-array arg #1 to array_diff_key(). |
tests/ph7/002-integration/function/array_diff_key/array_diff_key_type_error_second.phpt |
Verifies TypeError for non-array arg #2 to array_diff_key(). |
tests/ph7/002-integration/function/array_diff_uassoc/array_diff_uassoc_missing_args.phpt |
Verifies ArgumentCountError for array_diff_uassoc() with no args. |
tests/ph7/002-integration/function/array_diff_uassoc/array_diff_uassoc_type_error_first.phpt |
Verifies TypeError for non-array arg #1 to array_diff_uassoc(). |
tests/ph7/002-integration/function/array_diff_uassoc/array_diff_uassoc_type_error_second.phpt |
Verifies TypeError for non-array middle arg to array_diff_uassoc(). |
tests/ph7/002-integration/function/array_diff_uassoc/array_diff_uassoc_callback_not_array_or_string.phpt |
Verifies callback-related TypeError when 2nd arg isn’t array/string (treated as callback). |
tests/ph7/002-integration/function/array_diff_uassoc/array_diff_uassoc_callback_array_invalid.phpt |
Verifies callback-related TypeError for array callback of wrong shape. |
tests/ph7/002-integration/function/array_diff_uassoc/array_diff_uassoc_callback_not_callable.phpt |
Verifies callback-related TypeError for non-callable callback argument. |
tests/ph7/001-smoke/function/array_diff/array_diff_basic.phpt |
Smoke test for basic array_diff behavior preserving keys. |
tests/ph7/001-smoke/function/array_diff/array_diff_multiple.phpt |
Smoke test for array_diff with more than two arrays. |
tests/ph7/001-smoke/function/array_diff_assoc/array_diff_assoc.phpt |
Refocuses existing smoke test to array_diff_assoc only. |
tests/ph7/001-smoke/function/array_diff_assoc/array_diff_assoc_return_same.phpt |
Smoke test: single-arg array_diff_assoc returns input unchanged. |
tests/ph7/001-smoke/function/array_diff_assoc/array_diff_assoc_value_mismatch.phpt |
Smoke test: same key but different value should be kept. |
tests/ph7/001-smoke/function/array_diff_assoc/array_diff_assoc_numeric_keys.phpt |
Smoke test: handles integer keys correctly. |
tests/ph7/001-smoke/function/array_diff_assoc/array_diff_assoc_multiple_arrays.phpt |
Smoke test: supports diffing against multiple arrays. |
tests/ph7/001-smoke/function/array_diff_key/array_diff_key_return_same.phpt |
Smoke test: single-arg array_diff_key returns input unchanged. |
tests/ph7/001-smoke/function/array_diff_key/array_diff_key_numeric_keys.phpt |
Smoke test: compares integer keys ignoring values. |
tests/ph7/001-smoke/function/array_diff_key/array_diff_key_multiple_arrays.phpt |
Smoke test: supports diffing keys across multiple arrays. |
tests/ph7/001-smoke/function/array_diff_uassoc/array_diff_uassoc_return_same.phpt |
Smoke test: two-arg array_diff_uassoc (array + callback) returns input unchanged. |
tests/ph7/001-smoke/function/array_diff_uassoc/array_diff_uassoc_multiple_arrays.phpt |
Smoke test: array_diff_uassoc with multiple arrays + callback. |
Comment on lines
4099
to
4101
| } | ||
| int keep = 1; | ||
| for( i = 1 ; i < nArg - 1; i++ ){ |
There was a problem hiding this comment.
Same issue as above: int keep = 1; is declared after statements inside the loop body, which is invalid in C89/C90. Declare keep at the beginning of the enclosing block (or introduce a new scope) to preserve C89 compatibility.
Coverage
Details: https://github.com/alganet/PHL/actions/runs/22432386383 |
0897807 to
ad781f4
Compare
ad781f4 to
d24ec15
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.