{type: Noop} does not match type string

[codemod-js]

{type: Noop} does not match type string

[mcMickJuice]

Would you please post your plugin code here so we can try to repro?

[MagicDuck]

hi @mcMickJuice,

Thanks for looking into this issue and thanks for the awesome tool. I feel so much power running it over 2000 legacy js files and watching it change them, muhahaha :)

Regarding t.noop(), here's the AST Explorer saved session that also exhibits the behaviour: https://astexplorer.net/#/gist/f414b2747916f09125b4a467e5b6ae19/latest

When I ran this type of stuff manually/locally using babel.transform() with [email protected], I remember it working correctly.

The babel docs also document t.noop: https://babeljs.io/docs/core-packages/babel-types/#apinoop

[mcMickJuice]

all props go to @eventualbuddha as he is the project creator and maintainer. I'm just helping out.

I can look at this sometime this week and get back to you. Hopefully its just a version issue.

[eventualbuddha]

@MagicDuck codemod now uses babel 7.x, but this problem seems to still exist. I think it's actually a recast issue as the assertion that fails is in recast. I think recast needs to be taught about Noop nodes. @benjamn, what do you think?

Encountered an error while processing test.js:
Error: {type: Noop} does not match type string
    at Type.Tp.assert (/Users/donovan/src/babel-codemod/node_modules/ast-types/lib/types.js:60:19)
    at /Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:1698:22
    at FastPath.each (/Users/donovan/src/babel-codemod/node_modules/recast/lib/fast-path.js:145:7)
    at printStatementSequence (/Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:1670:10)
    at /Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:544:20
    at FastPath.call (/Users/donovan/src/babel-codemod/node_modules/recast/lib/fast-path.js:119:16)
    at genericPrintNoParens (/Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:543:26)
    at genericPrint (/Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:164:9)
    at printRootGenerically (/Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:105:15)
    at maybeReprint (/Users/donovan/src/babel-codemod/node_modules/recast/lib/printer.js:97:16)
1 file(s), 0 modified, 1 errors

[eventualbuddha]

Ah, looks like recast does support Noop and has since v0.10.14 (~2.5 years). I wonder why this is happening, then…

[MagicDuck]

Hmm, now that i look closer at it, according to the error stack, the error is thrown from ast-types, could it be that lib that does not support it? After all, that checking function looks fairly complex: https://github.com/benjamn/ast-types/blob/master/lib/types.js#L60 ...

[eventualbuddha]

Ah, it looks like ast-types defines Noop like so:

  def("Noop")
    .bases("Node")
    .build();

Notably, it does not have a base of Statement, so this part of printer.js will not match it as a Comment or a Statement and will assert that it is a string where it fails:

        if (namedTypes.Comment.check(stmt)) {
            // The pretty printer allows a dangling Comment node to act as
            // a Statement when the Comment can't be attached to any other
            // non-Comment node in the tree.
            sawComment = true;
        } else if (namedTypes.Statement.check(stmt)) {
            sawStatement = true;
        } else {
            // When the pretty printer encounters a string instead of an
            // AST node, it just prints the string. This behavior can be
            // useful for fine-grained formatting decisions like inserting
            // blank lines.
            isString.assert(stmt);
        }

My naïve fix would be to make Noop be the same as EmptyStatement:

// The empty .build() here means that an EmptyStatement can be constructed
// (i.e. it's not abstract) but that it needs no arguments.
    def("EmptyStatement").bases("Statement").build();

Unfortunately, the commit that adds Noop to babel is pretty light on explanation. It seems to mostly be used as an alternative to null for type annotations.

@MagicDuck I think you could simply use t.emptyStatement() instead of t.noop() and get the behavior you're looking for.

[MagicDuck]

Ah thanks @eventualbuddha, that should be enough for my needs. Much appreciate your time ! 😁

[eventualbuddha]

benjamn/ast-types#245

[eventualbuddha]

Not really anything to do here, so closing.