Skip to content

Commit

Permalink
[BUGFIX] TS copy operator handles existing reference
Browse files Browse the repository at this point in the history 
This implements a rather unexpected detail when combining
reference "=<" and copy "<" operator from old TypoScript
parser.

Copy operator:

lib.source1 = TEXT
lib.source1.value = source1Value
lib.source2.otherValue = source2OtherValue
tt_content.target < lib.source1
tt_content.target < lib.source2

Result:

tt_content.target = TEXT
tt_content.target.otherValue = source2OtherValue

The tt_content.target existing value from lib.source1 is
kept, but children are reset - lib.source1.value is not
within tt_content.target anymore.

Now reference first, then copy:

lib.source1 = TEXT
lib.source1.value = source1Value
lib.source2.otherValue = source2OtherValue
tt_content.target =< lib.source1
tt_content.target < lib.source2

Old parser after resolving reference in FE:

tt_content.target = TEXT
tt_content.target.value = source1Value
tt_content.target.otherValue = source2OtherValue

New parser after resolving reference in FE:

tt_content.target = TEXT
tt_content.target.otherValue = source2OtherValue

The old parser keeps existing references when the copy
operator does not reset the value to something else.

The patch adapts the new parser to behave identical and
refactors the AST copy method to better documented, more
easy and eventually also quicker code.

Resolves: #100115
Releases: main, 12.4
Change-Id: Ia03cefd6e550833bdc9120a179881d04bf70d117
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80461
Reviewed-by: Tomek Wolda?ski <[email protected]>
Tested-by: Christian Kuhn <[email protected]>
Reviewed-by: Christian Kuhn <[email protected]>
Reviewed-by: Stefan B�rk <[email protected]>
Tested-by: Tomek Wolda?ski <[email protected]>
Tested-by: core-ci <[email protected]>
  • Loading branch information
@twoldanski @lolli42
twoldanski authored and lolli42 committed Aug 10, 2023
1 parent 43d92d7 commit 758d744
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 40 deletions.
94 changes: 56 additions & 38 deletions Classes/TypoScript/AST/AbstractAstBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@

use Psr\EventDispatcher\EventDispatcherInterface;
use TYPO3\CMS\Core\TypoScript\AST\CurrentObjectPath\CurrentObjectPath;
use TYPO3\CMS\Core\TypoScript\AST\CurrentObjectPath\CurrentObjectPathStack;
use TYPO3\CMS\Core\TypoScript\AST\Event\EvaluateModifierFunctionEvent;
use TYPO3\CMS\Core\TypoScript\AST\Node\ChildNode;
use TYPO3\CMS\Core\TypoScript\AST\Node\ChildNodeInterface;
use TYPO3\CMS\Core\TypoScript\AST\Node\NodeInterface;
use TYPO3\CMS\Core\TypoScript\AST\Node\ReferenceChildNode;
use TYPO3\CMS\Core\TypoScript\AST\Node\RootNode;
use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\IdentifierCopyLine;
use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\IdentifierReferenceLine;
use TYPO3\CMS\Core\TypoScript\Tokenizer\Line\IdentifierUnsetLine;
Expand All @@ -45,7 +45,7 @@ abstract class AbstractAstBuilder
protected array $flatConstants = [];
protected EventDispatcherInterface $eventDispatcher;

protected function handleIdentifierUnsetLine(IdentifierUnsetLine $line, CurrentObjectPath $currentObjectPath)
protected function handleIdentifierUnsetLine(IdentifierUnsetLine $line, CurrentObjectPath $currentObjectPath): void
{
$node = $currentObjectPath->getFirst();
$identifierStream = $line->getIdentifierTokenStream()->reset();
Expand All @@ -66,60 +66,78 @@ protected function handleIdentifierUnsetLine(IdentifierUnsetLine $line, CurrentO
}
}

protected function handleIdentifierCopyLine(IdentifierCopyLine $line, CurrentObjectPathStack $currentObjectPathStack, CurrentObjectPath $currentObjectPath): ?NodeInterface
protected function handleIdentifierCopyLine(IdentifierCopyLine $line, RootNode $rootNode, CurrentObjectPath $currentObjectPath): ?NodeInterface
{
$sourceIdentifierStream = $line->getValueTokenStream()->reset();
$sourceNode = $rootNode;
if ($sourceIdentifierStream->isRelative()) {
// Entry node is current node from current object path if relative, otherwise RootNode.
$sourceNode = $currentObjectPath->getLast();
} else {
$sourceNode = $currentObjectPathStack->getFirst()->getFirst();
}
while ($identifierToken = $sourceIdentifierStream->getNext()) {
if (!$foundNode = $sourceNode->getChildByName($identifierToken->getValue())) {
// Go through source token stream and locate the sourceNode to copy from.
if (!$sourceNode = $sourceNode->getChildByName($identifierToken->getValue())) {
// Source node not found - nothing to do for this line
return null;
}
$sourceNode = $foundNode;
}
$isSourceNodeValueNull = true;
if ($sourceNode->getValue() !== null) {
// When the source node value is not null, it will override the target node value if that exists.
$isSourceNodeValueNull = false;
}
$targetNode = $currentObjectPath->getFirst();

// Locate/create the targets parent node the copied source should be added as child to,
// and get the name of the node we're dealing with.
$targetIdentifierTokenStream = $line->getIdentifierTokenStream()->reset();
$previousTargetIdentifierToken = $targetIdentifierTokenStream->peekNext();
while ($targetIdentifierToken = $targetIdentifierTokenStream->getNext()) {
$hasNext = (bool)($targetIdentifierTokenStream->peekNext() ?? false);
if (!$hasNext) {
if ($isSourceNodeValueNull) {
$existingTargetNodeValue = $targetNode->getChildByName($previousTargetIdentifierToken->getValue())?->getValue();
if ($existingTargetNodeValue === null) {
$existingTargetNodeValue = $targetNode->getChildByName($targetIdentifierToken->getValue())?->getValue();
}
} else {
// Blindly remove existing node if exists
$targetNode->removeChildByName($previousTargetIdentifierToken->getValue());
}
// Clone full source tree and update identifier name
/** @var ChildNodeInterface $clonedNode */
$clonedNode = clone $sourceNode;
$clonedNode->updateName($targetIdentifierToken->getValue());
if ($isSourceNodeValueNull && $existingTargetNodeValue) {
$clonedNode->setValue($existingTargetNodeValue);
}
$targetNode->addChild($clonedNode);
// Done
return $clonedNode;
$targetParentNode = $currentObjectPath->getFirst();
$targetTokenName = null;
while ($targetToken = $targetIdentifierTokenStream->getNext()) {
$targetTokenName = $targetToken->getValue();
if (!($targetIdentifierTokenStream->peekNext() ?? false)) {
break;
}
$previousTargetIdentifierToken = $targetIdentifierToken;
$newTargetNode = $targetNode->getChildByName($targetIdentifierToken->getValue());
if ($newTargetNode === null) {
$newTargetNode = new ChildNode($targetIdentifierToken->getValue());
$targetNode->addChild($newTargetNode);
if (!$foundNode = $targetParentNode->getChildByName($targetTokenName)) {
// Add new node as new child of current last element in path
$foundNode = new ChildNode($targetTokenName);
$targetParentNode->addChild($foundNode);
}
$targetNode = $newTargetNode;
$targetParentNode = $foundNode;
}
return null;

$existingTarget = null;
if ($isSourceNodeValueNull) {
// When the node to copy has no value, but the existing target has,
// the value from the existing target is kept. Also, if the existing
// node is a ReferenceChildNode and the source does not override this,
// source children are added to the existing reference instead of
// dropping the existing target.
$existingTarget = $targetParentNode->getChildByName($targetTokenName);
$existingTargetNodeValue = $existingTarget?->getValue();
} else {
// Blindly remove existing target node if exists and the value is not overridden by source.
$targetParentNode->removeChildByName($targetTokenName);
}
if ($existingTarget instanceof ReferenceChildNode) {
// When existing target is a ReferenceChildNode, keep it and
// copy children from source into existing target.
$targetNode = $existingTarget;
foreach ($sourceNode->getNextChild() as $sourceChild) {
$targetNode->addChild(clone $sourceChild);
}
} else {
// Clone full source node tree, update name and add as child to parent node.
/** @var ChildNodeInterface $targetNode */
$targetNode = clone $sourceNode;
$targetNode->updateName($targetTokenName);
$targetParentNode->addChild($targetNode);
}
if ($isSourceNodeValueNull && $existingTargetNodeValue) {
// If value of old existing target should be kept, set in now.
$targetNode->setValue($existingTargetNodeValue);
}

return $targetNode;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion Classes/TypoScript/AST/AstBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function build(LineStream $lineStream, RootNode $ast, array $flatConstant
$this->handleIdentifierUnsetLine($line, $currentObjectPath);
} elseif ($line instanceof IdentifierCopyLine) {
// "foo < bar": Copy a node source path to a target path
$this->handleIdentifierCopyLine($line, $currentObjectPathStack, $currentObjectPath);
$this->handleIdentifierCopyLine($line, $ast, $currentObjectPath);
} elseif ($line instanceof IdentifierFunctionLine) {
// "foo := addToList(42)": Evaluate functions
$node = $this->getOrAddNodeFromIdentifierStream($currentObjectPath, $line->getIdentifierTokenStream());
Expand Down
2 changes: 1 addition & 1 deletion Classes/TypoScript/AST/CommentAwareAstBuilder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function build(LineStream $lineStream, RootNode $ast, array $flatConstant
$this->handleIdentifierUnsetLine($line, $currentObjectPath);
} elseif ($line instanceof IdentifierCopyLine) {
// "foo < bar": Copy a node source path to a target path
$node = $this->handleIdentifierCopyLine($line, $currentObjectPathStack, $currentObjectPath);
$node = $this->handleIdentifierCopyLine($line, $ast, $currentObjectPath);
if ($node && $previousLineComments) {
foreach ($previousLineComments as $previousLineComment) {
$node->addComment($previousLineComment);
Expand Down
90 changes: 90 additions & 0 deletions Tests/Unit/TypoScript/AST/AstBuilderInterfaceTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1656,6 +1656,96 @@ public static function buildReferenceDataProvider(): \Generator
],
],
];

$expectedAst = new RootNode();
$referenceSourceNode = new ChildNode('referenceSource');
$referenceSourceNode->setValue('referenceSourceValue');
$referenceSourceChildNode = new ChildNode('referenceSourceChild');
$referenceSourceChildNode->setValue('referenceSourceChildValue');
$referenceSourceNode->addChild($referenceSourceChildNode);
$expectedAst->addChild($referenceSourceNode);
$copySourceNode = new ChildNode('copySource');
$copySourceChildNode = new ChildNode('copySourceChild');
$copySourceChildNode->setValue('copySourceChildValue');
$copySourceNode->addChild($copySourceChildNode);
$expectedAst->addChild($copySourceNode);
$targetNode = new ReferenceChildNode('target');
$targetNode->setReferenceSourceStream(
(new IdentifierTokenStream())
->append((new IdentifierToken(TokenType::T_IDENTIFIER, 'referenceSource', 3, 10)))
);
$targetChildNode = new ChildNode('copySourceChild');
$targetChildNode->setValue('copySourceChildValue');
$targetNode->addChild($targetChildNode);
$expectedAst->addChild($targetNode);
yield 'copy operator keeps existing reference node' => [
"referenceSource = referenceSourceValue\n" .
"referenceSource.referenceSourceChild = referenceSourceChildValue\n" .
"copySource.copySourceChild = copySourceChildValue\n" .
"target =< referenceSource\n" .
"target < copySource\n",
$expectedAst,
[
'referenceSource' => 'referenceSourceValue',
'referenceSource.' => [
'referenceSourceChild' => 'referenceSourceChildValue',
],
'copySource.' => [
'copySourceChild' => 'copySourceChildValue',
],
'target' => '< referenceSource',
'target.' => [
'copySourceChild' => 'copySourceChildValue',
],
],
];

$expectedAst = new RootNode();
$referenceSourceNode = new ChildNode('referenceSource');
$referenceSourceNode->setValue('referenceSourceValue');
$referenceSourceChildNode = new ChildNode('referenceSourceChild');
$referenceSourceChildNode->setValue('referenceSourceChildValue');
$referenceSourceNode->addChild($referenceSourceChildNode);
$expectedAst->addChild($referenceSourceNode);
$copySourceNode = new ChildNode('copySource');
$copySourceChildNode = new ChildNode('copySourceChild');
$copySourceChildNode->setValue('copySourceChildValue');
$copySourceNode->addChild($copySourceChildNode);
$expectedAst->addChild($copySourceNode);
$libNode = new ChildNode('lib');
$targetNode = new ReferenceChildNode('target');
$targetNode->setReferenceSourceStream(
(new IdentifierTokenStream())
->append((new IdentifierToken(TokenType::T_IDENTIFIER, 'referenceSource', 3, 14)))
);
$targetChildNode = new ChildNode('copySourceChild');
$targetChildNode->setValue('copySourceChildValue');
$targetNode->addChild($targetChildNode);
$libNode->addChild($targetNode);
$expectedAst->addChild($libNode);
yield 'copy operator keeps existing reference node in nested target' => [
"referenceSource = referenceSourceValue\n" .
"referenceSource.referenceSourceChild = referenceSourceChildValue\n" .
"copySource.copySourceChild = copySourceChildValue\n" .
"lib.target =< referenceSource\n" .
"lib.target < copySource\n",
$expectedAst,
[
'referenceSource' => 'referenceSourceValue',
'referenceSource.' => [
'referenceSourceChild' => 'referenceSourceChildValue',
],
'copySource.' => [
'copySourceChild' => 'copySourceChildValue',
],
'lib.' => [
'target' => '< referenceSource',
'target.' => [
'copySourceChild' => 'copySourceChildValue',
],
],
],
];
}

/**
Expand Down

0 comments on commit 758d744

Please sign in to comment.