Community
Participate
Working Groups
For a line like the following a "missing semicolon" warning is shown for the return statement. Its semicolon is there, it's just a few editor lines down. return blocks.length && blocks[0].start === block.start && blocks[0].end === block.end + changeCount && blocks[0].typeId === block.typeId && blocks[0].getBlocks().length === block.getBlocks().length; <-- it's here
The warning is valid -- this is a dangerous JS parsing quirk. The code you posted is equivalent to the following: > return undefined > blocks.length && > blocks[0].start === block.start && > blocks[0].end === block.end + changeCount && > blocks[0].typeId === block.typeId && > blocks[0].getBlocks().length === block.getBlocks().lengthngth === block.getBlocks().length; IOW, it returns nothing and then there's a no-op Expression statement after the return. We should really have an "unreachable code" warning on the expression statement, which would make the problem here clearer.
(In reply to Mark Macdonald from comment #1) > IOW, it returns nothing and then there's a no-op Expression statement after > the return. We should really have an "unreachable code" warning on the > expression statement, which would make the problem here clearer. We correctly forward / report the parse error (Illegal return statement) in this case. In tolerant mode we can recover from this problem and produce the following tree: { "type": "Program", "body": [ { "type": "ReturnStatement", "argument": null }, { "type": "ExpressionStatement", "expression": { "type": "LogicalExpression", "operator": "&&", "left": { Notice the return argument is null. I'm not sure there would be any value in a new rule to report on the same problem code with a different error / warning.
(In reply to Michael Rennie from comment #2) > (In reply to Mark Macdonald from comment #1) > > > IOW, it returns nothing and then there's a no-op Expression statement after > > the return. We should really have an "unreachable code" warning on the > > expression statement, which would make the problem here clearer. > > We correctly forward / report the parse error (Illegal return statement) in > this case. Of course if we wrap it all up like: function f() { var blocks = [], block, changeCount; return blocks.length && blocks[0].start === block.start && blocks[0].end === block.end + changeCount && blocks[0].typeId === block.typeId && blocks[0].getBlocks().length === block.getBlocks().length; } f(); we don't need tolerant mode to produce a null handle to the return statement value. Leading to no parse error.
(In reply to Michael Rennie from comment #3) Heres the related spec for fun: http://es5.github.io/#x12.9
(In reply to Michael Rennie from comment #3) Right.. I was assuming Grant's original code was inside a function.
(In reply to Mark Macdonald from comment #5) > Right.. I was assuming Grant's original code was inside a function. Yes, it's in a function. This characteristic of js parsing was unexpected by me to the point that I assumed a lint error rather than reacting to the warning. A more specific warning message would have clued me into the real issue.
(In reply to Grant Gayed from comment #6) > (In reply to Mark Macdonald from comment #5) > > Right.. I was assuming Grant's original code was inside a function. > > Yes, it's in a function. > > This characteristic of js parsing was unexpected by me to the point that I > assumed a lint error rather than reacting to the warning. A more specific > warning message would have clued me into the real issue. This rule would be a bit tricky to implement without inspecting the source. The problem is that there is no token descriptor for a line break. So assuming we have a snippet like: function f() { return true; } we get tokens like: { "type": "Keyword", "value": "return" }, { "type": "Boolean", "value": "true" }, { "type": "Punctuator", "value": ";" }, The other option is that I could make this a parse problem, since it does technically violate the spec. We already have a general 'Unexpected <thing>' error in Esprima, I could report 'Unexpected line break' or something.
Created the eslint rule + tests: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=6ca8408f2dd694515acab6fc626a930acfa53db4 The default severity for the new rule is error.
