Skip to content

Commit a282ff2

Browse files
committed
where, sortBy, indexBy and friends: throw on mixed arrays instead of silently skipping scalars
- row-only methods each did something different when a scalar sat next to rows: most skipped it, sortBy kept it, where/whereNot dropped it from both sides (so where + whereNot didn't add up to the whole collection) - a scalar next to rows means the array was built wrong (wrapped an API response one level too high, or assigned a value onto a result set), so now assertNestedArray() requires every element to be a row and throws naming the bad element: "where(): Expected a nested array of rows, but element 'count' is not a row (int)" - fast: the existing rowsOnly flag makes the check O(1) for result sets; only a stale flag triggers a rescan, which repairs the flag on the way through - empty arrays still pass, DB results can never hit this - removed the per-loop is_array() guards the assert makes dead, updated CHANGELOG/UPGRADING/ai-reference, converted the old skip-behavior tests to expect the throw
1 parent 9fdb29f commit a282ff2

8 files changed

Lines changed: 140 additions & 59 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,15 @@ the docs - IDEs show a strikethrough with the replacement.
110110
- true/false mean 1/0 (true used to match any truthy value, even `'abc'`)
111111

112112
See [UPGRADING.md](UPGRADING.md).
113+
- Row-only methods (`where()`, `whereNot()`, `whereInList()`, `sortBy()`,
114+
`indexBy()`, `groupBy()`, `column()`, `columnAt()`) throw
115+
`InvalidArgumentException` naming the offending element when the array
116+
mixes rows and scalar values, instead of silently skipping the scalars
117+
(`sortBy()` kept them). A scalar next to rows means the array was built
118+
wrong - usually a wrapped API response (`['count' => 5, 'items' => [...]]`)
119+
or a value assigned onto a result set - and skipping it hid the mistake.
120+
Database results and empty arrays are unaffected. See
121+
[UPGRADING.md](UPGRADING.md).
113122
- A missing field stays a SmartNull through the whole chain instead of
114123
becoming an empty SmartString at the first method call. Same output as
115124
before (echoes `""`, `or()` still fires), but chains no longer dead-end:

UPGRADING.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,26 @@ automatically unless your composer.json pins `itools/smartstring` lower.*
133133
>
134134
> Regex: `->(where|whereNot|contains)\([^)]*(null|true|false)\s*\)`
135135
136+
### Row-only methods throw on mixed arrays
137+
138+
> `where()`, `whereNot()`, `whereInList()`, `sortBy()`, `indexBy()`,
139+
> `groupBy()`, `column()`, and `columnAt()` now require every element to be
140+
> a row. An array mixing rows and scalar values throws
141+
> `InvalidArgumentException` naming the element, instead of silently
142+
> skipping the scalars:
143+
>
144+
> ```php
145+
> $data = SmartArrayHtml::new(['count' => 5, 'items' => [['id' => 1]]]);
146+
> $data->where('id', 1); // before: returned 0-1 rows, 'count' silently ignored
147+
> // after: throws "where(): Expected a nested array of
148+
> // rows, but element 'count' is not a row (int)"
149+
> ```
150+
>
151+
> Database results and empty arrays are unaffected - this only fires on
152+
> hand-built arrays that mix shapes. The error usually means the array was
153+
> wrapped one level too high (`->items` was the intended collection) or a
154+
> scalar was assigned onto a result set.
155+
136156
### Silent changes
137157
138158
> - `print_r()` and `var_dump()` show just the array data, like dumping a

docs/ai-reference.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,10 @@ the new collection.
237237

238238
## Filtering and Sorting
239239

240-
All return a new collection; nested-only methods throw
241-
`InvalidArgumentException` on flat arrays and vice versa.
240+
All return a new collection; nested-only methods require every element to be
241+
a row and throw `InvalidArgumentException` on flat arrays or on mixed arrays
242+
where an element is not a row (empty arrays pass); flat-only methods likewise
243+
throw on nested input.
242244

243245
| Method | Behavior |
244246
|------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------|

src/SmartArrayBase.php

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ public function sort(int $flags = SORT_REGULAR): static
591591

592592
/**
593593
* Returns a new SmartArray sorted ascending by the specified field.
594-
* Only works on nested arrays (throws on flat).
594+
* Works on arrays of rows only: throws if the array is flat or any element is not a row.
595595
*
596596
* Rows missing the field sort first: the missing value counts as null for
597597
* ordering only (like MySQL ORDER BY), and rows are returned unchanged.
@@ -616,7 +616,7 @@ public function sortBy(string $field, int $flags = SORT_REGULAR): static
616616
}
617617
$this->warnIfMissing($field);
618618

619-
// sort by field value, treating missing fields as null (?? also covers non-array rows in mixed data)
619+
// sort by field value, treating missing fields as null
620620
$sorted = $this->toArray();
621621
$fieldValues = array_map(fn($row) => $row[$field] ?? null, $sorted);
622622
array_multisort($fieldValues, SORT_ASC, $flags, $sorted);
@@ -665,7 +665,7 @@ public function filter(?callable $callback = null): static
665665

666666
/**
667667
* Returns a new SmartArray containing only elements where a field matches a value.
668-
* Only works on nested arrays (throws on flat).
668+
* Works on arrays of rows only: throws if the array is flat or any element is not a row.
669669
*
670670
* How values match:
671671
* - Numbers match numeric strings: where('id', 5) matches '5', where('price', 1) matches '1.00'
@@ -698,7 +698,7 @@ public function where(array|string $field, mixed $value = null): static
698698
// change one copy, change all four.
699699
$matches = [];
700700
foreach ($this->toArray() as $key => $row) {
701-
if (is_array($row) && !empty($row[$field])) {
701+
if (!empty($row[$field])) {
702702
$matches[$key] = $row;
703703
}
704704
}
@@ -713,7 +713,7 @@ public function where(array|string $field, mixed $value = null): static
713713
// repeated 4x, see the first where() loop for why
714714
$matches = [];
715715
foreach ($this->toArray() as $key => $row) {
716-
if (is_array($row) && array_key_exists($field, $row) && self::valueMatches($row[$field], $value)) {
716+
if (array_key_exists($field, $row) && self::valueMatches($row[$field], $value)) {
717717
$matches[$key] = $row;
718718
}
719719
}
@@ -767,7 +767,7 @@ public function whereNot(string $field, mixed $value = null): static
767767
// repeated 4x, see the first where() loop for why
768768
$matches = [];
769769
foreach ($this->toArray() as $key => $row) {
770-
if (is_array($row) && empty($row[$field])) {
770+
if (empty($row[$field])) {
771771
$matches[$key] = $row;
772772
}
773773
}
@@ -779,7 +779,7 @@ public function whereNot(string $field, mixed $value = null): static
779779
// repeated 4x, see the first where() loop for why
780780
$matches = [];
781781
foreach ($this->toArray() as $key => $row) {
782-
if (is_array($row) && (!array_key_exists($field, $row) || !self::valueMatches($row[$field], $value))) {
782+
if (!array_key_exists($field, $row) || !self::valueMatches($row[$field], $value)) {
783783
$matches[$key] = $row;
784784
}
785785
}
@@ -928,9 +928,6 @@ public function indexBy(string $field): static
928928
// Index by field; rows with a null or missing value index under '' (duplicates: last wins)
929929
$values = [];
930930
foreach ($this->toArray() as $row) {
931-
if (!is_array($row)) {
932-
continue; // scalar rows have no fields to index by
933-
}
934931
$key = $row[$field] ?? '';
935932
$key = is_bool($key) ? (int)$key : (string)$key; // string cast keeps float precision; ints re-key as ints, bools as 1/0
936933
$values[$key] = $row;
@@ -981,9 +978,6 @@ public function groupBy(string $field): static
981978

982979
$values = [];
983980
foreach ($this->toArray() as $row) {
984-
if (!is_array($row)) {
985-
continue; // scalar rows have no fields to group by
986-
}
987981
$key = $row[$field] ?? '';
988982
$key = is_bool($key) ? (int)$key : (string)$key; // string cast keeps float precision; ints re-key as ints, bools as 1/0
989983
$values[$key][] = $row;
@@ -1016,9 +1010,6 @@ public function columnAt(int $index): static
10161010

10171011
$values = [];
10181012
foreach ($this->toArray() as $row) {
1019-
if (!is_array($row)) {
1020-
continue; // scalar rows have no columns to extract
1021-
}
10221013
$count = count($row);
10231014
$rowIndex = ($index < 0) ? $count + $index : $index; // Convert negative indexes to positive
10241015

@@ -1572,17 +1563,35 @@ private function assertFlatArray(): void
15721563
}
15731564

15741565
/**
1575-
* Assert that array has at least one nested array in values.
1566+
* Assert that every element is a row (nested array). Empty arrays pass, so
1567+
* empty result sets flow through row-only methods without error.
15761568
*
1577-
* @throws InvalidArgumentException If the array is flat.
1569+
* Row-only methods can rely on every element being a child SmartArray, so
1570+
* their loops don't need per-row is_array() checks.
1571+
*
1572+
* @throws InvalidArgumentException If the array is flat or contains non-row elements.
15781573
*/
15791574
private function assertNestedArray(): void
15801575
{
1581-
if (!empty($this->data) && $this->isFlat()) {
1582-
$function = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1]['function'];
1583-
$error = "$function(): Expected a nested array, but got a flat array";
1584-
throw new InvalidArgumentException($error);
1576+
// Construction and writes maintain rowsOnly, so result sets pass in O(1)
1577+
if ($this->rowsOnly) {
1578+
return;
1579+
}
1580+
1581+
// rowsOnly false means a scalar was stored at some point, but an unset may
1582+
// have removed it since, so scan to see what's really here
1583+
foreach ($this->data as $key => $value) {
1584+
if (!$value instanceof self) {
1585+
$function = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2)[1]['function'];
1586+
$error = $this->isNested()
1587+
? "$function(): Expected a nested array of rows, but element '$key' is not a row (" . get_debug_type($value) . ")"
1588+
: "$function(): Expected a nested array, but got a flat array";
1589+
throw new InvalidArgumentException($error);
1590+
}
15851591
}
1592+
1593+
// All rows after all: the flag went stale-false after an unset, set it right
1594+
$this->rowsOnly = true;
15861595
}
15871596

