Fixes #229. Throws syntax error for empty functions and doesnt block … #230
Conversation
|
Hmm, this can be a breaking change as far as I can tell. On the other hand not throwing an error can be bad too. Not sure how to proceed. If you need this for CSS Lint I guess we should merge it. I'm just not sure about any other projects that depend on parer-lib. |
| @@ -1986,6 +1986,11 @@ Parser.prototype = function() { | |||
| functionText = tokenStream.token().value; | |||
| this._readWhitespace(); | |||
| expr = this._expr(true); | |||
| if (expr === null) { | |||
There was a problem hiding this comment.
I could remove this, and only go with the proposed fix in the while-loop.
There was a problem hiding this comment.
I can't help you here. I'm not so much familiar with the code.
There was a problem hiding this comment.
@nschonni , do you have a preference on how to proceed? The list of dependent packages of parserlib is at https://www.npmjs.com/browse/depended/parserlib , so that's not too many.
| if (expr === null) { | ||
| // the FUNCTION is set if there's a ( . It doesn't check for a closing ) in identOrFunctionToken in TokenStream.. | ||
| // if there's nothing between the brackets, expr is null | ||
| throw new SyntaxError("Expected an expression in the function on line " + tokenStream.token().startLine + ", col " + tokenStream.token().startCol + ".", tokenStream.token().startLine, tokenStream.token().startCol); |
There was a problem hiding this comment.
You don't need to emit the line number message yourself; all the other SyntaxErrors in this code just pass them as the second and third arguments. Following that practice will simplify this line a lot.
| @@ -1986,6 +1986,11 @@ Parser.prototype = function() { | |||
| functionText = tokenStream.token().value; | |||
| this._readWhitespace(); | |||
| expr = this._expr(true); | |||
| if (expr === null) { | |||
| // the FUNCTION is set if there's a ( . It doesn't check for a closing ) in identOrFunctionToken in TokenStream.. | |||
There was a problem hiding this comment.
This is a bit confusingly worded. Perhaps this:
"The FUNCTION token has already checked that there is an opening (."
Or omit this line of comment entirely. The level of commenting in the line below is normal for other similar situations in the codebase.
|
Some functions can be empty according to the spec e.g. all |
…anymore.
Maybe I shouldn't throw the SyntaxError though. What do you think?