facebook / jscodeshift

A JavaScript codemod toolkit.
https://jscodeshift.com
MIT License
9.23k stars 477 forks source link

Closing parenthesis of conditional spread removed erroneously #267

Open dunnbobcat opened 6 years ago

dunnbobcat commented 6 years ago

I've got a transform to remove empty object spreads like ...{}:

module.exports = {
  default: function(file, api, options) {
    const j = api.jscodeshift;
    const ast = j(file.source);

    ast
      .find(j.SpreadProperty, {argument: {type: 'ObjectExpression'}})
      .filter(p => p.node.argument.properties.length === 0)
      .remove();

    return ast.toSource();
  },
  parser: 'flow',
};

I'm running it against the following code:

let cond = false;
let obj = {
  ...(cond ? {x: 4} : {y: 5}),
  ...{},
};

The result of the transform is as follows:

let cond = false;
let obj = {
  ...(cond ? {x: 4} : {y: 5}
};

You can see that the closing parenthesis of the conditional spread was removed, resulting in broken syntax. I get the same result if I move the empty object spread above the conditional spread.

I attempted to reproduce the issue in recast (0.15.0) directly:

const recast = require('recast');
const flow = require('recast/parsers/flow');
const builders = recast.types.builders;

recast.run(
  function(ast, callback) {
    recast.visit(ast, {
      visitSpreadElement: function(path) {
        const arg = path.node.argument;
        if (arg.type === 'ObjectExpression' && arg.properties.length === 0) {
          path.prune();
          return false;
        }
        this.traverse(path);
      },
    });

    callback(ast);
  },
  {
    parser: flow
  }
);

But its output is correct:

let cond = false;
let obj = {
  ...(cond ? {x: 4} : {y: 5})
};
fkling commented 6 years ago

Which recast version is your jscodeshift installation using (jscodeshift --version should tell you).

dunnbobcat commented 6 years ago
> jscodeshift --version
jscodeshift: 0.5.1
 - babel: 6.26.3
 - babylon: 7.0.0-beta.47
 - flow: 0.74.0
 - recast: 0.15.0
rubennorte commented 6 years ago

You can reproduce the issue with AST explorer: https://astexplorer.net/#/gist/8269fb593017340ed9486d0c57a0a3ff/f30217f92bb6d197ef0bbfdb7eb70c006d9eca6a

dunnbobcat commented 6 years ago

Any updates on this?

dunnbobcat commented 6 years ago

@fkling, have you had a chance to debug this?

kpsuperplane commented 6 years ago

@fkling @dunnbobcat I've reproduced this issue and it only seems to happen with the flow parser. Running jscodeshift with the vanilla parser works fine.

dunnbobcat commented 5 years ago

Friendly ping

fkling commented 5 years ago

This seems to work for as expected in the latest version (v0.6.0), after updating recast. Could you confirm please?