Dev #14
Dev #14
Conversation
…and-values Added support for fetching keys and values from array collector
There was a problem hiding this comment.
Pull request overview
This PR adds new functionality to the dataset component by introducing keys() and values() methods to ArrayCollector and expanding scalar key type support in IndexedCollector.
Key Changes:
- Added
keys()andvalues()methods toArrayCollectorfor extracting keys and values from collected arrays - Expanded
IndexedCollectorto support all scalar key types (float, bool, null) instead of just string and int - Added comprehensive test coverage for the new
ArrayCollectormethods
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/RunOpenCode/Component/Dataset/src/Collector/ArrayCollector.php |
Added keys() and values() methods to provide array key/value extraction functionality |
src/RunOpenCode/Component/Dataset/tests/Collector/ArrayCollectorTest.php |
Added test cases for the new keys() and values() methods |
src/RunOpenCode/Component/Dataset/src/Collector/IndexedCollector.php |
Expanded scalar key support from string/int to all scalar types; updated type annotations and added phpstan-ignore comments for scalar array key limitations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| \is_string($offset) || \is_int($offset) => \array_key_exists($offset, $this->scalarIndex), | ||
| \is_scalar($offset) => \array_key_exists($offset, $this->scalarIndex), // @phpstan-ignore-line | ||
| \is_object($offset) => $this->objectIndex->contains($offset), | ||
| default => throw new UnsupportedException('Only object, string and integer keys are supported.'), |
There was a problem hiding this comment.
The error message states "Only object, string and integer keys are supported" but the code now accepts all scalar types (including float and bool). The error message should be updated to reflect the expanded support, such as "Only object and scalar keys are supported" or "Only object, string, integer, float, and boolean keys are supported".
| \is_string($offset) || \is_int($offset) => $this->scalarIndex[$offset], | ||
| \is_scalar($offset) => $this->scalarIndex[$offset], // @phpstan-ignore-line | ||
| \is_object($offset) => $this->objectIndex[$offset], | ||
| default => throw new UnsupportedException('Only object, string and integer keys are supported.'), |
There was a problem hiding this comment.
The error message states "Only object, string and integer keys are supported" but the code now accepts all scalar types (including float and bool). The error message should be updated to reflect the expanded support, such as "Only object and scalar keys are supported" or "Only object, string, integer, float, and boolean keys are supported".
| /** | ||
| * Index of values with keys of scalar type. | ||
| * | ||
| * @var array<TKey, list<TValue>> | ||
| * @var array<scalar, list<TValue>> | ||
| */ | ||
| private array $scalarIndex = []; | ||
|
|
||
| /** | ||
| * Index of values with keys of object type. | ||
| * | ||
| * @var \SplObjectStorage<TKey&object, list<TValue>> | ||
| * @var \SplObjectStorage<object, list<TValue>> |
There was a problem hiding this comment.
The comment "Currently, the allowed key types are scalar values (int, float, string, bool, null) and objects." is inconsistent with the error messages in the code (lines 98, 120, 140) which state "Only object, string and integer keys are supported." Since the code change now supports all scalar types, the error messages should be updated to match this documentation, or vice versa.
| if (\is_scalar($key)) { | ||
| $this->scalarIndex[$key] = $this->scalarIndex[$key] ?? []; // @phpstan-ignore-line | ||
| $this->scalarIndex[$key][] = $value; // @phpstan-ignore-line |
There was a problem hiding this comment.
The IndexedCollector now supports all scalar types (float, bool, null) in addition to string and int, but there are no test cases covering these newly supported key types. Consider adding test cases for float and boolean keys to ensure the expanded scalar support works correctly.
No description provided.