Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[PropertyAccess] PropertyPathBuilder remove/append/remove fails by not resetting indexes #30389

Copy link
Copy link
Closed
@GregOriol

Description

@GregOriol
Issue body actions

Symfony version(s) affected: 4.2 and likely older (up to 2.x ?)

Description
I'm trying to use the PropertyPathBuilder and found this strange behavior: when you remove the last item from the path, append a new one and try to remove it, you get the "The offset %s is not within the property path", which doesn't seem correct

How to reproduce

$builder = new PropertyPathBuilder('old1.old2');

$builder->getPropertyPath(); // returns old1.old2
$builder->getLength(); // returns 2

$builder->remove($builder->getLength() - 1); // should remove old2
$builder->getPropertyPath()); // returns old1
$builder->getLength(); // returns 1

$builder->appendProperty('old3');
$builder->getPropertyPath(); // returns old1.old3
$builder->getLength(); // returns 2

// so far everything seems correct

$builder->remove($builder->getLength() - 1); // should remove old3, but fails as offset 1 doesn't exist
$builder->getPropertyPath(); // I expected that this would return old1?

Possible Solution
Having looked at the code of the builder in detail, the elements and isIndex arrays are updated using unset on remove and [] = $ on append, and unset doesn't reset the "next index", making the $builder->appendProperty('old3'); happen at index 2, while there is only index 0 left

In other words, after $builder->appendProperty('old3');, the internal list of elements is:

[
0 => 'old1',
2 => 'old3'
]

instead of what I had expected:

[
0 => 'old1',
1 => 'old3'
]

A solution could be to add:

$this->elements = array_values($this->elements);
$this->isIndex = array_values($this->isIndex);

at the end of the resize method, which is used by remove. This would reset the indexes/offsets.

Additional context
There is no test case for this scenario, so I'm not sure if it was intended like this (maybe I didn't understand correctly how it should be used?) or if nobody has yet tried something like this?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Morty Proxy This is a proxified and sanitized view of the page, visit original site.