fix(array-def): update list index validation to use PHP's append position

- Replace php::safeIndex with php::safeArrayIndex for consistent boundary checking
- Remove special case handling for property[count(property)] append operations
- Use zend_hash_next_free_element() based append position instead of length()
- Update tests to verify inclusive upper bound behavior for sparse arrays
- Add comprehensive test coverage for unset() hole scenarios
- Modify documentation to reflect new indexing behavior and boundaries
master
韩天峰 6 days ago
parent ab1854beb0
commit 0d0e62fbc7
  1. 13
      docs/ARRAY_DEF.md
  2. 13
      phpunit/code/array-def-inclusive-upper-bound.php
  3. 20
      phpunit/src/ArrayDefTest.php
  4. 42
      src/ArrayDef/ArrayDefSupportTrait.php
  5. 78
      tests/compiler/array-def/list-unset-holes.phpt
  6. 7
      tests/compiler/array-def/zend-and-native.phpt

@ -25,10 +25,15 @@ the normal write unchanged or reports a fatal type error. An `any` key/value is
checked with PHPX exact-type helpers at runtime. No coercive `intval()` or
string conversion is performed.
List writes support `[]`, an existing integer index, and the exact append form
`$object->property[count($object->property)]`. Existing-index writes use
`php::safeIndex()`; negative and out-of-range indexes fail at runtime. Maps do
not support `[]` append writes.
List writes support `[]` and non-negative integer indexes up to PHP's current
append position. Indexed writes uniformly emit `php::safeArrayIndex(index,
array)`. The helper uses `zend_hash_next_free_element()` and follows the
initial-index rule of `zend_hash_next_index_insert()`, which remains correct
when `unset()` has created holes or removed the highest numeric key. An index
equal to that value behaves like the next `$array[]` append; earlier indexes
may update or refill an element. Negative indexes and indexes beyond the append
position fail at runtime. There is no AST special case for
`property[count(property)]`. Maps do not support `[]` append writes.
The contract intentionally applies only to direct element assignment lowered
by TypePHP. Reads and in-place operators are unchanged. Values passed through

@ -0,0 +1,13 @@
<?php
class ArrayDefInclusiveUpperBound
{
#[ArrayDef(Type::String)]
public array $values = [];
}
function writeArrayDefInclusiveUpperBound(ArrayDefInclusiveUpperBound $box, int $index): void
{
$box->values[$index] = 'indexed';
$box->values[count($box->values)] = 'counted';
}

@ -1,5 +1,7 @@
<?php
use TypePhp\CompilerTest;
final class ArrayDefTest extends \BaseTest
{
public function testArrayDefDeclarationAndDirectWriteDiagnostics(): void
@ -16,4 +18,22 @@ final class ArrayDefTest extends \BaseTest
$this->exec('Native class types cannot be used in ArrayDef', 'array-def-native-class-value.php');
$this->exec('Std Container values cannot be stored in ArrayDef properties', 'array-def-std-container-value.php');
}
public function testListIndexUsesPhpAppendBoundaryWithoutAstSpecialCase(): void
{
global $translator;
$compiler = CompilerTest::create(ROOT_PATH);
$translator = $compiler;
$source = ROOT_PATH . '/phpunit/code/array-def-inclusive-upper-bound.php';
$compiler->addFiles([$source]);
$compiler->prepareFile($source);
$generated = $compiler->convertFile($source);
$code = file_get_contents($generated);
self::assertIsString($code);
self::assertSame(2, substr_count($code, 'php::safeArrayIndex('));
self::assertStringNotContainsString('.length() + 1', $code);
self::assertStringNotContainsString('.newItem()', $code);
}
}

