Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes #229. Throws syntax error for empty functions and doesnt block … #230

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixes #229. Throws syntax error for empty functions and doesnt block … #230

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f2d7c7d
Fixes #229. Throws syntax error for empty functions and doesnt block …
May 28, 2017
2b8459d
Merge branch 'master' into throw_syntax_error_if_function_expr_is_empty
frvge Dec 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/css/Parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,11 @@ Parser.prototype = function() {
functionText = tokenStream.token().value;
this._readWhitespace();
expr = this._expr(true);
if (expr === null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could remove this, and only go with the proposed fix in the while-loop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't help you here. I'm not so much familiar with the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

// the FUNCTION is set if there's a ( . It doesn't check for a closing ) in identOrFunctionToken in TokenStream..
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// 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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
functionText += expr;

// START: Horrible hack in case it's an IE filter
Expand All @@ -2009,7 +2014,7 @@ Parser.prototype = function() {

//functionText += this._term();
lt = tokenStream.peek();
while (lt !== Tokens.COMMA && lt !== Tokens.S && lt !== Tokens.RPAREN) {
while (lt !== Tokens.COMMA && lt !== Tokens.S && lt !== Tokens.RPAREN && lt !== Tokens.EOF) {
tokenStream.get();
functionText += tokenStream.token().value;
lt = tokenStream.peek();
Expand Down
2 changes: 1 addition & 1 deletion src/css/TokenStream.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ TokenStream.prototype = mix(new TokenStreamBase(), {
}
} else if (reader.peek() === ":") { // might be an IE function

// IE-specific functions always being with progid:
// IE-specific functions always begin with progid:
if (ident.toLowerCase() === "progid") {
ident += reader.readTo("(");
tt = Tokens.IE_FUNCTION;
Expand Down
8 changes: 7 additions & 1 deletion tests/css/Parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,8 @@ var YUITest = require("yuitest"),

_should: {
error: {
testIEFilter5: "Unexpected token '=' at line 1, col 14."
testIEFilter5: "Unexpected token '=' at line 1, col 14.",
testFunctionBroken: "Expected an expression in the function on line 1, col 1."
}
},

Expand Down Expand Up @@ -1505,6 +1506,11 @@ var YUITest = require("yuitest"),
var result = parser.parsePropertyValue("alpha(opacity=10)");

Assert.isInstanceOf(parserlib.css.PropertyValue, result);
},

testFunctionBroken: function() {
var parser = new Parser();
parser.parsePropertyValue("calc(");
}


Expand Down