Skip to content

Commit bb30704

Browse files
author
Pavel Lučivňák
committed
revert logic to simplify PR; only parenthesize at a level where it is necessary; improve tests
1 parent 873307f commit bb30704

7 files changed

Lines changed: 114 additions & 124 deletions

‎escodegen.js‎

Lines changed: 43 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,11 @@
240240
return str.replace(/\/\*[\s\S]*?\*\/|\/\/.*/g, '');
241241
}
242242

243+
function startsWithComment(result) {
244+
var str = toSourceNodeWhenNeeded(result).toString();
245+
return (/^\/\*[\s\S]*?\*\/|^\/\/.*/).test(str);
246+
}
247+
243248
function removeTrailingWhiteSpaces(str) {
244249
return str.replace(/\S(\s+)$/g, '');
245250
}
@@ -249,17 +254,17 @@
249254
str = removeTrailingWhiteSpaces(str);
250255
var counter = 0;
251256
for (var i = 0; i < str.length; ++i) {
252-
if (str[i] == leftPar) {
257+
if (str[i] === leftPar) {
253258
counter++;
254259
}
255-
else if (str[i] == rightPar) {
256-
if (counter == 0) {
260+
else if (str[i] === rightPar) {
261+
if (counter === 0) {
257262
return false;
258263
}
259264
counter--;
260265
}
261266
}
262-
return counter == 0 && str[str.length-1] == rightPar;
267+
return counter === 0 && str[0] === leftPar && str[str.length-1] === rightPar;
263268
}
264269

265270
function isParenthesizedByAnyBracketKind(str) {
@@ -272,40 +277,18 @@
272277
if (!hasLineTerminator(str)) {
273278
return false;
274279
}
275-
if (path.parent !== null && (
276-
path.parent.node.type === Syntax.ObjectExpression ||
277-
path.parent.node.type === Syntax.ArrayExpression ||
278-
path.parent.node.type === Syntax.Property ||
279-
path.parent.node.type === Syntax.ImportExpression
280-
)) {
281-
return false;
282-
}
283-
if (path.parent !== null &&
284-
path.node.type === Syntax.ImportSpecifier &&
285-
path.parent.node.type === Syntax.ImportDeclaration
286-
) {
280+
if (path.parent === null) {
287281
return false;
288282
}
289-
if (path.parent !== null && path.parent.parent !== null &&
290-
path.parent.parent.node.type === Syntax.MethodDefinition &&
291-
path.parent.node.type === Syntax.FunctionExpression &&
292-
path.node.type === Syntax.AssignmentPattern) {
293-
return false;
294-
}
295-
if (stmt.type === Syntax.MethodDefinition) {
283+
var parentNodeType = path.parent.node.type;
284+
if (parentNodeType !== Syntax.ReturnStatement &&
285+
parentNodeType !== Syntax.ThrowStatement &&
286+
parentNodeType !== Syntax.ArrowFunctionExpression) {
296287
return false;
297-
}
298-
if (path.parent !== null && path.parent.node.type === Syntax.VariableDeclaration) {
288+
};
289+
if (isParenthesizedByAnyBracketKind(str))
299290
return false;
300-
}
301-
if (!isParenthesizedByAnyBracketKind(str)) {
302-
return true;
303-
}
304-
305-
str = removeComments(str);
306-
var firstNewlineIdx = str.indexOf(newline);
307-
var firstParenthesisIdx = str.match(/[\(\[\{)]/).index;
308-
return firstNewlineIdx < firstParenthesisIdx;
291+
return true;
309292
}
310293

311294
function merge(target, override) {
@@ -752,10 +735,11 @@
752735

753736
function addComments(stmt, result, path) {
754737
var i, len, comment, save, tailingToStatement, specialBase, fragment,
755-
extRange, range, prevRange, prefix, infix, suffix, count;
738+
extRange, range, prevRange, prefix, infix, suffix, count,
739+
generatedLeadingComments, hasLeadingComments;
740+
save = result;
756741

757742
if (stmt.leadingComments && stmt.leadingComments.length > 0) {
758-
save = result;
759743

760744
if (preserveBlankLines) {
761745
comment = stmt.leadingComments[0];
@@ -813,26 +797,34 @@
813797
result.push(addIndent(fragment));
814798
}
815799
}
800+
generatedLeadingComments = true;
801+
}
816802

817-
if (!isExpression(stmt)) {
818-
result.push(addIndent(save));
819-
} else {
820-
var text = toSourceNodeWhenNeeded(result).toString();
821-
if (shouldParenthesize(text, stmt, path)) {
822-
result = addMultiLineIndent(result);
823-
result = [indent, result];
803+
hasLeadingComments = generatedLeadingComments || startsWithComment(result);
804+
var text = toSourceNodeWhenNeeded(result).toString();
805+
var parenthesize = shouldParenthesize(text, stmt, path);
824806

825-
result = ['(', newline, result];
807+
if (!hasLeadingComments) {
808+
result = save;
809+
} else if (hasLeadingComments && parenthesize) {
810+
if (generatedLeadingComments) {
811+
result = addMultiLineIndent(result);
812+
result = [indent, result];
826813

827-
withIndent(function () {
828-
result.push(addMultiLineIndent(save));
829-
});
814+
result = ['(', newline, result];
830815

831-
result.push([newline, base, ')']);
832-
} else {
833-
result.push(addIndent(save));
834-
}
816+
withIndent(function () {
817+
result.push(addMultiLineIndent(save));
818+
});
819+
820+
result.push([newline, base, ')']);
821+
} else {
822+
result = ['(', newline, indent, addMultiLineIndent(result), newline, base, ')'];
835823
}
824+
} else if (generatedLeadingComments) {
825+
result.push(addIndent(save));
826+
} else {
827+
result = save;
836828
}
837829

838830
if (stmt.trailingComments) {

‎test/comment/comment-as-first-element-in-parenthesis-expression.expected.js‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ function foo() {
4545
function foo() {
4646
return (
4747
// one
48-
3 + 3
49-
) - (
50-
// two
48+
3 + 3 - // two
5149
(1 + 1)
5250
);
5351
}
@@ -60,8 +58,8 @@ function foo(a, b, c) {
6058
function foo(a, b, c) {
6159
return (
6260
// comment
63-
a >= b && a <= c
64-
) || a === 42 || a === 666;
61+
a >= b && a <= c || a === 42 || a === 666
62+
);
6563
}
6664
function foo(a, b, c) {
6765
throw (

‎test/comment/variable-declaration.expected.js‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,27 @@
1-
let variable = (
2-
// one
3-
3 + 3
4-
) - (
5-
// two
6-
(1 + 1)
7-
);
1+
let variable = // comment
2+
3 + 3;
3+
let variable = // comment
4+
3 + 3;
5+
let variable = /* comment */
6+
3 + 3;
7+
let variable = /* comment
8+
comment
9+
comment
10+
*/
11+
3 + 3;
12+
let variable = // comment
13+
/* comment */
14+
// comment
15+
3 + 3;
16+
let variable = /* comment */
17+
// comment
18+
3 + 3;
19+
let variable = // one
20+
3 + 3 - // two
21+
(1 + 1);
22+
let age = // comment
23+
42,
24+
// comment
25+
height = 165;
26+
let variable = // comment
27+
a >= b && a <= c || a === 42 || a === 666;

‎test/comment/variable-declaration.expected.js.todo‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎test/comment/variable-declaration.js‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,29 @@
1+
let variable =
2+
// comment
3+
3+3;
4+
5+
let variable = // comment
6+
3+3;
7+
8+
let variable = /* comment */
9+
3+3;
10+
11+
let variable = /* comment
12+
comment
13+
comment
14+
*/
15+
3+3;
16+
17+
let variable =
18+
// comment
19+
/* comment */
20+
// comment
21+
3+3;
22+
23+
let variable = /* comment */
24+
// comment
25+
3+3;
26+
127
let variable =
228
(
329
// one
@@ -6,4 +32,16 @@ let variable =
632
(
733
// two
834
1+1
9-
)
35+
);
36+
37+
let age = // comment
38+
42,
39+
// comment
40+
height = 165;
41+
42+
let variable = (
43+
( // comment
44+
a >= b &&
45+
a <= c)
46+
|| a === 42 || a === 666
47+
);

‎test/comment/variable-declaration.js.expected.todo‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// TOOD: https://github.com/estools/escodegen/issues/435
2+
// the existing non-todo test expectation shall be adjusted accordingly once the issue is resolved

‎test/comment/variable-declaration.js.todo‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)