Skip to content

Commit 2dbc598

Browse files
daveedisdaveedis
committed
review pass: clearer errors, debug fixes, bracket-key parity
- where(['field']) list form throws a clear error suggesting ->where('field') instead of printing nonsense advice and then an internal TypeError - whereInList() rejects array values with a clear error instead of casting to the string "Array" with a PHP warning and matching nothing - bracket reads coerce float and bool offsets like plain PHP arrays ($arr[1.5] reads key 1) instead of an internal TypeError - SmartArrayRaw deprecation notices report the caller's file:line like every other notice, and new() fires one notice instead of two - SmartNull->debug() says "missing key or empty result" instead of dumping an empty array under the wrong class name - SmartNull uses SharedHelpers for xmpWrap() instead of its own copy - debug(1) runs the root-property reformat on the properties block only, so a data row keyed 'root' prints its stored value - debug() only probes load() when a handler is set, instead of throwing and catching an exception per key on plain arrays - UPGRADING: load() throws InvalidArgumentException for invalid field names (was RuntimeException in 2.7)
1 parent 248a58f commit 2dbc598

9 files changed

Lines changed: 137 additions & 35 deletions

‎UPGRADING.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ Full lists of what changed per release: [CHANGELOG.md](CHANGELOG.md).
122122
> (SmartString, SmartArray, SmartNull) - they unwrap and re-wrap for the
123123
> array's mode instead of throwing, so values copy between arrays without
124124
> calling `->value()` first. Only affects code that relied on those throws.
125+
> - `load()` throws `InvalidArgumentException` instead of `RuntimeException`
126+
> when the field name contains invalid characters, matching its empty-field
127+
> check. Only affects code catching `RuntimeException` around `load()`.
125128
126129
## v2.7.0
127130

‎src/Deprecations.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,12 @@ public function offsetSet(mixed $offset, mixed $value): void
428428
*/
429429
public function offsetGet(mixed $offset): static|SmartNull|SmartString|string|int|float|bool|null
430430
{
431-
$offset ??= ''; // PHP array semantics: $arr[null] reads key ''
431+
// PHP array key semantics: $arr[null] reads '', floats truncate ($arr[1.5] reads 1), bools read 1/0
432+
$offset = match (true) {
433+
$offset === null => '',
434+
is_float($offset), is_bool($offset) => (int) $offset,
435+
default => $offset,
436+
};
432437
$this->triggerArrayAccessDeprecation($offset, 'get');
433438
return $this->getElement($offset);
434439
}

‎src/SmartArrayBase.php‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,12 @@ public function where(array|string $field, mixed $value = null): static
538538

539539
// Deprecated: legacy array syntax, use chained ->where('field', value) calls instead
540540
$conditions = array_map([self::class, 'getRawValue'], $field);
541+
foreach ($conditions as $key => $listValue) {
542+
if (is_int($key)) { // a list like where(['featured']) has no field names to match on
543+
$hint = is_string($listValue) ? " Did you mean ->where('$listValue') to match rows where '$listValue' is non-empty?" : "";
544+
throw new InvalidArgumentException("where(): the array form takes ['field' => value] pairs, list given.$hint");
545+
}
546+
}
541547
$whereCalls = array_map(fn($k, $v) => "->where('$k', " . (is_numeric($v) ? $v : "'$v'") . ")", array_keys($conditions), $conditions);
542548
self::logDeprecation("Replace ->where([...]) with " . implode('', $whereCalls));
543549

