Skip to content

Commit

Permalink
bug #6058 Fix Tokens::insertSlices not moving around all affected t…
Browse files Browse the repository at this point in the history 
…okens (paulbalandan, SpacePossum)

This PR was squashed before being merged into the master branch (closes #6058).

Discussion
----------

Fix `Tokens::insertSlices` not moving around all affected tokens

While working on #5953 in refactoring to call `insertSlices` once, I stumbled into a problem where parse error would appear after fixing. After digging into it more, I found out that the culprit is in the `insertSlices` method itself. If there are multiple slices to be inserted in an index and this is repeated in other indexes, there exists a "hole" left in the moving around of tokens. This causes the unmoved tokens to be inadvertently replaced by other else possibly resulting to parse errors after.

Commits
-------

fbac555 Fix `Tokens::insertSlices` not moving around all affected tokens
  • Loading branch information
@SpacePossum
SpacePossum committed Dec 14, 2021
2 parents 2c944f8 + fbac555 commit 4ecba58
Show file tree
Hide file tree
Showing 2 changed files with 199 additions and 10 deletions.
25 changes: 15 additions & 10 deletions src/Tokenizer/Tokens.php
View file
Expand Up @@ -842,6 +842,7 @@ public function insertAt(int $index, $items): void
public function insertSlices(array $slices): void
{
$itemsCount = 0;

foreach ($slices as $slice) {
$itemsCount += \is_array($slice) || $slice instanceof self ? \count($slice) : 1;
}
Expand All @@ -858,20 +859,28 @@ public function insertSlices(array $slices): void

krsort($slices);

if (array_key_first($slices) > $oldSize) {
throw new \OutOfBoundsException('Cannot insert outside of collection.');
}

$insertBound = $oldSize - 1;

// since we only move already existing items around, we directly call into SplFixedArray::offset* methods.
// that way we get around additional overhead this class adds with overridden offset* methods.
foreach ($slices as $index => $slice) {
if (!\is_int($index) || $index < 0) {
throw new \OutOfBoundsException(sprintf('Invalid index "%s".', $index));
}

$slice = \is_array($slice) || $slice instanceof self ? $slice : [$slice];
$sliceCount = \count($slice);

for ($i = $insertBound; $i >= $index; --$i) {
$oldItem = parent::offsetExists($i) ? parent::offsetGet($i) : new Token('');
parent::offsetSet($i + $itemsCount, $oldItem);
parent::offsetSet($i + $itemsCount, parent::offsetGet($i));
}

$insertBound = $index - $sliceCount;
// adjust $insertBound as tokens between this index and the next index in loop
$insertBound = $index - 1;
$itemsCount -= $sliceCount;

foreach ($slice as $indexItem => $item) {
Expand All @@ -880,8 +889,8 @@ public function insertSlices(array $slices): void
}

$this->registerFoundToken($item);
$newOffset = $index + $itemsCount + $indexItem;
parent::offsetSet($newOffset, $item);

parent::offsetSet($index + $itemsCount + $indexItem, $item);
}
}
}
Expand All @@ -891,11 +900,7 @@ public function insertSlices(array $slices): void
*/
public function isChanged(): bool
{
if ($this->changed) {
return true;
}

return false;
return $this->changed;
}

public function isEmptyAt(int $index): bool
Expand Down
184 changes: 184 additions & 0 deletions tests/Tokenizer/TokensTest.php
View file
Expand Up @@ -1368,6 +1368,190 @@ public function provideGetMeaningfulTokenSiblingCases(): \Generator
yield [null, 0, -1, '<?php /**/ foo();'];
}

/**
* @dataProvider provideInsertSlicesAtMultiplePlacesCases
*
* @param array<int, Token> $slices
*/
public function testInsertSlicesAtMultiplePlaces(string $expected, array $slices): void
{
$input = <<<'EOF'
<?php
$after = get_class($after);
$before = get_class($before);
EOF;

$tokens = Tokens::fromCode($input);
$tokens->insertSlices([
16 => $slices,
6 => $slices,
]);

static::assertSame($expected, $tokens->generateCode());
}

public function provideInsertSlicesAtMultiplePlacesCases(): \Generator
{
yield 'one slice count' => [
<<<'EOF'
<?php
$after = get_class($after);
$before = get_class($before);
EOF
,
[new Token([T_WHITESPACE, ' '])],
];

yield 'two slice count' => [
<<<'EOF'
<?php
$after = (string) get_class($after);
$before = (string) get_class($before);
EOF
,
[new Token([T_STRING_CAST, '(string)']), new Token([T_WHITESPACE, ' '])],
];

yield 'three slice count' => [
<<<'EOF'
<?php
$after = !(bool) get_class($after);
$before = !(bool) get_class($before);
EOF
,
[new Token('!'), new Token([T_BOOL_CAST, '(bool)']), new Token([T_WHITESPACE, ' '])],
];
}

