benjamn / recast

JavaScript syntax tree transformer, nondestructive pretty-printer, and automatic source map generator
MIT License
4.98k stars 348 forks source link

Critical assertion error generating source maps #831

Closed GianlucaGuarini closed 1 year ago

GianlucaGuarini commented 3 years ago

I am working since days on a Riot.js compiler bug, this issue is driving me mad. I have noticed that the problem is coming from the recast.print method when it tries to generate the sourcemaps.

I was able to reproduce the issue with a few lines of code here

That's the code to reproduce it in case you want to try to copy and paste it directly in your editor:

const recast = require('recast')
const types = recast.types
const builders = recast.types.builders
const scope = builders.identifier('scope')

const expression = `(

                                                 {
    complete: onComplete,
    translateX: 100
  })`

const ast = recast.parse(expression, {
  sourceFileName: 'test.js',

})

// replace the scope of the identifiers found
types.visit(ast, {
  visitIdentifier(path) {
    path.replace(builders.memberExpression(
      scope,
      path.node,
      false
    ))

    return false
  }
})

// crate a function 
const result = builders.functionExpression(
  null,
  [scope],
  builders.blockStatement([builders.returnStatement(
    ast.program.body[0].expression
  )])
)

// generate the output
const output = recast.print(result, {
  sourceMapName: `source.map`
})

return output

Any hint or help solving this issue is really appreciated as long as the sourcemap output will be correct :)

Thank you

eventualbuddha commented 3 years ago

Replacing an identifier with a member expression is not always safe. You’ll be replacing the keys in your object with e.g. scope.translateX, which is not a valid key.

GianlucaGuarini commented 3 years ago

@eventualbuddha thank you for your feedback. The @riotjs/compiler is much more complex than this demo. The code above is just needed to show the issue in recast that seems to be clearly related to the breaking spaces since the following demo works as expected: https://runkit.com/gianlucaguarini/5fe44ae13e9cdb001a56fc21

eventualbuddha commented 3 years ago

Ah, you're right. I don't know what's causing the issue you're seeing, but it does appear to be a bug. Thanks for clarifying.

GianlucaGuarini commented 3 years ago

@benjamn do you accept PRs? I might be interested in solving this issue with a patch

eventualbuddha commented 3 years ago

Yep, though it may not happen quickly.

GianlucaGuarini commented 3 years ago

@eventualbuddha I have tried to keep the patch simple https://github.com/benjamn/recast/pull/989 but your help on it is highly appreciated.

GianlucaGuarini commented 1 year ago

I am not able to reproduce this issue with the newest recast version. I am going to close this issue and my PR. Thank you anyway :)