`multi-var` transform conflicts with `let` transform #205

Open
e-cloud opened this Issue Jan 5, 2017 · 3 comments

Projects

None yet

2 participants

@e-cloud
Contributor
e-cloud commented Jan 5, 2017

First:
I try to convert some file with var declaration.
got error stacktrace:

/home/cloud/devlab/refactor-tool/node_modules/lebab/lib/Logger.js:33
        line: node.loc.start.line,
                      ^

TypeError: Cannot read property 'start' of undefined
    at Logger.warn (/home/cloud/devlab/refactor-tool/node_modules/lebab/lib/Logger.js:33:23)
    at logWarningForVarKind (/home/cloud/devlab/refactor-tool/node_modules/lebab/lib/transform/let.js:204:12)
    at /home/cloud/devlab/refactor-tool/node_modules/lebab/lib/transform/let.js:176:7
    at Array.forEach (native)
    at transformVarsToLetOrConst (/home/cloud/devlab/refactor-tool/node_modules/lebab/lib/transform/let.js:164:31)
    at leaveFunction (/home/cloud/devlab/refactor-tool/node_modules/lebab/lib/transform/let.js:158:3)
    at Controller.leave (/home/cloud/devlab/refactor-tool/node_modules/lebab/lib/transform/let.js:48:9)
    at Controller.__execute (/home/cloud/devlab/refactor-tool/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/home/cloud/devlab/refactor-tool/node_modules/estraverse/estraverse.js:491:28)
    at Object.traverse (/home/cloud/devlab/refactor-tool/node_modules/estraverse/estraverse.js:713:27)

It turns out VariableDeclaration doesn't have loc property. But the Logger try to log the location

@nene
Collaborator
nene commented Jan 5, 2017

I somewhat imagine what could be causing this, but could you provide some example code that causes this and name the transform that you're using. My first attempt of reproducing this didn't succeed.

@e-cloud
Contributor
e-cloud commented Jan 5, 2017

the usage:

function tranform5To6(src) {
	return lebab.transform(src, [
		'arrow',
		'for-of',
		'arg-rest',
		'arg-spread',
		'obj-method',
		'obj-shorthand',
		'commonjs',
		'multi-var',
		'let',
		'includes',
		'template',
		'default-param',
		'class'
	]).code
}

I'm sorry, i don't know what atual file cause the problem. The files i try to transform is from https://github.com/traceglMPL/tracegl/

You can try https://github.com/traceglMPL/tracegl/blob/master/tracegl.js.

Mind that some string like "\033" would break down babylon and lebab. use "\x1b"

@nene
Collaborator
nene commented Jan 5, 2017 edited

Turns out that the problem is caused by using multi-var and let transforms together - and in that particular order. Can be reproduced with the following code:

foo++;
var foo = 1, bar = 2;

The problem is that these transforms modify the same things and as a result step on each others toes. multi-var creates new VariableDeclaration nodes for which it doesn't give any location information and let transform is unable to transform one of these declarations, so it wants to print out error for which it wants to use the location information.

  • First off I have to say that applying lots of transforms in one go is a recipe for running into all kinds of problems. Lebab README warns you against that. You really should expect what each transform does, not apply them blindly.
  • A temporary fix for this in Lebab could be to ignore the missing line number information when printing out errors.
  • A proper fix would be to ensure we always add line number information when creating new nodes. There are lots of places where we create new nodes, so it's not so easily solvable.
  • An alternative solution is to convert to string and back after each transform. By re-parsing the code, all new nodes will get proper location data. That would slow down Lebab when using multiple transforms, but it's not a recommended practice anyway.

Thanks for the bug report.

@nene nene changed the title from cannot convert `var` to `multi-var` transform conflicts with `let` transform Jan 5, 2017
@nene nene added the bug label Jan 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment