Skip to content

Commit

Permalink
Fixed rendering of let and const inside if, for and while that always…
Browse files Browse the repository at this point in the history 
… require brackets. Also refactored the algorithm that detects if brackets are required. Fixes #60
  • Loading branch information
@mck89
mck89 committed Aug 3, 2023
1 parent fa979e5 commit 3f57998
Show file tree
Hide file tree
Showing 11 changed files with 1,977 additions and 51 deletions.
2 changes: 1 addition & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
},
"extra": {
"branch-alias": {
"dev-master": "1.15.2-dev"
"dev-master": "1.15.3-dev"
}
}
}
3 changes: 3 additions & 0 deletions doc/changelog.md
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
Changelog
==========

#### 1.15.3
* Fixed rendering of `let` and `const` inside `if`, `for` and `while` that always require brackets

#### 1.15.2
* Fixed bug where async keyword was lost when rendering an object async method

Expand Down
97 changes: 47 additions & 50 deletions lib/Peast/Renderer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
if ($body->getType() !== "BlockStatement") {
$code .= $this->renderOpts->sao . $this->renderNode($body);
} else {
$code .= $this->renderStatementBlock($body, true);
$code .= $this->renderStatementBlock($node, $body, true);
}
break;
case "AwaitExpression":
Expand Down Expand Up @@ -203,7 +203,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
case "ClassBody":
case "Program":
$code .= $this->renderStatementBlock(
$node->getBody(), false, false, true, false
$node, $node->getBody(), false, false, true, false
);
break;
case "BreakStatement":
Expand Down Expand Up @@ -242,7 +242,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$this->renderOpts->sirb .
")";
}
$code .= $this->renderStatementBlock($node->getBody(), true);
$code .= $this->renderStatementBlock($node, $node->getBody(), true);
break;
case "ChainExpression":
case "ExpressionStatement":
Expand All @@ -258,7 +258,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$code .= " extends " . $this->renderNode($superClass);
}
$code .= $this->renderStatementBlock(
$node->getBody(), true
$node, $node->getBody(), true
);
break;
case "ConditionalExpression":
Expand All @@ -278,7 +278,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
case "DoWhileStatement":
$code .= "do" .
$this->renderStatementBlock(
$node->getBody(), null, true
$node, $node->getBody(), null, true
) .
$this->renderOpts->sao .
"while" .
Expand Down Expand Up @@ -349,7 +349,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$this->renderNode($node->getRight()) .
$this->renderOpts->sirb .
")" .
$this->renderStatementBlock($node->getBody());
$this->renderStatementBlock($node, $node->getBody());
unset($this->renderOpts->forceSingleLine);
break;
case "ForStatement":
Expand All @@ -372,7 +372,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
}
$code .= $this->renderOpts->sirb .
")" .
$this->renderStatementBlock($node->getBody());
$this->renderStatementBlock($node, $node->getBody());
unset($this->renderOpts->forceSingleLine);
break;
case "FunctionDeclaration":
Expand Down Expand Up @@ -403,7 +403,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
) .
$this->renderOpts->sirb .
")" .
$this->renderStatementBlock($node->getBody(), true);
$this->renderStatementBlock($node, $node->getBody(), true);
break;
case "ImportExpression":
$code .= "import(" .
Expand All @@ -424,28 +424,17 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$this->renderNode($node->getTest()) .
$this->renderOpts->sirb .
")";
$consequent = $node->getConsequent();
$alternate = $node->getAlternate();

$forceBracketsConsequent = $forceBracketsAlternate = null;
if (!$this->renderOpts->awb && $alternate) {
$forceBracketsConsequent = $this->checkIfPartsBracketsRequirement(
$consequent
);
$forceBracketsAlternate = $this->checkIfPartsBracketsRequirement(
$alternate
);
}

$code .= $this->renderStatementBlock(
$consequent, $forceBracketsConsequent
$node, $node->getConsequent()
);
if ($alternate) {
if ($alternate = $node->getAlternate()) {
$code .= $this->renderOpts->sao .
"else" .
$this->renderStatementBlock(
$node,
$alternate,
$forceBracketsAlternate,
null,
true
);
}
Expand Down Expand Up @@ -553,7 +542,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$code .= $this->renderNode($node->getLabel()) .
":";
if ($body->getType() === "BlockStatement") {
$code .= $this->renderStatementBlock($body, true);
$code .= $this->renderStatementBlock($node, $body, true);
} else {
$code .= $this->renderOpts->nl .
$this->getIndentation() .
Expand Down Expand Up @@ -738,7 +727,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
break;
case "StaticBlock":
$code .= "static";
$code .= $this->renderStatementBlock($node->getBody(), true);
$code .= $this->renderStatementBlock($node, $node->getBody(), true);
break;
case "Super":
$code .= "super";
Expand All @@ -753,6 +742,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
if (count($node->getConsequent())) {
$code .= $this->renderOpts->nl .
$this->renderStatementBlock(
$node,
$node->getConsequent(),
false
);
Expand All @@ -767,7 +757,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$this->renderOpts->sirb .
")" .
$this->renderStatementBlock(
$node->getCases(), true, false, false
$node, $node->getCases(), true, false, false
);
break;
case "TaggedTemplateExpression":
Expand Down Expand Up @@ -796,15 +786,15 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
break;
case "TryStatement":
$code .= "try" .
$this->renderStatementBlock($node->getBlock(), true);
$this->renderStatementBlock($node, $node->getBlock(), true);
if ($handler = $node->getHandler()) {
$code .= $this->renderOpts->sao .
$this->renderNode($handler);
}
if ($finalizer = $node->getFinalizer()) {
$code .= $this->renderOpts->sao .
"finally" .
$this->renderStatementBlock($finalizer, true);
$this->renderStatementBlock($node, $finalizer, true);
}
break;
case "UnaryExpression":
Expand Down Expand Up @@ -855,7 +845,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$this->renderNode($node->getTest()) .
$this->renderOpts->sirb .
")" .
$this->renderStatementBlock($node->getBody());
$this->renderStatementBlock($node, $node->getBody());
break;
case "WithStatement":
$code .= "with" .
Expand All @@ -865,7 +855,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
$this->renderNode($node->getObject()) .
$this->renderOpts->sirb .
")" .
$this->renderStatementBlock($node->getBody());
$this->renderStatementBlock($node, $node->getBody());
break;
case "YieldExpression":
$code .= "yield";
Expand All @@ -888,7 +878,8 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)

/**
* Renders a node as a block statement
*
*
* @param Syntax\Node\Node $parent Parent node
* @param Syntax\Node\Node|array $node Node or array of
* nodes to render
* @param bool $forceBrackets Overrides brackets
Expand All @@ -907,7 +898,7 @@ protected function renderNode(Syntax\Node\Node $node, $addSemicolon = false)
* @return string
*/
protected function renderStatementBlock(
$node, $forceBrackets = null, $mandatorySeparator = false,
$parent, $node, $forceBrackets = null, $mandatorySeparator = false,
$addSemicolons = true, $incIndent = true
) {
$code = "";
Expand All @@ -928,8 +919,7 @@ protected function renderStatementBlock(
} else {
//Insert curly brackets if required by the formatter or if the
//number of nodes to render is different from one
$hasBrackets = $this->renderOpts->awb ||
(is_array($node) && count($node) !== 1);
$hasBrackets = $this->renderOpts->awb || $this->needsBrackets($parent, $node);
}
$currentIndentation = $this->getIndentation();

Expand Down Expand Up @@ -1038,46 +1028,53 @@ protected function joinNodes($nodes, $separator, $addSemicolons=false)
}

/**
* Checks if consequent and alternate nodes of an IfStatement require the
* mandatory insertion of brackets around them to avoid the pattern where
* the "else" code is rendered as part of an inner IfStatement.
*
* @param Syntax\Node\Node $node Node
* Check if the node or the array of nodes need brackets to be rendered
*
* @param Syntax\Node\Node $parent Parent node
* @param Syntax\Node\Node|array $node Node or array of
* nodes to render
*
* @return null|bool
* @return bool
*/
protected function checkIfPartsBracketsRequirement($node)
protected function needsBrackets($parent, $node)
{
if ($node->getType() === "BlockStatement" && count($node->getBody()) > 1) {
return null;
if (is_array($node) && count($node) !== 1) {
return true;
}
$forceBrackets = null;
$node = is_array($node) ? $node[0] : $node;
$addBrackets = false;
$optBracketNodes = array(
"DoWhileStatement", "ForInStatement", "ForOfStatement",
"ForStatement", "WhileStatement", "WithStatement"
);
$checkFn = function ($n) use ($optBracketNodes, &$forceBrackets) {
$inIfWithElse = $parent->getType() === "IfStatement" && $parent->getAlternate();
$checkFn = function ($n) use ($inIfWithElse, $optBracketNodes, &$addBrackets) {
$type = $n->getType();
if ($type === "IfStatement") {
if ($inIfWithElse && $type === "IfStatement") {
if (!$n->getAlternate()) {
$forceBrackets = true;
$addBrackets = true;
}
return Traverser::DONT_TRAVERSE_CHILD_NODES;
} elseif ($type === "BlockStatement") {
if (count($n->getBody()) !== 1) {
return Traverser::DONT_TRAVERSE_CHILD_NODES;
}
} elseif ($type === "LabeledStatement") {
} elseif ($inIfWithElse && $type === "LabeledStatement") {
if ($n->getBody()->getType() === "BlockStatement") {
$forceBrackets = true;
$addBrackets = true;
}
return Traverser::DONT_TRAVERSE_CHILD_NODES;
} elseif ($type === "VariableDeclaration") {
if (in_array($n->getKind(), array($n::KIND_LET, $n::KIND_CONST))) {
$addBrackets = true;
}
return Traverser::DONT_TRAVERSE_CHILD_NODES;
} elseif (!in_array($type, $optBracketNodes)) {
return Traverser::DONT_TRAVERSE_CHILD_NODES;
}
};
$node->traverse($checkFn);
return $forceBrackets;
return $addBrackets;
}

/**
Expand Down
1 change: 1 addition & 0 deletions test/Peast/Syntax/ES2015/files/ForStatement/Invalid18.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
for (;;) const test = true;
1 change: 1 addition & 0 deletions test/Peast/Syntax/ES2015/files/VariableDeclarations/Invalid4.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
while (true) const test = true;
1 change: 1 addition & 0 deletions test/Peast/Syntax/ES2015/files/VariableDeclarations/Invalid5.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
if (true) const test = true;
0 ...ntax/ES2015/files/IfStatement/Invalid4.js → ...15/files/VariableDeclarations/Invalid6.js
View file
File renamed without changes.
45 changes: 45 additions & 0 deletions test/Peast/Syntax/ES2015/files/VariableDeclarations/LetConstInIfForWhile.Render.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
if (true) {
const test = true;
}
for (; ; ) {
const test = true;
}
while (true) {
const test = true;
}
if (true) {
let test = true;
}
for (; ; ) {
let test = true;
}
while (true) {
let test = true;
}
/**************************************************/
if(true){const test=true;}for(;;){const test=true;}while(true){const test=true;}if(true){let test=true;}for(;;){let test=true;}while(true){let test=true;}
/**************************************************/
if ( true )
{
const test = true;
}
for ( ; ; )
{
const test = true;
}
while ( true )
{
const test = true;
}
if ( true )
{
let test = true;
}
for ( ; ; )
{
let test = true;
}
while ( true )
{
let test = true;
}
Loading

0 comments on commit 3f57998

Please sign in to comment.