-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat: add Arr::push() method for array manipulation with dot notation #884
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add push method to ArrMixin for pushing values to array paths - Add corresponding output method to Arr class - Add comprehensive tests covering various use cases - Support both dot notation keys and root level pushing
- Add Arr::push() method to ArrMixin for pushing values to arrays using dot notation - Support both nested array paths and root-level array pushing - Include comprehensive test coverage for various scenarios - Handle type validation with clear error messages - Return new array instead of modifying by reference to work with macro system The method allows pushing values to arrays at any depth using dot notation, similar to Arr::set() but specifically for array appending operations.
Walkthrough新增 Arr::push 方法:在 output/Arr.php 中声明了空实现;在 ArrMixin 中提供具体闭包实现,支持 dot 语法向数组及其嵌套数组追加元素,并在非数组目标时报错。测试用例覆盖空键追加、嵌套路径创建与合并、及异常路径。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Arr as Arr::push (macro)
participant Store as Array/Data
Client->>Arr: push(array, key, ...values)
alt key == null
Arr->>Store: append ...values to root array
Arr-->>Client: updated array
else key != null
Arr->>Store: get value at dot(key)
alt value is array or null
Arr->>Store: merge value with ...values
Arr->>Store: set merged at dot(key)
Arr-->>Client: updated array
else value is not array
Arr-->>Client: throw InvalidArgumentException
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Note: Using configuration file /phpstan.neon.dist. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/macros/output/Hyperf/Collection/Arr.php (1)
102-108: 为 IDE stub 添加最小实现与抑制标注,消除未使用参数告警当前方法体为空,静态分析(PHPMD)提示
$array/$key/$values未使用。作为仅供 IDE/静态分析的占位实现,可最小化使用这些参数并返回原数组,或在 PHPDoc 中抑制该规则,避免噪音。建议如下调整:
- 在 PHPDoc 增加
@SuppressWarnings(PHPMD.UnusedFormalParameter)。- 在方法体
unset($key, $values); return $array;,以契合签名且不改变运行期语义(真实实现由 mixin 提供)。可直接应用此补丁:
/** * Push an item into an array using "dot" notation. * @return array + * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public static function push(array $array, string|int|null $key, mixed ...$values) { - } + // Stub for IDE/static analysis; real implementation is provided by mixin at runtime. + unset($key, $values); + return $array; + }src/macros/src/ArrMixin.php (2)
224-229: root 级追加可用 array_push 简化并避免空循环当前对
$key === null的处理用foreach逐个追加。可以用array_push在有值时一次性追加,空参时短路,简洁且避免无用循环:- if ($key === null) { - foreach ($values as $value) { - $array[] = $value; - } - return $array; - } + if ($key === null) { + if ($values !== []) { + array_push($array, ...$values); + } + return $array; + }
231-237: 复用 Arr::array 以减少重复并统一异常信息你在同文件已提供
Arr::array(),其默认值与异常消息格式与此处一致。可直接复用以减少重复与维护成本:- $current = Arr::get($array, $key, []); - - if (! is_array($current)) { - throw new InvalidArgumentException( - sprintf('Array value for key [%s] must be an array, %s found.', $key, gettype($current)) - ); - } + $current = Arr::array($array, $key, []);tests/Macros/ArrTest.php (2)
11-16: 在测试中注册 mixin 的方式可行;可考虑集中到测试引导文件当前在本文件显式
Arr::mixin(new ArrMixin());可工作无误。若后续有更多涉及宏的测试,建议集中到测试引导(如tests/bootstrap.php或已引入的Stubs/Common.php),以避免重复注册与顺序依赖。
250-272: 可补充一个“无值追加为 no-op”的边界用例建议再加一个用例,验证当
...$values为空时数组保持不变(根级与嵌套路径各一例),以固定该行为,防止未来回归。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/macros/output/Hyperf/Collection/Arr.php(1 hunks)src/macros/src/ArrMixin.php(1 hunks)tests/Macros/ArrTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/macros/output/Hyperf/Collection/Arr.php (1)
src/macros/src/ArrMixin.php (2)
push(221-246)array(38-51)
src/macros/src/ArrMixin.php (2)
src/macros/output/Hyperf/Collection/Arr.php (3)
push(106-108)array(34-36)Arr(17-140)src/trigger/src/TriggerManager.php (1)
get(63-66)
tests/Macros/ArrTest.php (2)
src/macros/src/ArrMixin.php (3)
ArrMixin(27-247)array(38-51)push(221-246)src/macros/output/Hyperf/Collection/Arr.php (3)
Arr(17-140)array(34-36)push(106-108)
🪛 PHPMD (2.15.0)
src/macros/output/Hyperf/Collection/Arr.php
106-106: Avoid unused parameters such as '$array'. (Unused Code Rules)
(UnusedFormalParameter)
106-106: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
106-106: Avoid unused parameters such as '$values'. (Unused Code Rules)
(UnusedFormalParameter)
🪛 GitHub Actions: tests
src/macros/src/ArrMixin.php
[error] 229-246: PHP CS Fixer dry-run found fixable issues in ArrMixin.php (rules: phpdoc_no_alias_tag, unary_operator_spaces, not_operator_with_successor_space, header_comment, blank_lines_before_namespace, no_whitespace_in_blank_line). Command 'vendor/bin/php-cs-fixer fix src/macros/src/ArrMixin.php --dry-run --diff --verbose' exited with code 8.
🔇 Additional comments (1)
tests/Macros/ArrTest.php (1)
250-272: Arr::push 覆盖面良好,断言符合预期
- 覆盖了点语法嵌套路径创建与追加
- 覆盖了
key = null根级 push- 覆盖了目标非数组时的异常消息匹配
整体用例清晰、稳定。
|
|
||
| $current = Arr::get($array, $key, []); | ||
|
|
||
| if (!is_array($current)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
修复 not 运算符空格风格(CI 报错),与文件内现有风格保持一致
本文件其他方法均使用 if (! is_...) 的风格;此处少了空格,且 CI 已由 PHP-CS-Fixer 报出可修复问题。建议统一:
- if (!is_array($current)) {
+ if (! is_array($current)) {此外,同一代码块附近(大约行 232、238、244)的空白行包含多余空格,请一并移除以通过 no_whitespace_in_blank_line 规则。修复后再执行一次 vendor/bin/php-cs-fixer fix src/macros/src/ArrMixin.php 确认 CI 通过。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!is_array($current)) { | |
| if (! is_array($current)) { |
🤖 Prompt for AI Agents
In src/macros/src/ArrMixin.php around line 233, change the negation spacing from
`if (!is_array($current))` to match the file's style `if (!
is_array($current))`, and remove trailing spaces on the blank lines near lines
~232, 238, and 244 to satisfy the no_whitespace_in_blank_line rule; after making
these whitespace fixes run vendor/bin/php-cs-fixer fix
src/macros/src/ArrMixin.php to confirm the CI issue is resolved.
This PR adds a new Arr::push() method to the Hyperf Collection Arr class, enabling users to push values to arrays at any depth using dot notation.
Features
Usage Examples
Implementation Details
Testing
All tests pass including:
The implementation has been thoroughly tested and follows the existing codebase patterns.
Summary by CodeRabbit