Skip to content

Commit

Permalink
Fix the buggy code dealing with nested if's
Browse files Browse the repository at this point in the history
Summary: Old code only deals with
         if () {
           if ()
         } else {}

         but does not handle following case properly
         if () {
           for () {
             if ()
           }
         } else {}

         This patch has a quick fix that always put braces around TRUE
         statements if there is an else part.

Reviewed By: rdbayer

Test Plan: The code triggers this bug is in html/js/lib/ui/sort.js

           See the reduced test case, and tested on my sandbox, it works
           fine.

Revert: OK

DiffCamp Revision: 70592
  • Loading branch information
Feng Qian committed Oct 23, 2009
1 parent 42ba4fb commit f2579af
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 20 deletions.
33 changes: 13 additions & 20 deletions node.cpp
Expand Up @@ -977,35 +977,28 @@ rope_t NodeIf::render(render_guts_t* guts, int indentation) const {
ret += (*node)->render(guts, indentation);
ret += ")";

// If rendering a nested if with no else then {}'s are required, hacky :/
++node;
bool needBraces = guts->pretty || (*node)->childNodes().empty();
if (!needBraces) {
Node* firstStatement = (*node)->childNodes().front();
while (dynamic_cast<NodeStatementList*>(firstStatement) != NULL) {
firstStatement = firstStatement->childNodes().front();
}
if (dynamic_cast<NodeIf*>(firstStatement) != NULL) {
if (firstStatement->childNodes().back() == NULL) {
needBraces = true;
}
}
}
ret += (*node)->renderBlock(needBraces, guts, indentation);
// Currently we need braces if it has else statement
// TODO: braces are not needed if no nested-if statement.
Node* ifBlock = *++node;
Node* elseBlock = *++node;

bool needBraces = guts->pretty || ifBlock->childNodes().empty()
|| elseBlock != NULL;
ret += ifBlock->renderBlock(needBraces, guts, indentation);

// Render else
if (*++node != NULL) {
if (elseBlock != NULL) {
ret += guts->pretty ? " else" : "else";

// Special-case for rendering else if's
if (typeid(**node) == typeid(NodeIf)) {
if (typeid(*elseBlock) == typeid(NodeIf)) {
if (guts->sanelineno) {
(*node)->renderLinenoCatchup(guts, ret);
elseBlock->renderLinenoCatchup(guts, ret);
}
ret += " ";
ret += (*node)->render(guts, indentation);
ret += elseBlock->render(guts, indentation);
} else {
rope_t block = (*node)->renderBlock(false, guts, indentation);
rope_t block = elseBlock->renderBlock(false, guts, indentation);
if (block[0] != '{' && block[0] != ' ') {
ret += " ";
}
Expand Down
11 changes: 11 additions & 0 deletions tests/missing-brace-2.js
@@ -0,0 +1,11 @@
function foo(a, b) {
if (a) {
if (b) {
print('b');
}
} else {
print('a');
}
}

foo(true, false);
14 changes: 14 additions & 0 deletions tests/missing-brace.js
@@ -0,0 +1,14 @@
function foo(a, b) {
if (a) {
for (var i = 0; i < 1; i++) {
if (!a)
break;
else if (b)
print('hello');
}
} else {
print('ok');
}
}

foo(true, false);

0 comments on commit f2579af

Please sign in to comment.