jscodeshift icon indicating copy to clipboard operation
jscodeshift copied to clipboard

Missing parentheses around LogicalExpression inside of UnaryExpression

Open elipsitz opened this issue 3 years ago • 6 comments

I believe I've stumbled onto a bug in jscodeshift: I created a transform to convert

(a && b) || c();

to

if (!(a && b)) {
  c();
}

However, when I wrap the initial LogicalExpression in a ! UnaryExpression, jscodeshift instead outputs:

if (!a && b) {
  c();
}

which has a different meaning due to the missing parentheses.

The issue on AST explorer: https://astexplorer.net/#/gist/4f8d333d868a270bd53098d4f283e8fa/5cb2224e8c7cbc178130c3da2ab6683e4a1064bc

Output of jscodeshift --version:

jscodeshift: 0.11.0
 - babel: 7.13.14
 - babylon: 7.13.13
 - flow: 0.148.0
 - recast: 0.20.4

I don't believe this is an issue with recast, because I wrote some code that does the same thing purely using recast, and it works:

var recast = require("recast");
const b = recast.types.builders;

const code = [
  "(a && b) || c();"
].join("\n");

const ast = recast.parse(code);
const expression = ast.program.body[0].expression;
const body = b.blockStatement([b.expressionStatement(expression.right)]);
const cond = b.unaryExpression("!", expression.left);
const ifStatement = b.ifStatement(cond, body, null);
ast.program.body[0] = ifStatement;
const output = recast.print(ast).code;

console.log(output);

outputs:

if (!(a && b)) {
  c();
}

elipsitz avatar Apr 05 '21 04:04 elipsitz

Maybe this issue is related:

https://github.com/facebook/jscodeshift/issues/442

fcsonline avatar Nov 02 '21 23:11 fcsonline

I have the feeling that it is related to recast. Check this out:

https://github.com/benjamn/recast/pull/923

And also:

https://github.com/benjamn/recast/pulls?q=is%3Apr+is%3Aopen+parenthesis

fcsonline avatar Nov 03 '21 10:11 fcsonline

@fcsonline I think what you need is adding node.left.prefix = true

https://astexplorer.net/#/gist/4f8d333d868a270bd53098d4f283e8fa/b2b9239fb3c2281723659f4d55a8efb944919d51

export default function transformer(file, api) {
  const j = api.jscodeshift;
  var program = j(file.source);

  program
    .find(j.ExpressionStatement, {
      expression: {
        type: "LogicalExpression",
      },
    })
    .replaceWith((p) => {
      var node = p.node.expression;
      if (node.operator == "||") {
        node.left.prefix = true
        var expr = j.unaryExpression("!", node.left, true);
        var then = j.blockStatement([j.expressionStatement(node.right)]);
        return j.ifStatement(expr, then, null);
      } else {
        return p;
      }
    });

  return program.toSource();
}

mobily avatar Dec 11 '21 20:12 mobily

Thanks for the information @mobily !!. Do you have a link to the documentation related to this?

fcsonline avatar Dec 11 '21 22:12 fcsonline

unfortunately, I don't 😢 (I just remember this, because I had a similar case in one of my codemods)

mobily avatar Dec 11 '21 23:12 mobily

I mean, I think this is quite important to be documented because it can break you code without being aware of it. When I saw this thread it was the first time I have seen that a codemod can have unexpected side effects.

fcsonline avatar Dec 13 '21 19:12 fcsonline