@ -117,16 +117,14 @@ trait ArrayDefSupportTrait
return new ArrayDefWritePlan(true, null, $value);
}
if ($arrayDef->isList() && $this->isArrayDefCountAppend($left->dim, $left->var)) {
return new ArrayDefWritePlan(true, null, $value);
}
$expectedKey = $arrayDef->keyType ?? Type::INT;
$key = $this->parseExprAsValue($left->dim);
$key = $this->validateArrayDefWriteValue($left->var, $left->dim, $key, $expectedKey, 'key');
if ($arrayDef->isList()) {
$array = $this->parseWritableIdentifier($left->var);
$key = 'php::safeIndex(' . $key . ', ' . $array . '.length())';
// PHP's append index does not shrink after unset(). Element count
// is therefore not a valid list-write boundary for sparse arrays.
$key = 'php::safeArrayIndex(' . $key . ', ' . $array . ')';
}
return new ArrayDefWritePlan(false, $key, $value);
@ -254,38 +252,4 @@ trait ArrayDefSupportTrait
], true);
}
private function isArrayDefCountAppend(Expr $dim, Expr $property): bool
{
if (!$dim instanceof Expr\FuncCall
|| !$dim->name instanceof Node\Name
|| strtolower(ltrim($dim->name->toString(), '\\')) !== 'count'
|| count($dim->args) !== 1
|| !$dim->args[0] instanceof Node\Arg
) {
return false;
}
return $this->isSameArrayDefProperty($dim->args[0]->value, $property);
}
private function isSameArrayDefProperty(Expr $left, Expr $right): bool
{
if ($left instanceof Expr\StaticPropertyFetch && $right instanceof Expr\StaticPropertyFetch) {
return $left->class instanceof Node\Name
&& $right->class instanceof Node\Name
&& $left->name instanceof Node\Identifier
&& $right->name instanceof Node\Identifier
&& strcasecmp($left->class->toString(), $right->class->toString()) === 0
&& strcasecmp($left->name->toString(), $right->name->toString()) === 0;
}
if (!$left instanceof Expr\PropertyFetch || !$right instanceof Expr\PropertyFetch
|| !$left->name instanceof Node\Identifier || !$right->name instanceof Node\Identifier
|| strcasecmp($left->name->toString(), $right->name->toString()) !== 0
) {
return false;
}
return $left->var instanceof Expr\Variable
&& $right->var instanceof Expr\Variable
&& is_string($left->var->name)
&& $left->var->name === $right->var->name;
}
}

@ -0,0 +1,78 @@
--TEST--
ArrayDef list indexed writes use PHP's append position after unset
--FILE--
<?php
class ArrayDefListWithHoles
{
#[ArrayDef(Type::String)]
public array $values = [];
}
#[Native]
class NativeArrayDefListWithHoles
{
#[ArrayDef(Type::Int)]
public array $values = [];
}
function writeListValue(ArrayDefListWithHoles $box, int $index, string $value): void
{
$box->values[$index] = $value;
}
function writeNativeListValue(NativeArrayDefListWithHoles $box, int $index, int $value): void
{
$box->values[$index] = $value;
}
function main(): void
{
$box = new ArrayDefListWithHoles();
writeListValue($box, 0, 'zero');
$box->values[] = 'one';
$box->values[] = 'two';
unset($box->values[1]);
writeListValue($box, 3, 'three');
unset($box->values[3]);
writeListValue($box, 4, 'four');
writeListValue($box, 1, 'one-again');
var_dump($box->values);
try {
writeListValue($box, 6, 'gap');
} catch (Error $error) {
echo "list gap rejected\n";
}
$native = new NativeArrayDefListWithHoles();
$native->values[] = 10;
$native->values[] = 20;
unset($native->values[1]);
writeNativeListValue($native, 2, 30);
unset($native->values[2]);
writeNativeListValue($native, 3, 40);
var_dump($native->values);
}
?>
--EXPECT--
array(4) {
[0]=>
string(4) "zero"
[2]=>
string(3) "two"
[4]=>
string(4) "four"
[1]=>
string(9) "one-again"
}
list gap rejected
array(2) {
[0]=>
int(10)
[3]=>
int(40)
}

@ -62,6 +62,7 @@ function main(): void
$native->counters['ok'] = 11;
writeDynamicList($zend, 1, 'dynamic');
writeDynamicList($zend, count($zend->names), 'appended');
writeDynamicMap($native, 'dynamic', 12);
var_dump($zend->names, $zend->labels, ZendArrayDefBox::$staticCounters, $promoted->values, $native->values, $native->counters);
@ -87,18 +88,20 @@ function main(): void
echo "map value type checked\n";
}
try {
writeDynamicList($zend, 20, 'out');
writeDynamicList($zend, count($zend->names) + 1, 'out');
} catch (Error $error) {
echo "list bounds checked\n";
}
}
?>
--EXPECT--
array(2) {
array(3) {
[0]=>
string(7) "changed"
[1]=>
string(7) "dynamic"
[2]=>
string(8) "appended"
}
array(1) {
[10]=>

Loading…
Cancel
Save