Closed lucivpav closed 3 years ago
@micschro
arrow functions are affected too! https://github.com/estools/escodegen/issues/426#issuecomment-707540314 Solved.
Maybe these kind of bugs should be treated from another point of view - if a parenthesis expression contains comments, then the parenthesis must stay.
@lucivpav the new test is failing...
It is failing on purpose, because I did not implement a fix of arrow functions in this PR yet :)
another scenario that's related to broken parens
(1).toString()
is generated as
1.toString()
which is invalid js :(
1.toString()
Not sure if you're saying that's broken on this branch or in the latest release, but it's not reproducible on master:
> require('escodegen').generate({ type: 'CallExpression', callee: { type: 'MemberExpression', object: { type: 'Literal', value: 1 }, property: { type: 'Identifier', name: 'toString' }, computed: false, optional: false }, arguments: [], optional: false })
'1 .toString()'
1.toString()
Not sure if you're saying that's broken on this branch or in the latest release, but it's not reproducible on master:
require('escodegen').generate({ type: 'CallExpression', callee: { type: 'MemberExpression', object: { type: 'Literal', value: 1 }, property: { type: 'Identifier', name: 'toString' }, computed: false, optional: false }, arguments: [], optional: false }) '1 .toString()'
the output should wrap the Literal number with parens, running the generated code fails with a syntax error
Did you notice the space between .
and toString
? That makes it run.
@papandreou my bad. although I still think this is incorrect behavior is incorrect but it's not very important
You're of course entitled to your opinion, but why do you think it's incorrect? ;)
the behavior isn't consistent. for example:
(1).toString(); // outputs "1"
(1+2).toString(); // outputs "3"
1 + 2 .toString(); // outputs "12"
is converted to
1 .toString(); // outputs "1"
(1 + 2).toString(); // outputs "3"
1 + 2 .toString(); // outputs "12"
maybe this is just a js thing that is confusing...
maybe this is just a js thing that is confusing...
Yeah, I think those examples are JavaScript being weird more than escodegen :)
@michaelficarra could you please take a look at this PR, when you have time?
Do not merge as it is now. I discovered a bug in cases like these:
class MyClass {
/**
* description
*/
foo(a, b) {
a.bar(b);
}
}
Fixes issue #365 (maybe more).
Changes: