Skip to content
This repository has been archived by the owner on May 19, 2024. It is now read-only.

Commit

Permalink
Fixed code quality issues
Browse files Browse the repository at this point in the history
  • Loading branch information
MewesK committed Feb 13, 2016
1 parent d76c982 commit 6cd78ae
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 156 deletions.
2 changes: 1 addition & 1 deletion Tests/Twig/AbstractTwigTest.php
Expand Up @@ -47,7 +47,7 @@ protected function getDocument($templateName, $format)
$requestStack = new RequestStack();
$requestStack->push($request);

$appVariable = new AppVariable($format);
$appVariable = new AppVariable();
$appVariable->setRequestStack($requestStack);

// generate source from template
Expand Down
4 changes: 2 additions & 2 deletions Twig/Node/XlsNode.php
Expand Up @@ -12,10 +12,10 @@ abstract class XlsNode extends \Twig_Node
/**
* @return string[]
*/
public abstract function getAllowedParents();
abstract public function getAllowedParents();

/**
* @return bool
*/
public abstract function canContainText();
abstract public function canContainText();
}
111 changes: 111 additions & 0 deletions Twig/NodeHelper.php
@@ -0,0 +1,111 @@
<?php

namespace MewesK\TwigExcelBundle\Twig;

use MewesK\TwigExcelBundle\Twig\Node\XlsNode;
use Twig_Error_Syntax;
use Twig_Node;
use Twig_Node_Block;
use Twig_Node_BlockReference;
use Twig_Node_Expression_MethodCall;
use Twig_Node_Expression_Name;
use Twig_Node_Text;
use Twig_Parser;

/**
* Class NodeHelper
*
* @package MewesK\TwigExcelBundle\Twig\TokenParser
*/
class NodeHelper
{
/**
* @param Twig_Node $node
*/
public static function fixMacroCallsRecursively(Twig_Node $node)
{
foreach ($node->getIterator() as $key => $subNode) {
if ($subNode instanceof Twig_Node_Expression_MethodCall) {
/**
* @var \Twig_Node_Expression_Array $argumentsNode
*/
$argumentsNode = $subNode->getNode('arguments');
$argumentsNode->addElement(new Twig_Node_Expression_Name('phpExcel', null), null);
} elseif ($subNode instanceof Twig_Node && $subNode->count() > 0) {
self::fixMacroCallsRecursively($subNode);
}
}
}

/**
* @param Twig_Node $node
* @param Twig_Parser $parser
*/
public static function removeTextNodesRecursively(Twig_Node $node, Twig_Parser $parser)
{
foreach ($node->getIterator() as $key => $subNode) {
if ($subNode instanceof Twig_Node_Text) {
// Never delete a block body
if ($key === 'body' && $node instanceof Twig_Node_Block) {
continue;
}

$node->removeNode($key);
} elseif ($subNode instanceof Twig_Node_BlockReference) {
self::removeTextNodesRecursively($parser->getBlock($subNode->getAttribute('name')), $parser);
} elseif ($subNode instanceof Twig_Node && $subNode->count() > 0) {
if ($subNode instanceof XlsNode && $subNode->canContainText()) {
continue;
}
self::removeTextNodesRecursively($subNode, $parser);
}
}
}

/**
* @param XlsNode $node
* @param array $path
* @throws Twig_Error_Syntax
*/
public static function checkAllowedParents(XlsNode $node, array $path)
{
$parentName = null;

foreach (array_reverse($path) as $className) {
if (strpos($className, 'MewesK\TwigExcelBundle\Twig\Node\Xls') === 0) {
$parentName = $className;
break;
}
}

if ($parentName === null) {
return;
}

foreach ($node->getAllowedParents() as $className) {
if ($className === $parentName) {
return;
}
}

throw new Twig_Error_Syntax(sprintf('Node "%s" is not allowed inside of Node "%s".', get_class($node), $parentName));
}

/**
* @param Twig_Node $node
* @return bool
*/
public static function checkContainsXlsNode(Twig_Node $node)
{
foreach ($node->getIterator() as $key => $subNode) {
if ($node instanceof XlsNode) {
return true;
} elseif ($subNode instanceof Twig_Node && $subNode->count() > 0) {
if (self::checkContainsXlsNode($subNode)) {
return true;
}
}
}
return false;
}
}
65 changes: 9 additions & 56 deletions Twig/NodeVisitor/SyntaxCheckNodeVisitor.php
Expand Up @@ -3,6 +3,7 @@
namespace MewesK\TwigExcelBundle\Twig\NodeVisitor;

use MewesK\TwigExcelBundle\Twig\Node\XlsNode;
use MewesK\TwigExcelBundle\Twig\NodeHelper;
use Twig_BaseNodeVisitor;
use Twig_Environment;
use Twig_Error_Syntax;
Expand All @@ -29,7 +30,7 @@ class SyntaxCheckNodeVisitor extends Twig_BaseNodeVisitor
*/
protected function doEnterNode(Twig_Node $node, Twig_Environment $env)
{
if (($node instanceof Twig_Node_Block || $node instanceof Twig_Node_Macro) && !$node->hasAttribute('twigExcelBundle') && $this->checkContainsXlsNode($node)) {
if (($node instanceof Twig_Node_Block || $node instanceof Twig_Node_Macro) && !$node->hasAttribute('twigExcelBundle') && NodeHelper::checkContainsXlsNode($node)) {
if ($node instanceof Twig_Node_Block) {
throw new Twig_Error_Syntax('Block tags do not work together with Twig tags provided by TwigExcelBundle. Please use \'xlsblock\' instead.');
}
Expand All @@ -41,7 +42,13 @@ protected function doEnterNode(Twig_Node $node, Twig_Environment $env)
/**
* @var XlsNode $node
*/
$this->checkAllowedParents($node, $node->getAllowedParents());
try {
NodeHelper::checkAllowedParents($node, $this->path);
} catch(Twig_Error_Syntax $e) {
// reset path since throwing an error prevents doLeaveNode to be called
$this->path = [];
throw $e;
}
}

$this->path[] = get_class($node);
Expand All @@ -66,58 +73,4 @@ public function getPriority()
{
return 0;
}

//
// Helper
//

/**
* @param Twig_Node $node
* @param array $allowedParents
*
* @throws Twig_Error_Syntax
*/
protected function checkAllowedParents(Twig_Node $node, array $allowedParents)
{
$parentName = null;

foreach (array_reverse($this->path) as $className) {
if (strpos($className, 'MewesK\TwigExcelBundle\Twig\Node\Xls') === 0) {
$parentName = $className;
break;
}
}

if ($parentName === null) {
return;
}

foreach ($allowedParents as $className) {
if ($className === $parentName) {
return;
}
}

// reset path since throwing an error prevents doLeaveNode to be called
$this->path = [];
throw new Twig_Error_Syntax(sprintf('Node "%s" is not allowed inside of Node "%s".', get_class($node), $parentName));
}

/**
* @param Twig_Node $node
* @return bool
*/
private function checkContainsXlsNode(Twig_Node $node)
{
foreach ($node->getIterator() as $key => $subNode) {
if ($node instanceof XlsNode) {
return true;
} elseif ($subNode instanceof Twig_Node && $subNode->count() > 0) {
if ($this->checkContainsXlsNode($subNode)) {
return true;
}
}
}
return false;
}
}
33 changes: 0 additions & 33 deletions Twig/TokenParser/Traits/FixMacroCallsTrait.php

This file was deleted.

45 changes: 0 additions & 45 deletions Twig/TokenParser/Traits/RemoveTextNodeTrait.php

This file was deleted.

13 changes: 6 additions & 7 deletions Twig/TokenParser/XlsBlockTokenParser.php
Expand Up @@ -2,8 +2,7 @@

namespace MewesK\TwigExcelBundle\Twig\TokenParser;

use MewesK\TwigExcelBundle\Twig\TokenParser\Traits\FixMacroCallsTrait;
use MewesK\TwigExcelBundle\Twig\TokenParser\Traits\RemoveTextNodeTrait;
use MewesK\TwigExcelBundle\Twig\NodeHelper;
use Twig_Error_Syntax;
use Twig_Node_Block;
use Twig_Node_BlockReference;
Expand All @@ -17,9 +16,6 @@
*/
class XlsBlockTokenParser extends Twig_TokenParser_Block
{
use FixMacroCallsTrait;
use RemoveTextNodeTrait;

/**
* @param Twig_Token $token
* @return Twig_Node_BlockReference
Expand All @@ -31,11 +27,14 @@ public function parse(Twig_Token $token)
* @var Twig_Node_BlockReference $blockReference
*/
$blockReference = parent::parse($token);
/**
* @var Twig_Node_Block $block
*/
$block = $this->parser->getBlock($blockReference->getAttribute('name'));

// prepare block
$this->removeTextNodesRecursively($block);
$this->fixMacroCallsRecursively($block);
NodeHelper::removeTextNodesRecursively($block, $this->parser);
NodeHelper::fixMacroCallsRecursively($block);

// mark for syntax checks
foreach ($block->getIterator() as $node) {
Expand Down
10 changes: 3 additions & 7 deletions Twig/TokenParser/XlsDocumentTokenParser.php
Expand Up @@ -3,8 +3,7 @@
namespace MewesK\TwigExcelBundle\Twig\TokenParser;

use MewesK\TwigExcelBundle\Twig\Node\XlsDocumentNode;
use MewesK\TwigExcelBundle\Twig\TokenParser\Traits\FixMacroCallsTrait;
use MewesK\TwigExcelBundle\Twig\TokenParser\Traits\RemoveTextNodeTrait;
use MewesK\TwigExcelBundle\Twig\NodeHelper;
use Twig_Token;

/**
Expand All @@ -14,9 +13,6 @@
*/
class XlsDocumentTokenParser extends AbstractTokenParser
{
use FixMacroCallsTrait;
use RemoveTextNodeTrait;

/**
* @var bool
*/
Expand Down Expand Up @@ -50,8 +46,8 @@ public function parse(Twig_Token $token)

// parse body
$body = $this->parseBody();
$this->removeTextNodesRecursively($body);
$this->fixMacroCallsRecursively($body);
NodeHelper::removeTextNodesRecursively($body, $this->parser);
NodeHelper::fixMacroCallsRecursively($body);

// return node
return new XlsDocumentNode($properties, $body, $token->getLine(), $this->getTag(), $this->preCalculateFormulas, $this->diskCachingDirectory);
Expand Down

0 comments on commit 6cd78ae

Please sign in to comment.