Skip to content

Conversation

@huangdijia
Copy link
Member

Summary

This PR adds a new \hasAll\ method to the \Hyperf\Collection\Arr\ class that determines if all given keys exist in an array using \dot\ notation.

Changes

  • Added \hasAll\ method to \src/collection/src/Arr.php\
  • Added comprehensive tests for the new method in \src/collection/tests/ArrTest.php\
  • Method follows the same pattern as existing \hasAny\ method for consistency

Usage

\\

- Added hasAll method to determine if all given keys exist in an array using dot notation
- Added comprehensive tests for the new hasAll method
- Method follows same pattern as existing hasAny method for consistency
@huangdijia
Copy link
Member Author

代码评审:Added hasAll method to Arr class(#7502

总体结论

  • 新增 Arr::hasAll 方法实现清晰、与现有 hasAny 风格一致,并配套了覆盖面良好的单元测试与变更日志,整体质量较高,易于评审与合入。
  • 建议在少数边界与一致性细节上做轻微打磨(见下),以提升鲁棒性与与现有 API 的一致预期。

优点

  • 一致性:方法签名、空值处理、循环结构与 hasAny 对齐,便于理解与维护。
  • 语义明确:精准表达“全部键存在”的需求,调用处无需再手写循环。
  • 点号支持:复用 static::has,自动支持嵌套/点号路径。
  • 测试覆盖:包含空数组、空 keys、null keys、嵌套键等典型边界。

潜在问题

  • 数组空判断的适配性:if (! $array) return false; 对数组是可读的,但当 $arrayArrayAccess 实例时,对象在 PHP 中通常为 truthy,此判断不会生效;且该早退并非必要,因为逐键检查本身可给出准确结果。为避免语义混淆与类型差异,建议移除此分支或仅在确认为 array 时执行。
  • 空 keys 语义:当前对空数组 keys 返回 false(与很多实现一致,如 Laravel 的 Arr::has 也如此)。但“vacuous truth(空集全满足为真)”是常见数学语义。建议在 PR 描述或方法注释中强调该设计选择,并与现有 has/hasAny 语义保持一致。
  • ArrayAccess 覆盖:测试未包含 ArrayAccess(如 ArrayObject)场景。实现应可工作,但建议增加一个断言,确保未来改动不破坏此契约。
  • 数字键与混合键:测试集中在字符串与点号路径。可补充对纯数字键、混合键(如 0, '0')的覆盖,以防与 static::has 的实现边界交互出现意外。

改进建议

  • 精简早退逻辑:
    • 可移除 if (! $array) return false;,完全依赖循环与 static::has 也能保持正确性且更统一;
    • 或仅在确认为数组时检查:if (is_array($array) && $array === []) return false;
  • 文档注释:在方法注释中补充对空 keys 返回 false 的说明,减少调用方歧义。
  • 命名与定位:若项目中 Arr::has 已表示“全部存在”,可在文档中说明 hasAll 的增值点(例如语义更显式),或给出推荐使用场景,避免 API 重叠导致困惑。

测试建议

  • ArrayAccess
    • $arr = new \ArrayObject(['a' => 1, 'b' => ['c' => 2]]); $this->assertTrue(Arr::hasAll($arr, ['a', 'b.c']));
  • 数字键:
    • $arr = ['list' => [10, 20]]; $this->assertTrue(Arr::hasAll($arr, 'list.0'));
  • 单键为整数:
    • $arr = [0 => 'x']; $this->assertTrue(Arr::hasAll($arr, 0));

合并前检查

  • 代码风格:与现有风格一致,无明显问题。
  • 回归风险:方法新增、对既有代码零侵入;测试覆盖主要路径,风险较低。
  • 文档同步:Changelog 已更新;若有 API 文档/指南,建议补充示例与空 keys 语义说明。

如需,我可以基于上述建议提交一个跟进 PR,补充 ArrayAccess 与数字键测试,并调整(或限定)数组空判断的早退分支,以保持实现更稳健与一致。

@limingxinleo limingxinleo changed the title Added hasAll method to Arr class Added method Hyperf\Collection\Arr::hasAll(). Sep 3, 2025
@limingxinleo limingxinleo merged commit 1ea268b into hyperf:master Sep 3, 2025
70 of 71 checks passed
@huangdijia huangdijia deleted the feature/add-hasall-method-to-arr branch September 3, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants