Skip to content

Commit e4f98a5

Browse files
committed
deprecations: keep offsetExists() silent so ?? and empty() notify once
PHP calls offsetExists() then offsetGet() for $arr['key'] ?? and empty($arr['key']), so the notice added to existence checks this release printed every message twice for those forms. offsetExists() is signal-free again, matching 2.7.0: the read carries the one notice, and a bare isset() with no read stays silent but still returns the right answer. Dropped the CHANGELOG bullet announcing notices on existence checks and updated the test matrix to pin the new contract.
1 parent 4f8afc1 commit e4f98a5

4 files changed

Lines changed: 45 additions & 46 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,6 @@ the docs - IDEs show a strikethrough with the replacement.
121121
`$row->missing->or('n/a')` on a raw array throws the standard
122122
undefined-method Error instead of returning an HTML-encoding SmartString.
123123
Raw fallbacks use `??`. HTML mode is unchanged.
124-
- `isset($array['key'])` and `empty($array['key'])` follow `$onOffsetAccess`
125-
like reads, writes, and `unset()` - notice by default, exception in
126-
`'throw'` mode. Existence checks were the one silent form of the deprecated
127-
`[]` syntax. Property-syntax checks (`isset($array->key)`) stay
128-
signal-free.
129124
- `set()`, `->key = $value`, and array assignment unwrap Smart values
130125
(SmartString, SmartArray, SmartNull) instead of throwing, so values copy
131126
between arrays in any mode without calling `->value()` first. SmartNull
@@ -168,10 +163,8 @@ the docs - IDEs show a strikethrough with the replacement.
168163
TypeError that named library internals. `unset()` and `isset()` already
169164
worked this way.
170165
- Array-syntax deprecation notices suggest one replacement style across reads,
171-
writes, `isset()`, and `unset()`: `->key` and `->key = $value` for
172-
property-safe names, `->{0}` for integer keys, `->{'users.id'}` for other
173-
keys. Reads used to suggest `->get(0)` while existence checks suggested
174-
`->{0}`, so one `empty()` call printed two notices with different advice.
166+
writes, and `unset()`: `->key` and `->key = $value` for property-safe
167+
names, `->{0}` for integer keys, `->{'users.id'}` for other keys.
175168
Null and `''` keys suggest `->get('')` / `->set('', $value)` - the brace
176169
form is a fatal error for an empty property name.
177170
- Unknown methods on `SmartNull` throw the same `Error` as the rest of the

src/Deprecations.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ trait Deprecations
5151
* `$array->key` for reads, `$array->key = $value` for writes, and brace
5252
* syntax (`$array->{'users.id'}`) for keys property syntax can't type. This setting
5353
* controls how the library signals that deprecation at runtime. It covers
54-
* reads, writes, unset(), and isset()/empty() checks alike; only the
55-
* property forms (`$array->key`, `isset($array->key)`) are signal-free.
54+
* reads, writes, and unset(); existence checks (offsetExists) are signal-free
55+
* because PHP also calls offsetGet() for `??` and empty(), which carries the
56+
* one notice. Property forms (`$array->key`, `isset($array->key)`) are always
57+
* signal-free.
5658
*
5759
* 'log' - trigger_error(E_USER_DEPRECATED) only. Silent unless surfaced
5860
* by PHP's error handling. Use for legacy codebases mid-migration.
@@ -438,7 +440,9 @@ public function offsetGet(mixed $offset): static|SmartNull|SmartString|string|in
438440
*/
439441
public function offsetExists(mixed $offset): bool
440442
{
441-
$this->triggerArrayAccessDeprecation($offset, 'exists');
443+
// No notice here: PHP calls offsetExists() then offsetGet() for `??` and empty(),
444+
// and offsetGet() already notifies, so one here would print every message twice.
445+
// A bare isset() with no read stays silent; any access that reads data notifies.
442446
return isset($this->data[$offset]);
443447
}
444448

tests/Unit/GlobalSettingsTest.php

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@
2020
* Covers the full matrix: modes (notify/log/throw) x operations (offsetGet,
2121
* offsetSet, append, offsetUnset, offsetExists via isset() and empty()), the
2222
* exact suggestion text for each key shape, and the invalid-mode error. Also
23-
* pins what must stay signal-free: property syntax and the library's own
24-
* internal element access.
23+
* pins what must stay signal-free: property syntax, existence checks
24+
* (offsetExists - the read carries the notice for `??` and empty()), and the
25+
* library's own internal element access.
2526
*
2627
* Every test swaps the static through withOffsetAccess(), which restores it in a
2728
* finally block so a failure here cannot poison other test files.
@@ -68,18 +69,19 @@ private static function offsetOperations(): array
6869
'unset int key' => [function (SmartArrayBase $sa) { unset($sa[0]); }, ["Replace [0] with ->{0}"]],
6970
'unset invalid prop name' => [function (SmartArrayBase $sa) { unset($sa['users.id']); }, ["Replace ['users.id'] with ->{'users.id'}"]],
7071

71-
// offsetExists via isset(): one call, whether or not the key is there
72-
'isset string key' => [fn(SmartArrayBase $sa) => isset($sa['name']), ["Replace ['name'] with ->name"]],
73-
'isset int key' => [fn(SmartArrayBase $sa) => isset($sa[0]), ["Replace [0] with ->{0}"]],
74-
'isset invalid prop name' => [fn(SmartArrayBase $sa) => isset($sa['users.id']), ["Replace ['users.id'] with ->{'users.id'}"]],
75-
'isset missing key' => [fn(SmartArrayBase $sa) => isset($sa['zzz']), ["Replace ['zzz'] with ->zzz"]],
76-
77-
// offsetExists via empty(): PHP calls offsetExists, then offsetGet when the key exists,
78-
// so an existing key signals twice for one empty() check - both notices give the
79-
// same suggestion (reads and existence checks share one suggestion style)
80-
'empty existing key' => [fn(SmartArrayBase $sa) => empty($sa['name']), ["Replace ['name'] with ->name", "Replace ['name'] with ->name"]],
81-
'empty int key' => [fn(SmartArrayBase $sa) => empty($sa[0]), ["Replace [0] with ->{0}", "Replace [0] with ->{0}"]],
82-
'empty missing key' => [fn(SmartArrayBase $sa) => empty($sa['zzz']), ["Replace ['zzz'] with ->zzz"]],
72+
// offsetExists via isset(): silent - PHP also calls offsetGet() for `??` and
73+
// empty(), which carries the one notice, so a notice here would double every
74+
// message. A bare isset() gives no signal but still returns the right answer.
75+
'isset string key' => [fn(SmartArrayBase $sa) => isset($sa['name']), []],
76+
'isset int key' => [fn(SmartArrayBase $sa) => isset($sa[0]), []],
77+
'isset invalid prop name' => [fn(SmartArrayBase $sa) => isset($sa['users.id']), []],
78+
'isset missing key' => [fn(SmartArrayBase $sa) => isset($sa['zzz']), []],
79+
80+
// offsetExists via empty(): offsetExists is silent, then PHP calls offsetGet
81+
// when the key exists - one notice for existing keys, none for missing ones
82+
'empty existing key' => [fn(SmartArrayBase $sa) => empty($sa['name']), ["Replace ['name'] with ->name"]],
83+
'empty int key' => [fn(SmartArrayBase $sa) => empty($sa[0]), ["Replace [0] with ->{0}"]],
84+
'empty missing key' => [fn(SmartArrayBase $sa) => empty($sa['zzz']), []],
8385
];
8486
}
8587

@@ -140,7 +142,7 @@ public function testNotifyPerformsTheOperation(string $class): void
140142
$this->assertTrue($issetName);
141143
$this->assertFalse($issetMissing);
142144
$this->assertSame(['name' => 'Bob', '' => 'blank', 0 => 'zero', 'city' => 'Vancouver', 1 => 'appended'], $sa->toArray());
143-
$this->assertSame(6, substr_count($output, "\nDeprecated: "), 'one notice per offset operation');
145+
$this->assertSame(4, substr_count($output, "\nDeprecated: "), 'one notice per read/write; isset checks are silent');
144146
}
145147

146148
//endregion
@@ -190,8 +192,12 @@ public function testThrowRaisesRuntimeExceptionWithTheSuggestion(string $class,
190192
fn() => $this->captureOutput(fn() => $this->catchThrowable(fn() => $operation($sa)))
191193
));
192194

