RepeatedField: unset by index (#11425)

Instead of using `array_pop()` to remove last element use `unset()` to remove by index

Closes #11425

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/11425 from Leprechaunz:unset-by-index a47fba57e4
PiperOrigin-RevId: 508691545
pull/11888/head
Andrei Tcaci 2 years ago committed by Copybara-Service
parent 6aefc477d7
commit 363fa89b1f
  1. 4
      php/ext/google/protobuf/array.c
  2. 4
      php/src/Google/Protobuf/Internal/RepeatedField.php
  3. 27
      php/tests/ArrayTest.php

@ -406,13 +406,13 @@ PHP_METHOD(RepeatedField, offsetUnset) {
return; return;
} }
if (size == 0 || index != size - 1) { if (size == 0 || index < 0 || index >= size) {
php_error_docref(NULL, E_USER_ERROR, "Cannot remove element at %ld.\n", php_error_docref(NULL, E_USER_ERROR, "Cannot remove element at %ld.\n",
index); index);
return; return;
} }
upb_Array_Resize(intern->array, size - 1, Arena_Get(&intern->arena)); upb_Array_Delete(intern->array, index, 1);
} }
/** /**

@ -219,13 +219,13 @@ class RepeatedField implements \ArrayAccess, \IteratorAggregate, \Countable
public function offsetUnset($offset) public function offsetUnset($offset)
{ {
$count = count($this->container); $count = count($this->container);
if (!is_numeric($offset) || $count === 0 || $offset !== $count - 1) { if (!is_numeric($offset) || $count === 0 || $offset < 0 || $offset >= $count) {
trigger_error( trigger_error(
"Cannot remove element at the given index", "Cannot remove element at the given index",
E_USER_ERROR); E_USER_ERROR);
return; return;
} }
array_pop($this->container); array_splice($this->container, $offset, 1);
} }
/** /**

@ -655,4 +655,31 @@ class ArrayTest extends TestBase
$arr2 = clone $arr; $arr2 = clone $arr;
$this->assertSame($arr[0], $arr2[0]); $this->assertSame($arr[0], $arr2[0]);
} }
#########################################################
# Test offsetUnset
#########################################################
public function testOffsetUnset()
{
$arr = new RepeatedField(GPBType::INT32);
$arr[] = 0;
$arr[] = 1;
$arr[] = 2;
$this->assertSame(3, count($arr));
$this->assertSame(0, $arr[0]);
$this->assertSame(1, $arr[1]);
$this->assertSame(2, $arr[2]);
$arr->offsetUnset(1);
$this->assertSame(0, $arr[0]);
$this->assertSame(2, $arr[1]);
$arr->offsetUnset(0);
$this->assertSame(2, $arr[0]);
$arr->offsetUnset(0);
$this->assertCount(0, $arr);
}
} }

Loading…
Cancel
Save