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

Update: support logical assignments in core rules (refs #13569) #13618

Merged
merged 15 commits into from Aug 31, 2020
Merged
2 changes: 2 additions & 0 deletions docs/rules/operator-assignment.md
Expand Up @@ -23,6 +23,8 @@ JavaScript provides shorthand operators that combine variable assignment and som

This rule requires or disallows assignment operator shorthand where possible.

The rule applies to the operators listed in the above table.
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

## Options

This rule has a single string option:
Expand Down
25 changes: 24 additions & 1 deletion lib/rules/constructor-super.js
Expand Up @@ -5,6 +5,13 @@

"use strict";


//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -60,7 +67,23 @@ function isPossibleConstructor(node) {
return node.name !== "undefined";

case "AssignmentExpression":
return isPossibleConstructor(node.right);
if (node.operator === "=") {
return isPossibleConstructor(node.right);
}

if (astUtils.isLogicalAssignmentOperator(node.operator)) {
return (
isPossibleConstructor(node.left) ||
isPossibleConstructor(node.right)
);
}

/**
* All other assignment operators are mathematical assignment operators (arithmetic or bitwise).
* An assignment expression with a mathematical operator can either evaluate to a primitive value,
* or throw, depending on the operands. Thus, it cannot evaluate to a constructor function.
*/
return false;

case "LogicalExpression":
return (
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/operator-assignment.js
Expand Up @@ -151,7 +151,7 @@ module.exports = {
* @returns {void}
*/
function prohibit(node) {
if (node.operator !== "=") {
if (node.operator !== "=" && !astUtils.isLogicalAssignmentOperator(node.operator)) {
context.report({
node,
messageId: "unexpected",
Expand Down
29 changes: 27 additions & 2 deletions lib/rules/utils/ast-utils.js
Expand Up @@ -40,6 +40,8 @@ const STATEMENT_LIST_PARENTS = new Set(["Program", "BlockStatement", "SwitchCase
const DECIMAL_INTEGER_PATTERN = /^(0|[1-9]\d*)$/u;
const OCTAL_ESCAPE_PATTERN = /^(?:[^\\]|\\[^0-7]|\\0(?![0-9]))*\\(?:[1-7]|0[0-9])/u;

const LOGICAL_ASSIGNMENT_OPERATORS = new Set(["&&=", "||=", "??="]);

/**
* Checks reference if is non initializer and writable.
* @param {Reference} reference A reference to check.
Expand Down Expand Up @@ -722,6 +724,15 @@ function isMixedLogicalAndCoalesceExpressions(left, right) {
);
}

/**
* Checks if the given operator is a logical assignment operator.
* @param {string} operator The operator to check.
* @returns {boolean} `true` if the operator is a logical assignment operator.
*/
function isLogicalAssignmentOperator(operator) {
return LOGICAL_ASSIGNMENT_OPERATORS.has(operator);
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -1567,7 +1578,20 @@ module.exports = {
return true; // possibly an error object.

case "AssignmentExpression":
return module.exports.couldBeError(node.right);
if (node.operator === "=") {
return module.exports.couldBeError(node.right);
}

if (isLogicalAssignmentOperator(node.operator)) {
return module.exports.couldBeError(node.left) || module.exports.couldBeError(node.right);
}

/**
* All other assignment operators are mathematical assignment operators (arithmetic or bitwise).
* An assignment expression with a mathematical operator can either evaluate to a primitive value,
* or throw, depending on the operands. Thus, it cannot evaluate to an `Error` object.
*/
return false;

case "SequenceExpression": {
const exprs = node.expressions;
Expand Down Expand Up @@ -1754,5 +1778,6 @@ module.exports = {
isSpecificId,
isSpecificMemberAccess,
equalLiteralValue,
isSameReference
isSameReference,
isLogicalAssignmentOperator
};
37 changes: 36 additions & 1 deletion tests/lib/rules/constructor-super.js
Expand Up @@ -16,7 +16,7 @@ const { RuleTester } = require("../../../lib/rule-tester");
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2020 } });
const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2021 } });

ruleTester.run("constructor-super", rule, {
valid: [
Expand All @@ -37,7 +37,18 @@ ruleTester.run("constructor-super", rule, {
"class A extends B { constructor() { if (true) { super(); } else { super(); } } }",
"class A extends (class B {}) { constructor() { super(); } }",
"class A extends (B = C) { constructor() { super(); } }",
"class A extends (B &&= C) { constructor() { super(); } }",
"class A extends (B ||= C) { constructor() { super(); } }",
"class A extends (B ??= C) { constructor() { super(); } }",
"class A extends (B &&= 5) { constructor() { super(); } }",
"class A extends (B ||= 5) { constructor() { super(); } }",
"class A extends (B ??= 5) { constructor() { super(); } }",
"class A extends (B || C) { constructor() { super(); } }",
"class A extends (B && 5) { constructor() { super(); } }",
"class A extends (5 && B) { constructor() { super(); } }",
"class A extends (B || 5) { constructor() { super(); } }",
"class A extends (B ?? 5) { constructor() { super(); } }",

"class A extends (a ? B : C) { constructor() { super(); } }",
"class A extends (B, C) { constructor() { super(); } }",

Expand Down Expand Up @@ -112,6 +123,30 @@ ruleTester.run("constructor-super", rule, {
code: "class A extends 'test' { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
{
code: "class A extends (B = 5) { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
{
code: "class A extends (B += C) { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
{
code: "class A extends (B -= C) { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
{
code: "class A extends (B **= C) { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
{
code: "class A extends (B |= C) { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
{
code: "class A extends (B &= C) { constructor() { super(); } }",
errors: [{ messageId: "badSuper", type: "CallExpression" }]
},
Comment on lines +135 to +154
Copy link
Member

Choose a reason for hiding this comment

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

I agree we can say these 5 new errors are a semver-minor bug fix. Does anyone else object to that?


// derived classes.
{
Expand Down
24 changes: 24 additions & 0 deletions tests/lib/rules/func-name-matching.js
Expand Up @@ -29,6 +29,9 @@ ruleTester.run("func-name-matching", rule, {
"foo = function foo() {};",
{ code: "foo = function foo() {};", options: ["always"] },
{ code: "foo = function bar() {};", options: ["never"] },
{ code: "foo &&= function foo() {};", parserOptions: { ecmaVersion: 2021 } },
{ code: "obj.foo ||= function foo() {};", parserOptions: { ecmaVersion: 2021 } },
{ code: "obj['foo'] ??= function foo() {};", parserOptions: { ecmaVersion: 2021 } },
"obj.foo = function foo() {};",
{ code: "obj.foo = function foo() {};", options: ["always"] },
{ code: "obj.foo = function bar() {};", options: ["never"] },
Expand Down Expand Up @@ -284,6 +287,27 @@ ruleTester.run("func-name-matching", rule, {
{ messageId: "matchVariable", data: { funcName: "bar", name: "foo" } }
]
},
{
code: "foo &&= function bar() {};",
parserOptions: { ecmaVersion: 2021 },
errors: [
{ messageId: "matchVariable", data: { funcName: "bar", name: "foo" } }
]
},
{
code: "obj.foo ||= function bar() {};",
parserOptions: { ecmaVersion: 2021 },
errors: [
{ messageId: "matchProperty", data: { funcName: "bar", name: "foo" } }
]
},
{
code: "obj['foo'] ??= function bar() {};",
parserOptions: { ecmaVersion: 2021 },
errors: [
{ messageId: "matchProperty", data: { funcName: "bar", name: "foo" } }
]
},
{
code: "obj.foo = function bar() {};",
parserOptions: { ecmaVersion: 6 },
Expand Down
5 changes: 5 additions & 0 deletions tests/lib/rules/no-bitwise.js
Expand Up @@ -22,7 +22,12 @@ ruleTester.run("no-bitwise", rule, {
valid: [
"a + b",
"!a",
"a && b",
"a || b",
"a += b",
{ code: "a &&= b", parserOptions: { ecmaVersion: 2021 } },
{ code: "a ||= b", parserOptions: { ecmaVersion: 2021 } },
{ code: "a ??= b", parserOptions: { ecmaVersion: 2021 } },
{ code: "~[1, 2, 3].indexOf(1)", options: [{ allow: ["~"] }] },
{ code: "~1<<2 === -8", options: [{ allow: ["~", "<<"] }] },
{ code: "a|0", options: [{ int32Hint: true }] },
Expand Down
17 changes: 17 additions & 0 deletions tests/lib/rules/no-extend-native.js
Expand Up @@ -160,6 +160,23 @@ ruleTester.run("no-extend-native", rule, {
code: "(Object?.defineProperty)(Object.prototype, 'p', { value: 0 })",
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "unexpected", data: { builtin: "Object" } }]
},

// Logical assignments
{
code: "Array.prototype.p &&= 0",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "unexpected", data: { builtin: "Array" } }]
},
{
code: "Array.prototype.p ||= 0",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "unexpected", data: { builtin: "Array" } }]
},
{
code: "Array.prototype.p ??= 0",
parserOptions: { ecmaVersion: 2021 },
errors: [{ messageId: "unexpected", data: { builtin: "Array" } }]
}

]
Expand Down
18 changes: 18 additions & 0 deletions tests/lib/rules/no-invalid-this.js
Expand Up @@ -719,6 +719,24 @@ const patterns = [
errors,
valid: [NORMAL],
invalid: [USE_STRICT, IMPLIED_STRICT, MODULES]
},
{
code: "obj.method &&= function () { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 2021 },
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "obj.method ||= function () { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 2021 },
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
},
{
code: "obj.method ??= function () { console.log(this); z(x => console.log(x, this)); }",
parserOptions: { ecmaVersion: 2021 },
valid: [NORMAL, USE_STRICT, IMPLIED_STRICT, MODULES],
invalid: []
}
];

Expand Down
51 changes: 51 additions & 0 deletions tests/lib/rules/no-param-reassign.js
Expand Up @@ -368,6 +368,57 @@ ruleTester.run("no-param-reassign", rule, {
messageId: "assignmentToFunctionParamProp",
data: { name: "a" }
}]
},
{
code: "function foo(a) { a &&= b; }",
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "assignmentToFunctionParam",
data: { name: "a" }
}]
},
{
code: "function foo(a) { a ||= b; }",
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "assignmentToFunctionParam",
data: { name: "a" }
}]
},
{
code: "function foo(a) { a ??= b; }",
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "assignmentToFunctionParam",
data: { name: "a" }
}]
},
{
code: "function foo(a) { a.b &&= c; }",
options: [{ props: true }],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "assignmentToFunctionParamProp",
data: { name: "a" }
}]
},
{
code: "function foo(a) { a.b.c ||= d; }",
options: [{ props: true }],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "assignmentToFunctionParamProp",
data: { name: "a" }
}]
},
{
code: "function foo(a) { a[b] ??= c; }",
options: [{ props: true }],
parserOptions: { ecmaVersion: 2021 },
errors: [{
messageId: "assignmentToFunctionParamProp",
data: { name: "a" }
}]
}
]
});
21 changes: 19 additions & 2 deletions tests/lib/rules/no-throw-literal.js
Expand Up @@ -30,7 +30,10 @@ ruleTester.run("no-throw-literal", rule, {
"throw new foo();", // NewExpression
"throw foo.bar;", // MemberExpression
"throw foo[bar];", // MemberExpression
"throw foo = new Error();", // AssignmentExpression
"throw foo = new Error();", // AssignmentExpression with the `=` operator
{ code: "throw foo &&= 'literal'", parserOptions: { ecmaVersion: 2021 } }, // AssignmentExpression with a logical operator
{ code: "throw foo.bar ||= 'literal'", parserOptions: { ecmaVersion: 2021 } }, // AssignmentExpression with a logical operator
{ code: "throw foo[bar] ??= 'literal'", parserOptions: { ecmaVersion: 2021 } }, // AssignmentExpression with a logical operator
"throw 1, 2, new Error();", // SequenceExpression
"throw 'literal' && new Error();", // LogicalExpression (right)
"throw new Error() || 'literal';", // LogicalExpression (left)
Expand Down Expand Up @@ -104,7 +107,21 @@ ruleTester.run("no-throw-literal", rule, {

// AssignmentExpression
{
code: "throw foo = 'error';",
code: "throw foo = 'error';", // RHS is a literal
errors: [{
messageId: "object",
type: "ThrowStatement"
}]
},
{
code: "throw foo += new Error();", // evaluates to a primitive value, or throws while evaluating
errors: [{
messageId: "object",
type: "ThrowStatement"
}]
},
{
code: "throw foo &= new Error();", // evaluates to a primitive value, or throws while evaluating
errors: [{
messageId: "object",
type: "ThrowStatement"
Expand Down