193-
$this->assertInstanceOf(RuntimeException::class, $thrown);
194-
$this->assertSame($expectedMessages[0] . ' in FILE:LINE.', $this->normalizeCaller($thrown->getMessage()));
195+
if ($expectedMessages === []) {
196+
$this->assertNull($thrown, 'signal-free operations run normally in throw mode');
197+
} else {
198+
$this->assertInstanceOf(RuntimeException::class, $thrown);
199+
$this->assertSame($expectedMessages[0] . ' in FILE:LINE.', $this->normalizeCaller($thrown->getMessage()));
200+
}
195201
$this->assertSame('', $output, 'throw mode replaces the notice, it does not add to it');
196202
$this->assertSame([], $deprecations, 'throw mode exits before trigger_error()');
197203
}
@@ -287,7 +293,7 @@ public function testInvalidModeThrowsForEveryOffsetOperation(): void
287293
'set' => function () use ($sa) { $sa['city'] = 'Vancouver'; },
288294
'append' => function () use ($sa) { $sa[] = 'appended'; },
289295
'unset' => function () use ($sa) { unset($sa['name']); },
290-
'isset' => fn() => isset($sa['name']),
296+
// no 'isset' row: offsetExists is signal-free and never consults the setting
291297
];
292298

293299
foreach ($operations as $label => $operation) {
@@ -431,36 +437,34 @@ public function testNullOffsetExistsAndUnsetUseTheEmptyStringKey(): void
431437

432438
$this->assertTrue($exists);
433439
$this->assertSame([], $sa->toArray());
440+
441+
// One notice, from the unset - the isset is signal-free
434442
$this->assertSame(
435-
"\nDeprecated: Replace [] with ->get('') in FILE:LINE.\n\nDeprecated: Replace [] with ->get('') in FILE:LINE.\n",
443+
"\nDeprecated: Replace [] with ->get('') in FILE:LINE.\n",
436444
$this->normalizeCaller($output),
437445
);
438446
$this->assertSame(
439-
["Replace [] with ->get('') in FILE:LINE.", "Replace [] with ->get('') in FILE:LINE."],
447+
["Replace [] with ->get('') in FILE:LINE."],
440448
$this->normalizeCaller($deprecations),
441449
);
442450
}
443451

444452
//endregion
445453
//region Nested chains
446454

447-
public function testNestedIssetChainSignalsThreeTimes(): void
455+
public function testNestedIssetChainSignalsOnce(): void
448456
{
449-
// isset($sa['user']['name']) reports ['user'] twice - PHP checks the outer key
450-
// with offsetExists, then reads it with offsetGet to reach the inner one. That
451-
// call sequence is PHP's, not ours; all notices agree on the suggestion.
457+
// isset($sa['user']['name']): PHP checks the outer key with offsetExists
458+
// (silent), reads it with offsetGet (the one notice) to reach the inner
459+
// offsetExists (silent). That call sequence is PHP's, not ours.
452460
$sa = SmartArray::new(['user' => ['name' => 'Bob']]);
453461

454462
[$exists, $deprecations] = $this->withOffsetAccess('log', fn() => $this->captureDeprecations(
455463
fn() => isset($sa['user']['name'])
456464
));
457465

458466
$this->assertTrue($exists);
459-
$this->assertSame([
460-
"Replace ['user'] with ->user in FILE:LINE.",
461-
"Replace ['user'] with ->user in FILE:LINE.",
462-
"Replace ['name'] with ->name in FILE:LINE.",
463-
], $this->normalizeCaller($deprecations));
467+
$this->assertSame(["Replace ['user'] with ->user in FILE:LINE."], $this->normalizeCaller($deprecations));
464468
}
465469

466470
public function testNestedReadChainSignalsOncePerLevel(): void

tests/Unit/ReadAccessTest.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -371,16 +371,14 @@ public function testIssetTreatsStoredNullAsMissing(string $class): void
371371
$this->assertFalse(isset($sa->middle));
372372
$this->assertFalse(isset($sa->zzz));
373373

374-
// Array-syntax existence checks follow $onOffsetAccess like reads and
375-
// writes do (default 'notify'); only the property forms are signal-free
374+
// Array-syntax existence checks are signal-free: for `??` and empty()
375+
// PHP also calls offsetGet(), which carries the one notice
376376
[, $output] = $this->captureOutput(function () use ($sa) {
377377
$this->assertFalse($sa->offsetExists('middle'));
378378
$this->assertFalse($sa->offsetExists('zzz'));
379379
$this->assertTrue(isset($sa['name']));
380380
});
381-
$this->assertSame(3, substr_count($output, 'Deprecated:'), 'one notice per array-syntax check');
382-
$this->assertStringContainsString("Replace ['middle'] with ->middle", $output);
383-
$this->assertStringContainsString("Replace ['zzz'] with ->zzz", $output);
381+
$this->assertSame('', $output, 'existence checks emit nothing');
384382
}
385383

386384
#[DataProvider('modeProvider')]

0 commit comments

Comments
 (0)