Skip to content

Commit

Permalink
[[FIX]] Relax restriction on asgnmnt to arguments
Browse files Browse the repository at this point in the history 
Previously, JSHint issued E031 ("Bad assignment.") when it encountered
assignment to a mapped `arguments` object. Not only is this message
vague, its implementation as an error incorrectly disallows
syntactically valid JavaScript.

Introduce a new warning which more clearly describes the rationale, and
emit the new warning in place of the existing error so that users may
opt out.

Although this change causes changes in messages reported by JSHint, it
is backwards-compatible because the prior behavior was implemented in
terms of an error. Existing users are therefore not reliant on the prior
message or code.
  • Loading branch information
@jugglinmike @rwaldron
jugglinmike authored and rwaldron committed Jul 24, 2018
1 parent 3aa02db commit 0a66710
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 51 deletions.
4 changes: 2 additions & 2 deletions src/jshint.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1341,7 +1341,7 @@ var JSHINT = (function() {

if (left.id === ".") {
if (!left.left || left.left.value === "arguments" && !state.isStrict()) {
warning("E031", assignToken);
warning("W143", assignToken);
}

state.nameStack.set(state.tokens.prev);
Expand All @@ -1351,7 +1351,7 @@ var JSHINT = (function() {
if (left.id === "{" || !left.left) {
warning("E031", assignToken);
} else if (left.left.value === "arguments" && !state.isStrict()) {
warning("E031", assignToken);
warning("W143", assignToken);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/messages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ var warnings = {
W139: "Function expressions should not be used as the second operand to instanceof.",
W140: "Missing comma.",
W141: "Empty {a}: this is unnecessary and can be removed.",
W142: "Empty {a}: consider replacing with `import '{b}';`."
W142: "Empty {a}: consider replacing with `import '{b}';`.",
W143: "Assignment to properties of a mapped arguments object may cause " +
"unexpected changes to formal parameters."
};

var info = {
Expand Down
43 changes: 0 additions & 43 deletions tests/test262/expectations.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,6 @@ test/built-ins/AsyncFunction/instance-length.js(default)
test/built-ins/AsyncFunction/instance-length.js(strict mode)
test/built-ins/AsyncFunction/instance-prototype-property.js(default)
test/built-ins/AsyncFunction/instance-prototype-property.js(strict mode)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-expansion-after-exhaustion.js(default)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-expansion-before-exhaustion.js(default)
test/built-ins/ArrayIteratorPrototype/next/args-mapped-truncation-before-exhaustion.js(default)
test/built-ins/Function/prototype/toString/AsyncFunction.js(default)
test/built-ins/Function/prototype/toString/AsyncFunction.js(strict mode)
test/built-ins/Function/prototype/toString/async-function-declaration.js(default)
Expand All @@ -246,15 +243,6 @@ test/built-ins/Function/prototype/toString/line-terminator-normalisation-LF.js(d
test/built-ins/Function/prototype/toString/line-terminator-normalisation-LF.js(strict mode)
test/built-ins/Function/prototype/toString/unicode.js(default)
test/built-ins/Function/prototype/toString/unicode.js(strict mode)
test/built-ins/Array/prototype/every/15.4.4.16-2-17.js(default)
test/built-ins/Array/prototype/forEach/15.4.4.18-2-17.js(default)
test/built-ins/Array/prototype/indexOf/15.4.4.14-2-17.js(default)
test/built-ins/Array/prototype/lastIndexOf/15.4.4.15-2-17.js(default)
test/built-ins/Array/prototype/reduce/15.4.4.21-2-17.js(default)
test/built-ins/Array/prototype/some/15.4.4.17-2-17.js(default)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-135.js(default)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-161.js(default)
test/built-ins/Object/defineProperties/15.2.3.7-5-b-214.js(default)
test/built-ins/Object/prototype/valueOf/S15.2.4.4_A14.js(default)
test/built-ins/Object/prototype/valueOf/S15.2.4.4_A14.js(strict mode)
test/built-ins/RegExp/S15.10.7_A1_T1.js(default)
Expand Down Expand Up @@ -352,30 +340,11 @@ test/language/identifiers/vals-rus-alpha-upper-via-escape-hex.js(default)
test/language/identifiers/vals-rus-alpha-upper-via-escape-hex.js(strict mode)
test/language/rest-parameters/array-pattern.js(default)
test/language/rest-parameters/array-pattern.js(strict mode)
test/language/rest-parameters/no-alias-arguments.js(default)
test/language/rest-parameters/object-pattern.js(default)
test/language/rest-parameters/object-pattern.js(strict mode)
test/language/arguments-object/10.5-7-b-2-s.js(default)
test/language/arguments-object/10.5-7-b-3-s.js(default)
test/language/arguments-object/10.6-10-c-ii-2.js(default)
test/language/arguments-object/10.6-13-c-1-s.js(strict mode)
test/language/arguments-object/10.6-1gs.js(strict mode)
test/language/arguments-object/10.6-2gs.js(strict mode)
test/language/arguments-object/S10.6_A3_T4.js(default)
test/language/arguments-object/S10.6_A5_T4.js(default)
test/language/arguments-object/unmapped/via-params-dflt.js(default)
test/language/arguments-object/unmapped/via-params-dstr.js(default)
test/language/arguments-object/unmapped/via-params-rest.js(default)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-4.js(default)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-4.js(default)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-4.js(default)
test/language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-4.js(default)
test/language/arguments-object/mapped/mapped-arguments-nonwritable-nonconfigurable-3.js(default)
test/language/arguments-object/mapped/nonconfigurable-descriptors-set-value-by-arguments.js(default)
test/language/arguments-object/mapped/nonconfigurable-nonenumerable-nonwritable-descriptors-set-by-arguments.js(default)
test/language/arguments-object/mapped/nonconfigurable-nonwritable-descriptors-set-by-arguments.js(default)
test/language/arguments-object/mapped/nonwritable-nonconfigurable-descriptors-set-by-arguments.js(default)
test/language/arguments-object/mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-arguments.js(default)
test/language/eval-code/indirect/block-decl-strict.js(default)
test/language/eval-code/indirect/block-decl-strict.js(strict mode)
test/language/eval-code/indirect/always-non-strict.js(strict mode)
Expand Down Expand Up @@ -654,14 +623,10 @@ test/language/expressions/exponentiation/exp-operator.js(strict mode)
test/language/expressions/delete/11.4.1-5-a-5gs.js(strict mode)
test/language/expressions/instanceof/S11.8.6_A3.js(default)
test/language/expressions/instanceof/S11.8.6_A3.js(strict mode)
test/language/expressions/postfix-decrement/11.3.2-2-3-s.js(default)
test/language/expressions/prefix-decrement/11.4.5-2-3-s.js(default)
test/language/expressions/prefix-increment/11.4.4-2-3-s.js(default)
test/language/expressions/tagged-template/constructor-invocation.js(default)
test/language/expressions/tagged-template/constructor-invocation.js(strict mode)
test/language/expressions/tagged-template/invalid-escape-sequences.js(default)
test/language/expressions/tagged-template/invalid-escape-sequences.js(strict mode)
test/language/expressions/yield/formal-parameters-after-reassignment-non-strict.js(default)
test/language/expressions/yield/rhs-omitted.js(default)
test/language/expressions/yield/rhs-omitted.js(strict mode)
test/language/expressions/yield/rhs-primitive.js(default)
Expand Down Expand Up @@ -702,7 +667,6 @@ test/language/expressions/function/dstr-dflt-ary-ptrn-rest-obj-id.js(strict mode
test/language/expressions/function/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(default)
test/language/expressions/function/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(strict mode)
test/language/expressions/function/param-dflt-yield-non-strict.js(default)
test/language/expressions/function/params-dflt-args-unmapped.js(default)
test/language/expressions/function/scope-param-rest-elem-var-close.js(default)
test/language/expressions/function/scope-param-rest-elem-var-open.js(default)
test/language/expressions/function/use-strict-with-non-simple-param.js(default)
Expand Down Expand Up @@ -737,7 +701,6 @@ test/language/expressions/generators/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(defa
test/language/expressions/generators/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(strict mode)
test/language/expressions/generators/param-dflt-yield.js(default)
test/language/expressions/generators/param-dflt-yield.js(strict mode)
test/language/expressions/generators/params-dflt-args-unmapped.js(default)
test/language/expressions/generators/scope-param-rest-elem-var-close.js(default)
test/language/expressions/generators/scope-param-rest-elem-var-open.js(default)
test/language/expressions/generators/use-strict-with-non-simple-param.js(default)
Expand Down Expand Up @@ -1046,8 +1009,6 @@ test/language/expressions/object/method-definition/name-prop-name-yield-expr.js(
test/language/expressions/object/method-definition/name-prop-name-yield-id.js(default)
test/language/expressions/object/method-definition/object-method-returns-promise.js(default)
test/language/expressions/object/method-definition/object-method-returns-promise.js(strict mode)
test/language/expressions/object/method-definition/params-dflt-gen-meth-args-unmapped.js(default)
test/language/expressions/object/method-definition/params-dflt-meth-args-unmapped.js(default)
test/language/expressions/object/method-definition/setter-use-strict-with-non-simple-param.js(default)
test/language/expressions/object/method-definition/setter-use-strict-with-non-simple-param.js(strict mode)
test/language/expressions/object/method-definition/use-strict-with-non-simple-param.js(default)
Expand Down Expand Up @@ -1406,7 +1367,6 @@ test/language/statements/generators/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(defau
test/language/statements/generators/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(strict mode)
test/language/statements/generators/param-dflt-yield.js(default)
test/language/statements/generators/param-dflt-yield.js(strict mode)
test/language/statements/generators/params-dflt-args-unmapped.js(default)
test/language/statements/generators/scope-param-rest-elem-var-close.js(default)
test/language/statements/generators/scope-param-rest-elem-var-open.js(default)
test/language/statements/generators/use-strict-with-non-simple-param.js(default)
Expand All @@ -1430,7 +1390,6 @@ test/language/statements/labeled/decl-gen.js(strict mode)
test/language/statements/labeled/decl-let.js(default)
test/language/statements/labeled/decl-let.js(strict mode)
test/language/statements/labeled/value-yield-non-strict.js(default)
test/language/statements/function/S13_A13_T3.js(default)
test/language/statements/function/dstr-ary-ptrn-rest-ary-elem.js(default)
test/language/statements/function/dstr-ary-ptrn-rest-ary-elem.js(strict mode)
test/language/statements/function/dstr-ary-ptrn-rest-ary-elision.js(default)
Expand Down Expand Up @@ -1460,7 +1419,6 @@ test/language/statements/function/dstr-dflt-ary-ptrn-rest-obj-id.js(strict mode)
test/language/statements/function/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(default)
test/language/statements/function/dstr-dflt-ary-ptrn-rest-obj-prop-id.js(strict mode)
test/language/statements/function/param-dflt-yield-non-strict.js(default)
test/language/statements/function/params-dflt-args-unmapped.js(default)
test/language/statements/function/scope-param-rest-elem-var-close.js(default)
test/language/statements/function/scope-param-rest-elem-var-open.js(default)
test/language/statements/function/use-strict-with-non-simple-param.js(default)
Expand All @@ -1485,7 +1443,6 @@ test/language/statements/return/S12.9_A1_T8.js(default)
test/language/statements/return/S12.9_A1_T8.js(strict mode)
test/language/statements/return/S12.9_A1_T9.js(default)
test/language/statements/return/S12.9_A1_T9.js(strict mode)
test/language/statements/for-of/arguments-mapped-mutation.js(default)
test/language/statements/for-of/decl-cls.js(default)
test/language/statements/for-of/decl-cls.js(strict mode)
test/language/statements/for-of/decl-fun.js(default)
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,10 @@ exports.assignment = function (test) {
];

var run = TestRun(test)
.addError(2, 20, "Bad assignment.")
.addError(3, 16, "Bad assignment.")
.addError(4, 20, "Bad assignment.")
.addError(5, 16, "Bad assignment.")
.addError(2, 20, "Assignment to properties of a mapped arguments object may cause unexpected changes to formal parameters.")
.addError(3, 16, "Assignment to properties of a mapped arguments object may cause unexpected changes to formal parameters.")
.addError(4, 20, "Assignment to properties of a mapped arguments object may cause unexpected changes to formal parameters.")
.addError(5, 16, "Assignment to properties of a mapped arguments object may cause unexpected changes to formal parameters.")
.addError(14, 5, "Bad assignment.");

run.test(code, { plusplus: true, es3: true });
Expand Down Expand Up @@ -2005,7 +2005,7 @@ exports["destructuring globals with syntax error"] = function (test) {
TestRun(test)
.addError(1, 19, "Extending prototype of native object: 'Number'.")
.addError(3, 9, "Bad assignment.")
.addError(4, 14, "Bad assignment.")
.addError(4, 14, "Assignment to properties of a mapped arguments object may cause unexpected changes to formal parameters.")
.addError(6, 7, "Do not assign to the exception parameter.")
.addError(7, 6, "Do not assign to the exception parameter.")
.addError(9, 9, "Bad assignment.")
Expand Down

0 comments on commit 0a66710

Please sign in to comment.