public function testInsertSlicesChangesState(): void
{
$tokens = Tokens::fromCode('<?php echo 1234567890;');

static::assertFalse($tokens->isChanged());
static::assertFalse($tokens->isTokenKindFound(T_COMMENT));
static::assertSame(5, $tokens->getSize());

$tokens->insertSlices([1 => new Token([T_COMMENT, '/* comment */'])]);

static::assertTrue($tokens->isChanged());
static::assertTrue($tokens->isTokenKindFound(T_COMMENT));
static::assertSame(6, $tokens->getSize());
}

/**
* @dataProvider provideInsertSlicesCases
*/
public function testInsertSlices(Tokens $expected, Tokens $tokens, array $slices): void
{
$tokens->insertSlices($slices);
static::assertTokens($expected, $tokens);
}

public function provideInsertSlicesCases(): iterable
{
// basic insert of single token at 3 different locations including appending as new token

$template = "<?php\n%s\n/* single token test header */%s\necho 1;\n%s";
$commentContent = '/* test */';
$commentToken = new Token([T_COMMENT, $commentContent]);
$from = Tokens::fromCode(sprintf($template, '', '', ''));

yield 'single insert @ 1' => [
Tokens::fromCode(sprintf($template, $commentContent, '', '')),
clone $from,
[1 => $commentToken],
];

yield 'single insert @ 3' => [
Tokens::fromCode(sprintf($template, '', $commentContent, '')),
clone $from,
[3 => Tokens::fromArray([$commentToken])],
];

yield 'single insert @ 9' => [
Tokens::fromCode(sprintf($template, '', '', $commentContent)),
clone $from,
[9 => [$commentToken]],
];

// basic tests for single token, array of that token and tokens object with that token

$openTagToken = new Token([T_OPEN_TAG, "<?php\n"]);
$expected = Tokens::fromArray([$openTagToken]);

$slices = [
[0 => $openTagToken],
[0 => [$openTagToken]],
[0 => Tokens::fromArray([$openTagToken])],
];

foreach ($slices as $i => $slice) {
yield 'insert open tag @ 0 into empty collection '.$i => [$expected, new Tokens(), $slice];
}

// test insert lists of tokens, index out of order

$setOne = [
new Token([T_ECHO, 'echo']),
new Token([T_WHITESPACE, ' ']),
new Token([T_CONSTANT_ENCAPSED_STRING, '"new"']),
new Token(';'),
];

$setTwo = [
new Token([T_WHITESPACE, ' ']),
new Token([T_COMMENT, '/* new comment */']),
];

$setThree = Tokens::fromArray([
new Token([T_VARIABLE, '$new']),
new Token([T_WHITESPACE, ' ']),
new Token('='),
new Token([T_WHITESPACE, ' ']),
new Token([T_LNUMBER, '8899']),
new Token(';'),
new Token([T_WHITESPACE, "\n"]),
]);

$template = "<?php\n%s\n/* header */%s\necho 789;\n%s";
$expected = Tokens::fromCode(
sprintf(
$template,
'echo "new";',
' /* new comment */',
"\$new = 8899;\n"
)
);
$from = Tokens::fromCode(sprintf($template, '', '', ''));

yield 'insert 3 token collections' => [$expected, $from, [9 => $setThree, 1 => $setOne, 3 => $setTwo]];

$sets = [];

for ($j = 0; $j < 4; ++$j) {
$set = ['tokens' => [], 'content' => ''];

for ($i = 0; $i < 10; ++$i) {
$content = sprintf('/* new %d|%s */', $j, $i);

$set['tokens'][] = new Token([T_COMMENT, $content]);
$set['content'] .= $content;
}

$sets[$j] = $set;
}

yield 'overlapping inserts of bunch of comments ' => [
Tokens::fromCode(sprintf("<?php\n%s/* line 1 */\n%s/* line 2 */\n%s/* line 3 */%s", $sets[0]['content'], $sets[1]['content'], $sets[2]['content'], $sets[3]['content'])),
Tokens::fromCode("<?php\n/* line 1 */\n/* line 2 */\n/* line 3 */"),
[1 => $sets[0]['tokens'], 3 => $sets[1]['tokens'], 5 => $sets[2]['tokens'], 6 => $sets[3]['tokens']],
];
}

private static function assertFindBlockEnd(int $expectedIndex, string $source, int $type, int $searchIndex): void
{
Tokens::clearCache();
Expand Down

0 comments on commit 4ecba58

Please sign in to comment.