@@ -614,7 +620,11 @@ public function whereInList(string $field, mixed $value): static
614620
{
615621
$this->assertNestedArray();
616622
$this->warnIfMissing($field);
617-
$value = (string) self::getRawValue($value);
623+
$value = self::getRawValue($value);
624+
if (!is_scalar($value) && $value !== null) {
625+
throw new InvalidArgumentException("whereInList(): expected a single value to match, got " . get_debug_type($value));
626+
}
627+
$value = (string) $value;
618628
$matches = [];
619629
foreach ($this->toArray() as $key => $row) {
620630
if (!isset($row[$field])) {
@@ -1092,8 +1102,9 @@ public function debug(int $debugLevel = 0): void
10921102
$properties = $this->getInternalProperties(); // gets public properties
10931103
$rootShort = self::stripNamespace(get_debug_type($properties['root']));
10941104
$properties['root'] = get_debug_type($properties['root']) . " #" . spl_object_id($properties['root']);
1095-
$output .= self::prettyPrintR($properties, $debugLevel, 0, "Object Properties");
1096-
$output = preg_replace("/^(\s+'root'\s+=> ).*?(\d+).*?$/m", "$1$rootShort #$2", $output); // format root property as: SmartArrayHtml #123
1105+
$propertiesOutput = self::prettyPrintR($properties, $debugLevel, 0, "Object Properties");
1106+
$propertiesOutput = preg_replace("/^(\s+'root'\s+=> ).*?(\d+).*?$/m", "$1$rootShort #$2", $propertiesOutput); // format root property as: SmartArrayHtml #123
1107+
$output .= $propertiesOutput; // regex runs on the properties block only so a data row keyed 'root' prints untouched
10971108
}
10981109

10991110
$output .= "\n";
@@ -1125,16 +1136,19 @@ private static function prettyPrintR(mixed $var, int $debugLevel = 0, int $depth
11251136
$wrappedKey = is_int($key) ? "[$key]" : "'$key'";
11261137
$thisKeyPrefix = str_pad($wrappedKey, $maxKeyLength) . " => ";
11271138

1128-
// add load comment
1139+
// add load comment for keys the handler resolves; without a handler (or for
1140+
// int keys, which load() doesn't take) there is nothing to probe
11291141
$loadComment = "";
1130-
$loadResult = false;
1131-
try {
1132-
$loadResult = $var->load($key);
1133-
} catch (Throwable) {
1134-
// ignore errors
1135-
}
1136-
if ($loadResult !== false && !$loadResult instanceof SmartNull) {
1137-
$loadComment = " // ->load('$key') for more";
1142+
if ($var instanceof self && $var->loadHandler && is_string($key)) {
1143+
$loadResult = false;
1144+
try {
1145+
$loadResult = $var->load($key);
1146+
} catch (Throwable) {
1147+
// ignore errors
1148+
}
1149+
if ($loadResult !== false && !$loadResult instanceof SmartNull) {
1150+
$loadComment = " // ->load('$key') for more";
1151+
}
11381152
}
11391153

11401154
// get output

‎src/SmartArrayRaw.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class SmartArrayRaw extends SmartArray
1313
*/
1414
public function __construct(array $array = [], bool|array|null $properties = [])
1515
{
16-
@trigger_error('SmartArrayRaw is deprecated. Use SmartArray instead.', E_USER_DEPRECATED);
16+
self::logDeprecation('Replace SmartArrayRaw with SmartArray');
1717
parent::__construct($array, $properties);
1818
}
1919

@@ -22,7 +22,7 @@ public function __construct(array $array = [], bool|array|null $properties = [])
2222
*/
2323
public static function new(array $array = [], array|bool $properties = []): static
2424
{
25-
@trigger_error('SmartArrayRaw::new() is deprecated. Use SmartArray::new() instead.', E_USER_DEPRECATED);
25+
// No notice here: the constructor below logs one, so new() stays at one notice per call
2626
return new static($array, $properties);
2727
}
2828
}

‎src/SmartNull.php‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
*/
2424
class SmartNull extends stdClass implements SmartBase, Iterator, ArrayAccess, JsonSerializable, Countable
2525
{
26+
use SharedHelpers;
27+
2628
//region Creation and Conversion
2729

2830
/**
@@ -88,6 +90,24 @@ public function value(): mixed
8890
//endregion
8991
//region Debugging and Help
9092

93+
/**
94+
* Displays diagnostic output. A SmartNull marks a missing key or empty result,
95+
* so there is no data to dump - the output says what this object is instead of
96+
* showing an empty array.
97+
*/
98+
public function debug(): void
99+
{
100+
$class = static::class;
101+
$output = <<<__TEXT__
102+
$class - missing key or empty result, value is null
103+
104+
Property reads and method calls return SmartNull again, so chains keep working.
105+
Check the key name for typos, or test with ->isNotEmpty() before use.
106+
__TEXT__;
107+
108+
echo self::xmpWrap("\n$output\n\n");
109+
}
110+
91111
/**
92112
* Prints links to the online documentation.
93113
*
@@ -102,19 +122,7 @@ public function help(): void
102122
Method reference: https://github.com/interactivetools-com/SmartArray/blob/main/docs/method-reference.md
103123
__TEXT__;
104124

105-
// Wrap in <xmp> for readability when output is (or will default to) HTML - same rule
106-
// as SmartArrayBase::xmpWrap(): skip on CLI (terminals show the tags literally), and
107-
// no Content-Type header means PHP sends its default text/html
108-
$inCli = PHP_SAPI === 'cli'
109-
|| ($_SERVER['SESSIONNAME'] ?? '') === 'Console' // Windows console
110-
|| empty($_SERVER['SCRIPT_NAME']); // only web servers set SCRIPT_NAME
111-
$headersList = implode("\n", headers_list());
112-
$isHtmlOutput = !preg_match('|^\s*Content-Type:\s*|im', $headersList)
113-
|| preg_match('|^\s*Content-Type:\s*text/html\b|im', $headersList);
114-
if (!$inCli && $isHtmlOutput) {
115-
$output = "<xmp>$output</xmp>";
116-
}
117-
echo $output;
125+
echo self::xmpWrap("\n$output\n\n");
118126
}
119127

120128
/**

‎tests/Unit/DebugTest.php‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,18 @@ public function testDebugLevelOnePrintsClosureLoadHandlerAsItsType(): void
269269
$this->assertStringContainsString("'author_id'", $output, 'the data still prints');
270270
}
271271

272+
public function testDebugLevelOneLeavesDataRowsKeyedRootUntouched(): void
273+
{
274+
// The root-property reformat runs on the properties block only, so a data
275+
// key that happens to be named 'root' prints its stored value
276+
$sa = SmartArray::new(['root' => 'abc123']);
277+
278+
[, $output] = $this->captureOutput(fn() => $sa->debug(1));
279+
280+
$this->assertStringContainsString("'root' => 'abc123'", $output, 'data value intact');
281+
$this->assertMatchesRegularExpression("/'root'\s+=> SmartArray #\d+/", $output, 'root property still reformatted');
282+
}
283+
272284
//endregion
273285
//region debug(): mysqli metadata
274286

@@ -522,7 +534,23 @@ public function testSmartNullHelpPrintsDocLinksPlainOnCli(): void
522534
__TEXT__;
523535

524536
$this->assertNull($result, 'help() is void');
525-
$this->assertSame($expected, $output);
537+
$this->assertSame("\n$expected\n", $output, 'same xmpWrap() framing as SmartArray::help()');
538+
}
539+
540+
/**
541+
* debug() on a SmartNull says what the object is - a missing key or empty
542+
* result - instead of dumping an empty array of the wrong class.
543+
*/
544+
public function testSmartNullDebugDescribesTheMissingValue(): void
545+
{
546+
$smartNull = SmartArrayHtml::new(['title' => 'Hello'])->titel;
547+
548+
[$result, $output] = $this->captureOutput(fn() => $smartNull->debug());
549+
550+
$this->assertNull($result, 'debug() is void');
551+
$this->assertStringContainsString('Itools\SmartArray\SmartNull - missing key or empty result, value is null', $output);
552+
$this->assertStringContainsString('->isNotEmpty()', $output);
553+
$this->assertStringNotContainsString('SmartArrayHtml', $output, 'no longer misreported as an empty array');
526554
}
527555

528556
//endregion

‎tests/Unit/DeprecationsTest.php‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -718,20 +718,27 @@ public function testSmartArrayRawConstructorDeprecatesAndBehavesLikeSmartArray()
718718
{
719719
[$sa, $deprecations] = $this->captureDeprecations(fn() => new SmartArrayRaw(['name' => 'Bob']));
720720

721-
$this->assertSame(['SmartArrayRaw is deprecated. Use SmartArray instead.'], $deprecations);
721+
$this->assertCount(1, $deprecations);
722+
$this->assertMatchesRegularExpression(
723+
'/^Replace SmartArrayRaw with SmartArray in DeprecationsTest\.php:\d+\.$/',
724+
$deprecations[0],
725+
'notice names the caller, like every other deprecation',
726+
);
722727
$this->assertInstanceOf(SmartArray::class, $sa);
723728
$this->assertSame(['name' => 'Bob'], $sa->toArray());
724729
$this->assertSame('Bob', $sa->name, 'raw mode: values are plain PHP, not SmartStrings');
725730
}
726731

727-
public function testSmartArrayRawNewDeprecatesTwice(): void
732+
public function testSmartArrayRawNewDeprecatesOnce(): void
728733
{
734+
// new() logs nothing itself; the constructor it calls logs the one notice
729735
[$sa, $deprecations] = $this->captureDeprecations(fn() => SmartArrayRaw::new(['name' => 'Bob']));
730736

731-
$this->assertSame([
732-
'SmartArrayRaw::new() is deprecated. Use SmartArray::new() instead.',
733-
'SmartArrayRaw is deprecated. Use SmartArray instead.',
734-
], $deprecations, 'the factory logs, then the constructor it calls logs again');
737+
$this->assertCount(1, $deprecations);
738+
$this->assertMatchesRegularExpression(
739+
'/^Replace SmartArrayRaw with SmartArray in DeprecationsTest\.php:\d+\.$/',
740+
$deprecations[0],
741+
);
735742
$this->assertSame(['name' => 'Bob'], $sa->toArray());
736743
}
737744

‎tests/Unit/GlobalSettingsTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,21 @@ public function testNullOffsetReadUsesTheEmptyStringKey(): void
423423
$this->assertSame(["Replace [''] with ->get('') in FILE:LINE."], $this->normalizeCaller($deprecations));
424424
}
425425

426+
public function testFloatAndBoolOffsetReadsCoerceLikePhpArrayKeys(): void
427+
{
428+
// PHP array key semantics: floats truncate ($arr[1.5] reads 1), bools read
429+
// 1/0 - previously an internal TypeError from getElement()
430+
$sa = SmartArray::new(['a', 'b', 'c']);
431+
432+
[[$values, ], $deprecations] = $this->withOffsetAccess('notify', fn() => $this->captureDeprecations(
433+
fn() => $this->captureOutput(fn() => [$sa[1.5], $sa[true], $sa[false]])
434+
));
435+
436+
$this->assertSame(['b', 'b', 'a'], $values);
437+
$this->assertCount(3, $deprecations);
438+
$this->assertStringContainsString('[1]', $deprecations[0], 'notice shows the coerced key');
439+
}
440+
426441
public function testNullOffsetExistsAndUnsetUseTheEmptyStringKey(): void
427442
{
428443
$sa = SmartArray::new(['' => 'blank']);

‎tests/Unit/WhereTest.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,17 @@ public function testWhereArraySyntaxIsDeprecatedButChains(string $class): void
115115
$this->assertStringContainsString("->where('status', 'active')->where('role', 'admin')", $deprecations[0], 'deprecation shows the chained replacement');
116116
}
117117

118+
#[DataProvider('modeProvider')]
119+
public function testWhereArrayListFormThrowsWithHint(string $class): void
120+
{
121+
// where(['featured']) is a half-migration of where('featured', ...): a list
122+
// has no field names, so the error names the form the caller meant
123+
$this->expectException(InvalidArgumentException::class);
124+
$this->expectExceptionMessage("where(): the array form takes ['field' => value] pairs, list given. Did you mean ->where('featured') to match rows where 'featured' is non-empty?");
125+
126+
$class::new([['featured' => 1]])->where(['featured']);
127+
}
128+
118129
#[DataProvider('modeProvider')]
119130
public function testWhereOnFlatThrows(string $class): void
120131
{
@@ -296,6 +307,17 @@ public function testWhereInListUnwrapsSmartStringValues(string $class): void
296307
$this->assertCount(1, $sa->whereInList('show_on', new SmartString('menu')));
297308
}
298309

310+
#[DataProvider('modeProvider')]
311+
public function testWhereInListRejectsArrayValues(string $class): void
312+
{
313+
// Previously cast to the literal string "Array" with a PHP warning naming
314+
// the library file, then matched nothing
315+
$this->expectException(InvalidArgumentException::class);
316+
$this->expectExceptionMessage('whereInList(): expected a single value to match, got array');
317+
318+
$class::new([['show_on' => 'menu']])->whereInList('show_on', ['menu']);
319+
}
320+
299321
#[DataProvider('modeProvider')]
300322
public function testWhereInListOnFlatThrows(string $class): void
301323
{

0 commit comments

Comments
 (0)