Bug 488523

Comment 1 Olivier Thomann 2016-02-26 13:46:33 EST

The problem comes from the line:
		if (.isSwept () || .isExploded ()) return;
for the if statement, node.test is undefined. The syntax is incorrect. So the bug is either that the recovered nodes are not properly initialized or the walk of the tree should check the undefined condition in if statement.

Curtis, Michael, I think the ast is malformed in that case of boggus code. Should we report this issue to the used library?

Comment 2 Olivier Thomann 2016-02-26 14:15:49 EST

I think the test property should be set to null in this case. This is what we expect to find in the rules when getting an invalid if statement.
I think the parseTolerant function in esprima.js should return null when an error is found.

    function parseTolerant(parseFunction) {
        return function () {
            try {
                return parseFunction.apply(null, arguments);
            } catch (e) {
				recordError(e);
            }
            return null;
        };
    }
We might also want to return a recovered node in this case.

Comment 3 Michael Rennie 2016-02-26 14:27:41 EST

The problem is indeed that we were failing to recover properly from the broken if statement:

 (.isSwept () || .isExploded ()) return;

The AST ended up with a null inserted for the 'test' node, which caused Tern to fail in the preInfer phase (long before our tooling gets to run). The AST should have been filled in with the recovered node.

Fix: 

http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=7ab376791bc40b57e1c3a87b8646701fb3fc44a1

> Curtis, Michael, I think the ast is malformed in that case of boggus code. Should we report this issue to the used library?

No, the recovery support is our customization to the parser.