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.

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

Fix crash when at-function-named-arguments encounters trailing commas in function calls #271

Conversation

thibaudcolas
Copy link
Contributor

@thibaudcolas thibaudcolas commented Sep 23, 2018

When at-function-named-arguments is enabled, stylelint crashes when the rule attempts to parse function calls that have a trailing comma (has been part of Sass since ± 2016). For example:

.b {
  border: reset(
    $value: 40px,
  );
}

This will cause the following error:

    TypeError: Cannot read property 'key' of undefined

       98 |             case "always": {
    >  99 |               if (arg.key && isScssVarRegExp.test(arg.key)) {
          |                                                    ^
      100 |                 return;
      101 |               }

      at src/rules/at-function-named-arguments/index.js:99:52
      [...]

The error is caused by groupByKeyValue of parseFunctionArguments, which creates an empty group when seeing the comma. It seems like a simple fix would be to ignore the comma if it is trailing, this seems to work based on the tests I added.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 92.287% when pulling 7be866f on thibaudcolas:bug/at-function-named-arguments-trailing-comma into 993f32e on kristerkari:master.

@kristerkari
Copy link
Collaborator

Thanks! The fix looks good.

@kristerkari kristerkari merged commit b15cf1c into stylelint-scss:master Sep 23, 2018
@kristerkari
Copy link
Collaborator

the fix is available in version 3.3.1 👍

@thibaudcolas thibaudcolas deleted the bug/at-function-named-arguments-trailing-comma branch September 24, 2018 05:24
@thibaudcolas
Copy link
Contributor Author

Thank you 👌 I can confirm it works on my project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants