Skip to content

Commit

Permalink
[[FIX]] Regular args can come after args with default
Browse files Browse the repository at this point in the history
Fixes #1779
  • Loading branch information
lukeapage authored and jugglinmike committed Aug 1, 2015
1 parent 4d4cfcd commit f2a59f1
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 8 deletions.
6 changes: 4 additions & 2 deletions src/jshint.js
Original file line number Diff line number Diff line change
Expand Up @@ -2800,10 +2800,12 @@ var JSHINT = (function() {
}
}

// it is a syntax error to have a regular argument after a default argument
// It is valid to have a regular argument after a default argument
// since undefined can be used for missing parameters. Still warn as it is
// a possible code smell.
if (pastDefault) {
if (state.tokens.next.id !== "=") {
error("E051", state.tokens.current);
error("W138", state.tokens.current);
}
}
if (state.tokens.next.id === "=") {
Expand Down
7 changes: 4 additions & 3 deletions src/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ var errors = {
E044: null,
E045: "Invalid for each loop.",
E046: "A yield statement shall be within a generator function (with syntax: `function*`)",
E047: null, // Vacant
E047: null,
E048: "{a} declaration not directly within block.",
E049: "A {a} cannot be named '{b}'.",
E050: "Mozilla requires the yield expression to be parenthesized here.",
E051: "Regular parameters cannot come after default parameters.",
E051: null,
E052: "Unclosed template literal.",
E053: "Export declaration must be in global scope.",
E054: "Class properties must be methods. Expected '(' but instead saw '{a}'.",
Expand Down Expand Up @@ -217,7 +217,8 @@ var warnings = {
W134: "The '{a}' option is only available when linting ECMAScript {b} code.",
W135: "{a} may not be supported by non-browser environments.",
W136: "'{a}' must be in function scope.",
W137: "Empty destructuring."
W137: "Empty destructuring.",
W138: "Regular parameters should not come after default parameters."
};

var info = {
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ exports.testDefaultArguments = function (test) {
.addError(14, "'bar' is not defined.")
.addError(14, "'num3' was used before it was declared, which is illegal for 'param' variables.")
.addError(15, "'num4' was used before it was declared, which is illegal for 'param' variables.")
.addError(18, "Regular parameters cannot come after default parameters.")
.addError(18, "Regular parameters should not come after default parameters.")
.addError(27, "'c' is not defined.")
.addError(33, "'d' was used before it was defined.")
.addError(36, "'e' was used before it was declared, which is illegal for 'param' variables.")
Expand All @@ -1447,7 +1447,7 @@ exports.testDefaultArguments = function (test) {
TestRun(test)
.addError(14, "'num3' was used before it was declared, which is illegal for 'param' variables.")
.addError(15, "'num4' was used before it was declared, which is illegal for 'param' variables.")
.addError(18, "Regular parameters cannot come after default parameters.")
.addError(18, "Regular parameters should not come after default parameters.")
.addError(36, "'e' was used before it was declared, which is illegal for 'param' variables.")
.test(src, { moz: true });

Expand All @@ -1461,7 +1461,7 @@ exports.testDefaultArguments = function (test) {
.addError(15, "'default parameters' is only available in ES6 (use esnext option).")
.addError(15, "'num4' was used before it was declared, which is illegal for 'param' variables.")
.addError(18, "'default parameters' is only available in ES6 (use esnext option).")
.addError(18, "Regular parameters cannot come after default parameters.")
.addError(18, "Regular parameters should not come after default parameters.")
.addError(26, "'default parameters' is only available in ES6 (use esnext option).")
.addError(31, "'default parameters' is only available in ES6 (use esnext option).")
.addError(33, "'default parameters' is only available in ES6 (use esnext option).")
Expand Down

6 comments on commit f2a59f1

@octref
Copy link

@octref octref commented on f2a59f1 Aug 17, 2015

Choose a reason for hiding this comment

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

@lukeapage Any idea when you'd publish next version to NPM including this change?

@evenfrost
Copy link

Choose a reason for hiding this comment

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

Interested in this as well.

@luisherranz
Copy link

Choose a reason for hiding this comment

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

Another vote here 👍

@lukeapage
Copy link
Member Author

Choose a reason for hiding this comment

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

@jugglinmike is working on it

@octref
Copy link

@octref octref commented on f2a59f1 Sep 27, 2015

Choose a reason for hiding this comment

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

2.9.0 was releated here several weeks ago can you put it on NPM?

@jugglinmike
Copy link
Member

Choose a reason for hiding this comment

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

Version 2.9.0 of this library was revoked shortly after being released: https://twitter.com/JSHint/status/639564453852356608

You can expect this fix in the next release, which will be published on npm, documented on the project's website, and announced on Twitter all at the same time.

Please sign in to comment.