fix: collapseWithKeys on empty collection #829
Conversation
|
Caution Review failedThe pull request is closed. """ 概述步骤说明在 变更
诗歌
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/macros/src/CollectionMixin.php (2)
30-32: 建议使用更明确的空值检查当前的实现是正确的，但可以使用更明确的空值检查方法来提高代码的可读性。
建议修改为：
- if (! $this->items) { + if (empty($this->items)) { return new static(); }这样可以：
- 更明确地表达检查空集合的意图
- 与 PHP 集合操作的惯用法保持一致
- 提高代码的可维护性
🧰 Tools
🪛 GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
[failure] 30-30:
Access to an undefined property FriendsOfHyperf\Macros\CollectionMixin::$items.
Line range hint
26-47: 建议增加返回值类型声明为了提高代码的类型安全性和可维护性，建议添加方法的返回值类型声明。
建议修改为：
- public function collapseWithKeys() + public function collapseWithKeys(): \Closure这样可以：
- 明确方法返回类型
- 提供更好的 IDE 支持
- 增强代码的可维护性
🧰 Tools
🪛 GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
[failure] 30-30:
Access to an undefined property FriendsOfHyperf\Macros\CollectionMixin::$items.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/macros/src/CollectionMixin.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
src/macros/src/CollectionMixin.php
[failure] 30-30:
Access to an undefined property FriendsOfHyperf\Macros\CollectionMixin::$items.
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/Macros/CollectionTest.php (1)
40-43: 建议添加更多边界测试用例当前测试很好地覆盖了空集合的场景，建议考虑添加以下测试场景以提高代码的健壮性：
- 包含 null 值的集合
- 包含 false 或空值的集合
示例代码：
test('test collapseWithKeysOnCollectionWithNullValues', function ($collection) { $data = new $collection([null, [1 => 'a'], null]); $this->assertEquals([1 => 'a'], $data->collapseWithKeys()->all()); })->with('collectionClassProvider'); test('test collapseWithKeysOnCollectionWithEmptyValues', function ($collection) { $data = new $collection([[], false, [2 => 'b'], '']); $this->assertEquals([2 => 'b'], $data->collapseWithKeys()->all()); })->with('collectionClassProvider');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Macros/CollectionTest.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.6
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.0
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.6
🔇 Additional comments (2)
tests/Macros/CollectionTest.php (2)
40-43: 测试用例设计合理！测试用例很好地验证了空集合的处理逻辑，并且与文件中的其他测试保持一致的风格。
40-43: 实现符合最佳实践测试实现遵循了良好的实践：
- 命名清晰明确
- 职责单一
- 与现有测试保持一致
- 使用数据供应器测试不同集合类型
Summary by CodeRabbit
collapseWithKeys方法中添加了对空项目集合的安全检查，防止在未初始化项目时出现潜在的处理错误。test collapseWithKeysOnEmptyCollection，确保collapseWithKeys方法在空集合上正确处理。