15881597
/**
@@ -1591,8 +1600,6 @@ private function assertNestedArray(): void
15911600
* names, so a miss there is almost always a typo. Everywhere else (lookup maps
15921601
* from indexBy()/column(), standalone arrays) keys are data, a miss is a normal
15931602
* no-match, and the access renders blank silently.
1594-
* Skipped for method-argument checks on mixed data (scalar config + array fields)
1595-
* since there's no first row to check against.
15961603
*
15971604
* @param string|int $key The key to check for
15981605
* @param bool $isOffset True for key access ($array->key), false for method args (where, sortBy, etc.)
@@ -1611,7 +1618,7 @@ private function warnIfMissing(string|int $key, bool $isOffset = false): void
16111618
if (!$isOffset) {
16121619
$first = $this->first();
16131620
if (!($first instanceof self)) {
1614-
return; // Non-uniform data (e.g., schemas with scalar config + array fields)
1621+
return; // empty array: first() returns SmartNull, no row to sample
16151622
}
16161623
$target = $first;
16171624
}

tests/Unit/FilterUniqueSortTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,17 @@ public function testSortByOnFlatThrows(string $class): void
221221
$class::new(['a', 'b'])->sortBy('name');
222222
}
223223

224+
#[DataProvider('modeProvider')]
225+
public function testSortByThrowsOnScalarRows(string $class): void
226+
{
227+
$sa = $class::new(['tableName' => 'users', 'fields' => ['a' => 1]]);
228+
229+
$this->expectException(InvalidArgumentException::class);
230+
$this->expectExceptionMessage("sortBy(): Expected a nested array of rows, but element 'tableName' is not a row (string)");
231+
232+
$sa->sortBy('a');
233+
}
234+
224235
#[DataProvider('modeProvider')]
225236
public function testSortByRejectsDirectionConstants(string $class): void
226237
{

tests/Unit/ProjectionTest.php

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,14 @@ public function testPluckNthOnFlatThrows(string $class): void
119119
}
120120

121121
#[DataProvider('modeProvider')]
122-
public function testColumnAtSkipsScalarRows(string $class): void
122+
public function testColumnAtThrowsOnScalarRows(string $class): void
123123
{
124-
// One array value makes the array "nested"; scalar rows have no columns to extract
125124
$sa = $class::new([['a', 'b'], 'scalar', ['c', 'd']]);
126125

127-
$this->assertSame(['a', 'c'], $sa->columnAt(0)->toArray());
126+
$this->expectException(InvalidArgumentException::class);
127+
$this->expectExceptionMessage("columnAt(): Expected a nested array of rows, but element '1' is not a row (string)");
128+
129+
$sa->columnAt(0);
128130
}
129131

130132
//endregion
@@ -240,18 +242,14 @@ public function testIndexByFloatAndBoolKeys(string $class): void
240242
}
241243

242244
#[DataProvider('modeProvider')]
243-
public function testIndexBySkipsScalarRows(string $class): void
245+
public function testIndexByThrowsOnScalarRows(string $class): void
244246
{
245-
// One array value makes the array "nested"; scalar rows have no fields to index by
246247
$sa = $class::new([['id' => 1, 'n' => 'a'], 'scalar', ['id' => 2, 'n' => 'b']]);
247248

248-
[$result, $output] = $this->captureOutput(fn() => $sa->indexBy('id'));
249+
$this->expectException(InvalidArgumentException::class);
250+
$this->expectExceptionMessage("indexBy(): Expected a nested array of rows, but element '1' is not a row (string)");
249251

250-
$this->assertSame([
251-
1 => ['id' => 1, 'n' => 'a'],
252-
2 => ['id' => 2, 'n' => 'b'],
253-
], $result->toArray());
254-
$this->assertSame('', $output);
252+
$sa->indexBy('id');
255253
}
256254

257255
//endregion
@@ -335,15 +333,14 @@ public function testGroupByFloatAndBoolKeys(string $class): void
335333
}
336334

337335
#[DataProvider('modeProvider')]
338-
public function testGroupBySkipsScalarRows(string $class): void
336+
public function testGroupByThrowsOnScalarRows(string $class): void
339337
{
340-
// One array value makes the array "nested"; scalar rows have no fields to group by
341338
$sa = $class::new([['g' => 'a', 'v' => 1], 'scalar', ['g' => 'a', 'v' => 2]]);
342339

343-
[$result, $output] = $this->captureOutput(fn() => $sa->groupBy('g'));
340+
$this->expectException(InvalidArgumentException::class);
341+
$this->expectExceptionMessage("groupBy(): Expected a nested array of rows, but element '1' is not a row (string)");
344342

345-
$this->assertSame(['a' => [['g' => 'a', 'v' => 1], ['g' => 'a', 'v' => 2]]], $result->toArray());
346-
$this->assertSame('', $output);
343+
$sa->groupBy('g');
347344
}
348345

349346
//endregion

tests/Unit/WarningsTest.php

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -188,18 +188,6 @@ public function testArgumentWarningSkippedOnEmptyArray(string $class): void
188188
$this->assertSame('', $output, 'an empty array has no rows to check the field against');
189189
}
190190

191-
#[DataProvider('modeProvider')]
192-
public function testArgumentWarningSkippedWhenFirstElementIsNotARow(string $class): void
193-
{
194-
// Mixed data (scalar config keys alongside array fields) has no first row
195-
// to sample, so the check is skipped rather than reporting a false miss
196-
$mixed = $class::new(['tableName' => 'users', 'fields' => ['a' => 1]]);
197-
198-
[, $output] = $this->captureOutput(fn() => $mixed->sortBy('zzz'));
199-
200-
$this->assertSame('', $output);
201-
}
202-
203191
#[DataProvider('modeProvider')]
204192
public function testMissingFieldWarningIsAlsoSentToErrorHandlers(string $class): void
205193
{

tests/Unit/WhereTest.php

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,19 +142,66 @@ public function testWhereAndWhereNotPartitionForEveryValueType(string $class): v
142142
}
143143

144144
#[DataProvider('modeProvider')]
145-
public function testWhereExcludesRowsMissingFieldAndNonArrayRows(string $class): void
145+
public function testWhereExcludesRowsMissingField(string $class): void
146146
{
147147
$sa = $class::new([
148-
'config' => 'scalar row',
149-
'a' => ['f' => 5],
150-
'b' => ['other' => 5],
148+
'a' => ['f' => 5],
149+
'b' => ['other' => 5],
151150
]);
152151

153152
[$result, ] = $this->captureOutput(fn() => $sa->where('f', 5));
154153

155154
$this->assertSame(['a' => ['f' => 5]], $result->toArray());
156155
}
157156

157+
#[DataProvider('modeProvider')]
158+
public function testWhereThrowsOnScalarRows(string $class): void
159+
{
160+
$sa = $class::new(['config' => 'scalar', 'a' => ['f' => 5]]);
161+
162+
$this->expectException(InvalidArgumentException::class);
163+
$this->expectExceptionMessage("where(): Expected a nested array of rows, but element 'config' is not a row (string)");
164+
165+
$sa->where('f', 5);
166+
}
167+
168+
#[DataProvider('modeProvider')]
169+
public function testWhereNotThrowsOnScalarRows(string $class): void
170+
{
171+
$sa = $class::new(['config' => 'scalar', 'a' => ['f' => 5]]);
172+
173+
$this->expectException(InvalidArgumentException::class);
174+
$this->expectExceptionMessage("whereNot(): Expected a nested array of rows, but element 'config' is not a row (string)");
175+
176+
$sa->whereNot('f', 5);
177+
}
178+
179+
#[DataProvider('modeProvider')]
180+
public function testWhereInListThrowsOnScalarRows(string $class): void
181+
{
182+
$sa = $class::new(['config' => 'scalar', 'a' => ['tags' => "\tred\t"]]);
183+
184+
$this->expectException(InvalidArgumentException::class);
185+
$this->expectExceptionMessage("whereInList(): Expected a nested array of rows, but element 'config' is not a row (string)");
186+
187+
$sa->whereInList('tags', 'red');
188+
}
189+
190+
#[DataProvider('modeProvider')]
191+
public function testWhereWorksAgainAfterScalarRowIsUnset(string $class): void
192+
{
193+
// Storing a scalar marks the array as not rows-only, and unset doesn't clear
194+
// the mark - the assert rescans, proves all remaining elements are rows, and passes
195+
$sa = $class::new(['config' => 'scalar', 'a' => ['f' => 5], 'b' => ['f' => 0]]);
196+
197+
[$result, ] = $this->captureOutput(function () use ($sa) {
198+
unset($sa['config']); // bracket unset echoes a deprecation, not under test here
199+
return $sa->where('f', 5);
200+
});
201+
202+
$this->assertSame(['a' => ['f' => 5]], $result->toArray());
203+
}
204+
158205
#[DataProvider('modeProvider')]
159206
public function testWhereUnwrapsSmartStringValues(string $class): void
160207
{

0 commit comments

Comments
 (0)