From e7cb5b157aa7e2dae2c374312b01936c82b27ac2 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 5 Jul 2022 20:50:46 +0200 Subject: [PATCH 01/37] feat: add rule logical-assignment-operators Fixes #13689 --- .../src/rules/logical-assignment-operators.md | 121 +++ lib/rules/index.js | 1 + lib/rules/logical-assignment-operators.js | 395 +++++++++ .../lib/rules/logical-assignment-operators.js | 827 ++++++++++++++++++ tools/rule-types.json | 1 + 5 files changed, 1345 insertions(+) create mode 100644 docs/src/rules/logical-assignment-operators.md create mode 100644 lib/rules/logical-assignment-operators.js create mode 100644 tests/lib/rules/logical-assignment-operators.js diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md new file mode 100644 index 00000000000..5ea6e3158b9 --- /dev/null +++ b/docs/src/rules/logical-assignment-operators.md @@ -0,0 +1,121 @@ +--- +title: logical-assignment-operators +layout: doc +edit_link: https://github.com/eslint/eslint/edit/main/docs/src/rules/logical-assignment-operators.md +rule_type: suggestion +--- + + + +Require or disallow assignment logical operator shorthand + +## Rule Details + +This rule requires or disallows logical assignment operator shorthand. +On the one hand the shorthand makes it clear that the variable reference is the same. +On the other hand the combined operator may be seen as "magic". +This is a feature introduced in ES2021. + +## Options + +This rule has a string and an object option. +String option: + +* `"always"` (default) +* `"never"` + +Object option (only available if string option is set to `"always"`): + +* `"enforceForIfStatements": false`(default) Do *not* check for equivalent if statements + +* `"enforceForIfStatements": true` Check for equivalent if statements + +### always + +Examples of **incorrect** code for this rule with the default `"always"` option: + +::: incorrect + +```js +/*eslint logical-assignment-operators: ["error", "always"]*/ + +a = a || b +a = a && b +a = a ?? b +``` + +::: + +Examples of **correct** code for this rule with the default `"always"` option: + +::: correct + +```js +/*eslint logical-assignment-operators: ["error", "always"]*/ + +a = b +a ||= b +a += b +``` + +::: + +### never + +Examples of **incorrect** code for this rule with the `"never"` option: + +::: incorrect + +```js +/*eslint logical-assignment-operators: ["error", "never"]*/ + +a ||= b +a &&= b +``` + +::: + +Examples of **correct** code for this rule with the `"never"` option: + +::: correct + +```js +/*eslint logical-assignment-operators: ["error", "never"]*/ + +a = a || b +a = a ?? b +``` + +::: + +### enforceForIfStatements + +This option checks for additional patterns with if statements which could be expressed with the logical assignment operator. +::: correct + +```js +/*eslint logical-assignment-operators: ["error", "always", { enforceForIfStatements: true }]*/ + +if (a) b = c +if (a === 0) a = b +``` + +::: + +::: incorrect + +```js +/*eslint logical-assignment-operators: ["error", "always", { enforceForIfStatements: true }]*/ + +if (a) a = b // <=> a &&= b +if (!a) a = b // <=> a ||= b + +if (a == null) a = b // <=> a ??= b +if (a === null || a === undefined) a = b // <=> a ??= b +``` + +::: + +## When Not To Use It + +Use of logical operator assignment shorthand is a stylistic choice. Leaving this rule turned off would allow developers to choose which style is more readable on a case-by-case basis. diff --git a/lib/rules/index.js b/lib/rules/index.js index aef47f5cadc..7038dd81471 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -72,6 +72,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({ "lines-around-comment": () => require("./lines-around-comment"), "lines-around-directive": () => require("./lines-around-directive"), "lines-between-class-members": () => require("./lines-between-class-members"), + "logical-assignment-operators": () => require("./logical-assignment-operators.js"), "max-classes-per-file": () => require("./max-classes-per-file"), "max-depth": () => require("./max-depth"), "max-len": () => require("./max-len"), diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js new file mode 100644 index 00000000000..f8db60c4dba --- /dev/null +++ b/lib/rules/logical-assignment-operators.js @@ -0,0 +1,395 @@ +/** + * @fileoverview Rule to replace assignment expressions with logical operator assignment + * @author Daniel Martens + */ +"use strict"; + +const { isSameReference } = require("./utils/ast-utils.js"); + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ +const astUtils = require("./utils/ast-utils.js"); + + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + + +/** + * Returns true iff either "undefined" or a void expression (eg. "void 0") + * @param {ASTNode} expression Expression to check + * @returns {boolean} True iff "undefined" or "void ..." + */ +function isUndefined(expression) { + return (expression.type === "Identifier" && expression.name === "undefined") || + (expression.type === "UnaryExpression" && expression.operator === "void"); +} + +/** + * Returns true iff the reference is either an identifier or member expression + * @param {ASTNode} expression Expression to check + * @returns {boolean} True for identifiers and member expressions + */ +function isReference(expression) { + return expression.type === "Identifier" || + expression.type === "MemberExpression"; +} + +/** + * Returns true iff the expression checks for nullish with loose equals. + * Examples: value == null, value == void 0 + * @param {ASTNode} expression Test condition + * @returns {boolean} True iff implicit nullish comparison + */ +function isImplicitNullishComparison(expression) { + return expression.type === "BinaryExpression" && + expression.operator === "==" && + isReference(expression.left) && + astUtils.isNullOrUndefined(expression.right); +} + +/** + * Returns true iff the expression checks for undefined and null. + * Example: value === null || value === undefined + * @param {ASTNode} expression Test condition + * @returns {boolean} True iff explicit nullish comparison + */ +function isExplicitNullishComparison(expression) { + return expression.type === "LogicalExpression" && + expression.operator === "||" && + expression.left.type === "BinaryExpression" && + expression.left.operator === "===" && + expression.right.type === "BinaryExpression" && + expression.right.operator === "===" && + astUtils.isSameReference(expression.left.left, expression.right.left, true) && + ((astUtils.isNullLiteral(expression.left.right) && isUndefined(expression.right.right)) || + (isUndefined(expression.left.right) && astUtils.isNullLiteral(expression.right.right))); +} + +/** + * Returns true for: + * truthiness checks: value, Boolean(value), !!value + * falsyness checks: !value, !Boolean(value) + * nullish checks: value == null, value === undefined || value === null + * @param {ASTNode} expression Test condition + * @returns {?{ reference: ASTNode, operator: '??'|'||'|'&&'}} Null if not a known existence + */ +function getExistence(expression) { + const isNegated = expression.type === "UnaryExpression" && expression.operator === "!"; + const base = isNegated ? expression.argument : expression; + + switch (true) { + case isReference(base): + return { reference: base, operator: isNegated ? "||" : "&&" }; + case base.type === "UnaryExpression" && base.operator === "!" && isReference(base.argument): + return { reference: base.argument, operator: "&&" }; + case base.type === "CallExpression" && base.callee.name === "Boolean" && base.arguments.length === 1 && isReference(base.arguments[0]): + return { reference: base.arguments[0], operator: isNegated ? "||" : "&&" }; + case isImplicitNullishComparison(expression): + return { reference: expression.left, operator: "??" }; + case isExplicitNullishComparison(expression): + return { reference: expression.left.left, operator: "??" }; + default: return null; + } +} + +/** + * Returns true iff the node is inside a with block + * @param {ASTNode} node Node to check + * @returns {boolean} True iff passed node is inside a with block + */ +function isInsideWithBlock(node) { + if (node.type === "Program") { + return false; + } + + return node.parent.type === "WithStatement" && node.parent.body === node ? true : isInsideWithBlock(node.parent); +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ +/** @type {import('../shared/types').Rule} */ +module.exports = { + meta: { + type: "suggestion", + + docs: { + description: "Require or disallow assignment logical operator shorthand", + recommended: false, + url: "https://eslint.org/docs/rules/logical-assignment-operators" + }, + + schema: { + type: "array", + oneOf: [{ + items: [ + { const: "always" }, + { + type: "object", + properties: { + enforceForIfStatements: { + type: "boolean" + } + }, + additionalProperties: false + } + ], + minItems: 0, // 0 for allowing passing no options + maxItems: 2 + }, { + items: [{ const: "never" }], + minItems: 1, + maxItems: 1 + }] + }, + fixable: "code", + // eslint-disable-next-line eslint-plugin/require-meta-has-suggestions -- Does not detect conditional suggestions + hasSuggestions: true, + messages: { + assignment: "Assignment (=) can be replaced with operator assignment ({{operator}}).", + useLogicalOperator: "Convert this assignment to use the operator {{ operator }}.", + logical: "Logical expression can be replaced with an assignment ({{ operator }}).", + convertLogical: "Convert this logical expression to an assignment.", + if: "If can be replaced with operator assignment.", + convertIf: "Replace this if with an logical assignment.", + unexpected: "Unexpected logical operator assignment ({{operator}}) shorthand.", + separate: "Separate the logical assignment into an assignment with a logical operator." + } + }, + + create(context) { + const mode = context.options[0] === "never" ? "never" : "always"; + const checkIf = mode === "always" && context.options.length > 1 && context.options[1].enforceForIfStatements; + const sourceCode = context.getSourceCode(); + + /** + * Do not fix if the access could be a getter + * @param {ASTNode} assignment Assignment expression + * @returns {boolean} True iff the fix is safe + */ + function shouldBeFixed(assignment) { + return assignment.left.type === "Identifier" && + (context.getScope().isStrict || !isInsideWithBlock(assignment)); + } + + + /** + * Adds a fixer or suggestion whether on the fix is safe. + * @param {{ messageId: string, node: ASTNode }} descriptor Report descriptor without fix or suggest + * @param {{ messageId: string, fix: Function }} suggestion Adds the fix or the whole suggestion as only element in "suggest" to suggestion + * @param {ASTNode} assignment Used to check if the fix is safe + * @returns {Object} Descriptor with either an added fix or suggestion + */ + function createConditionalFixer(descriptor, suggestion, assignment) { + if (shouldBeFixed(assignment)) { + return { + ...descriptor, + fix: suggestion.fix + }; + } + + return { + ...descriptor, + suggest: [suggestion] + }; + } + + + /** + * Returns the operator token for assignments and binary expressions + * @param {ASTNode} node AssignmentExpression or BinaryExpression + * @returns {import('eslint').AST.Token} Operator token between the left and right expression + */ + function getOperatorToken(node) { + return sourceCode.getFirstTokenBetween(node.left, node.right, token => token.value === node.operator); + } + + if (mode === "never") { + return { + + // foo ||= bar + "AssignmentExpression"(assignment) { + if (!astUtils.isLogicalAssignmentOperator(assignment.operator)) { + return; + } + + const descriptor = { + messageId: "unexpected", + node: assignment, + data: { operator: assignment.operator } + }; + const suggestion = { + messageId: "separate", + data: { operator: assignment.operator }, + *fix(ruleFixer) { + if (sourceCode.getCommentsInside(assignment).length > 0) { + return; + } + + const operatorToken = getOperatorToken(assignment); + + // -> foo = bar + yield ruleFixer.replaceText(operatorToken, "="); + + const assignmentText = sourceCode.getText(assignment.left); + const operator = assignment.operator.slice(0, -1); + + // -> foo = foo || bar + yield ruleFixer.insertTextAfter(operatorToken, ` ${assignmentText} ${operator}`); + + if ( + astUtils.getPrecedence(assignment.right) <= astUtils.getPrecedence({ type: "LogicalExpression", operator }) && + !astUtils.isParenthesised(sourceCode, assignment.right) + ) { + + // -> foo = foo || (bar) + yield ruleFixer.insertTextBefore(assignment.right, "("); + yield ruleFixer.insertTextAfter(assignment.right, ")"); + } + } + }; + + context.report(createConditionalFixer(descriptor, suggestion, assignment)); + } + }; + } + + return { + + // foo = foo || bar + "AssignmentExpression[operator='='][right.type='LogicalExpression']"(assignment) { + if (!astUtils.isSameReference(assignment.left, assignment.right.left, true)) { + return; + } + + const descriptor = { + messageId: "assignment", + node: assignment, + data: { operator: assignment.right.operator } + }; + const suggestion = { + messageId: "useLogicalOperator", + data: { operator: assignment.operator }, + *fix(ruleFixer) { + if (sourceCode.getCommentsInside(assignment).length > 0) { + return; + } + + const assignmentOperatorToken = getOperatorToken(assignment); + + // -> foo ||= foo || bar + yield ruleFixer.insertTextBefore(assignmentOperatorToken, assignment.right.operator); + + // -> foo ||= bar + yield ruleFixer.removeRange([assignment.right.range[0], assignment.right.right.range[0]]); + + if (astUtils.isParenthesised(sourceCode, assignment.right.right)) { + + // Opening parenthesis already removed + const closingParenthesis = sourceCode.getTokenAfter(assignment.right.right); + + yield ruleFixer.remove(closingParenthesis); + } + } + }; + + context.report(createConditionalFixer(descriptor, suggestion, assignment)); + }, + + // foo || (foo = bar) + 'LogicalExpression[right.type="AssignmentExpression"][right.operator="="]'(logical) { + if (!astUtils.isSameReference(logical.left, logical.right.left)) { + return; + } + + const descriptor = { + messageId: "logical", + node: logical, + data: { operator: logical.operator } + }; + const suggestion = { + messageId: "convertLogical", + data: { operator: logical.right.operator }, + fix(ruleFixer) { + if (sourceCode.getCommentsInside(logical).length > 0) { + return null; + } + + // Right side has to be parenthesized, otherwise would be parsed as (foo || foo) = bar which is illegal + const leftToStartRight = [logical.range[0], logical.right.range[0]]; + const removeStart = ruleFixer.removeRange(leftToStartRight); // Also removes start parenthesis + + const endParenthesis = sourceCode.getTokenAfter(logical.right); + const removeEndParenthesis = ruleFixer.remove(endParenthesis); + + const operatorToken = getOperatorToken(logical.right); + const changeOperator = ruleFixer.insertTextBefore(operatorToken, logical.operator); + + return [ + removeStart, // -> foo = bar) + removeEndParenthesis, // -> foo = bar + changeOperator // -> foo ||= bar + ]; + } + }; + + context.report(createConditionalFixer(descriptor, suggestion, logical.right)); + + }, + + // if (foo) foo = bar + "IfStatement[alternate=null]"(ifNode) { + if (!checkIf) { + return; + } + + const hasBody = ifNode.consequent.type === "BlockStatement"; + + if (hasBody && ifNode.consequent.body.length !== 1) { + return; + } + + const body = hasBody ? ifNode.consequent.body[0] : ifNode.consequent; + const existence = getExistence(ifNode.test); + + if ( + body.type === "ExpressionStatement" && + body.expression.type === "AssignmentExpression" && + body.expression.operator === "=" && + existence !== null && + isSameReference(existence.reference, body.expression.left) + ) { + const descriptor = { + messageId: "if", + node: ifNode + }; + const suggestion = { + messageId: "convertIf", + fix(ruleFixer) { + if (sourceCode.getCommentsInside(ifNode).length > 0) { + return null; + } + + const operatorToken = getOperatorToken(body.expression); + const logicalOperator = ruleFixer.insertTextBefore(operatorToken, existence.operator); + + const removeIf = ruleFixer.removeRange([ifNode.range[0], body.range[0]]); + const removeAfter = ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // Only present if "if" had a body + + return [ + removeIf, // -> foo = bar + logicalOperator, // -> foo ||= bar + removeAfter // -> foo ||= bar + ]; + } + }; + + context.report(createConditionalFixer(descriptor, suggestion, body.expression)); + } + } + }; + } +}; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js new file mode 100644 index 00000000000..7f2d0095de6 --- /dev/null +++ b/tests/lib/rules/logical-assignment-operators.js @@ -0,0 +1,827 @@ +/** + * @fileoverview Tests for logical-assignment-operators. + * @author Daniel Martens + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/logical-assignment-operators"), + { RuleTester } = require("../../../lib/rule-tester"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2021 } }); + +ruleTester.run("logical-assignment-operators", rule, { + valid: [ + + // Unrelated + "a || b", + "a && b", + "a ?? b", + "a || a || b", + "var a = a || b", + "a === undefined ? a : b", + "while (a) a = b", + + // Preferred + "a ||= b", + "a &&= b", + "a ??= b", + + // > Operator + "a += a || b", + "a *= a || b", + "a ||= a || b", + "a &&= a || b", + + // > Right + "a = a", + "a = b", + "a = a === b", + "a = a + b", + "a = a / b", + "a = fn(a) || b", + + // > Reference + "a = false || c", + "a = f() || g()", + "a = b || c", + "a = b || a", + "object.a = object.b || c", + "[a] = a || b", + "({ a } = a || b)", + + // Logical + "(a = b) || a", + "a + (a = b)", + "a || (b ||= c)", + "a || (b &&= c)", + "a || b === 0", + "a || fn()", + "a || (b && c)", + "a || (b ?? c)", + + // > Reference + "a || (b = c)", + "a || (a ||= b)", + "fn() || (a = b)", + "a.b || (a = b)", + + // If + "if (a) a = b", + { + code: "if (a) a = b", + options: ["always", { enforceForIfStatements: false }] + }, { + code: "if (a) { a = b } else {}", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) { a = b } else if (a) {}", + options: ["always", { enforceForIfStatements: true }] + }, + + // > Body + { + code: "if (a) {}", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) { before; a = b }", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) { a = b; after }", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) throw new Error()", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) a", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) a ||= b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) b = a", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) { a() }", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a) { a += a || b }", + options: ["always", { enforceForIfStatements: true }] + }, + + // > Test + { + code: "if (true) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (predicate(a)) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a?.b) a.b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (!a?.b) a.b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === b) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a != null) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null == a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null && a === undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === 0 || a === undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === 1) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a == null || a == undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === !0) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === +0) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === null) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === undefined || a === void 0) a = b", + options: ["always", { enforceForIfStatements: true }] + }, + + // > Reference + { + code: "if (a) b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (!a) b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (!!a) b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a == null) b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === undefined) b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || b === undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || b === undefined) b = c", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (Boolean(a)) b = c", + options: ["always", { enforceForIfStatements: true }] + }, + + // Never + { + code: "a = a || b", + options: ["never"] + }, { + code: "a = a && b", + options: ["never"] + }, { + code: "a = a ?? b", + options: ["never"] + }, { + code: "a = b", + options: ["never"] + }, { + code: "a += b", + options: ["never"] + }, { + code: "a -= b", + options: ["never"] + }, { + code: "a.b = a.b || c", + options: ["never"] + } + ], + invalid: [ + + // Assignment + { + code: "a = a || b", + output: "a ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a && b", + output: "a &&= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "&&" }, suggestions: [] }] + }, { + code: "a = a ?? b", + output: "a ??= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "??" }, suggestions: [] }] + }, { + code: "foo = foo || bar", + output: "foo ||= bar", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, + + // > Right + { + code: "a = a || fn()", + output: "a ||= fn()", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || b && c", + output: "a ||= b && c", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || (b || c)", + output: "a ||= b || c", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || (b ? c : d)", + output: "a ||= b ? c : d", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, + + // > Comments + { + code: "/* before */ a = a || b", + output: "/* before */ a ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || b // after", + output: "a ||= b // after", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a /* between */ = a || b", + output: null, + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || /* between */ b", + output: null, + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, + + // > Parenthesis + { + code: "(a) = a || b", + output: "(a) ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = (a) || b", + output: "a ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || (b)", + output: "a ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "(a = a || b)", + output: "(a ||= b)", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, + + // > Suggestions + { + code: "a.b = a.b ?? c", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "a.b ??= c" + }] + }] + }, { + code: "a.b.c = a.b.c ?? d", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "a.b.c ??= d" + }] + }] + }, { + code: "a[b] = a[b] ?? c", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "a[b] ??= c" + }] + }] + }, { + code: "this.prop = this.prop ?? {}", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "this.prop ??= {}" + }] + }] + }, + + // > With + { + code: "with (object) a = a || b", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "with (object) a ||= b" + }] + }] + }, { + code: "with (object) { a = a || b }", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "with (object) { a ||= b }" + }] + }] + }, { + code: "with (object) { if (condition) a = a || b }", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "with (object) { if (condition) a ||= b }" + }] + }] + }, { + code: "with (a = a || b) {}", + output: "with (a ||= b) {}", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "with (object) {} a = a || b", + output: "with (object) {} a ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = a || b; with (object) {}", + output: "a ||= b; with (object) {}", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "if (condition) a = a || b", + output: "if (condition) a ||= b", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, + + // Logical + { + code: "a || (a = b)", + output: "a ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a && (a = b)", + output: "a &&= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "&&" } }] + }, { + code: "a ?? (a = b)", + output: "a ??= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "??" } }] + }, { + code: "foo ?? (foo = bar)", + output: "foo ??= bar", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "??" } }] + }, + + // > Right + { + code: "a || (a = 0)", + output: "a ||= 0", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || (a = fn())", + output: "a ||= fn()", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || (a = (b || c))", + output: "a ||= (b || c)", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, + + // > Parenthesis + { + code: "(a) || (a = b)", + output: "a ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || ((a) = b)", + output: "(a) ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || (a = (b))", + output: "a ||= (b)", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, + + // > Comments + { + code: "/* before */ a || (a = b)", + output: "/* before */ a ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || (a = b) // after", + output: "a ||= b // after", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a /* between */ || (a = b)", + output: null, + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || /* between */ (a = b)", + output: null, + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, + + // > Suggestions + { + code: "a.b || (a.b = c)", + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "convertLogical", + output: "a.b ||= c" + }] + }] + }, { + code: "foo.bar || (foo.bar = baz)", + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "convertLogical", + output: "foo.bar ||= baz" + }] + }] + }, { + code: "with (object) a.b || (a.b = c)", + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "convertLogical", + output: "with (object) a.b ||= c" + }] + }] + }, + + // If + { + code: "if (a) a = b", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (Boolean(a)) a = b", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (!!a) a = b", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (!a) a = b", + output: "a ||= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (!Boolean(a)) a = b", + output: "a ||= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a == undefined) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a == null) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === null || a === undefined) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === undefined || a === null) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === null || a === void 0) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === null || a === void fn()) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === void 0 || a === null) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) { a = b }", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Parenthesis + { + code: "if ((a)) a = b", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) (a) = b", + output: "(a) &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) a = (b)", + output: "a &&= (b)", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) (a = b)", + output: "(a &&= b)", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Spacing + { + code: "if (a) a = b", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a)\n a = b", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) {\n a = b \n}", + output: "a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Comments + { + code: "/* before */ if (a) a = b", + output: "/* before */ a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) a = b /* after */", + output: "a &&= b /* after */", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) /* between */ a = b", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) a = /* between */ b", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Suggestions + { + code: "if (a.b) a.b = b", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "a.b &&= b" + }] + }] + }, { + code: "if (a[b].c) a[b].c = d", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "a[b].c &&= d" + }] + }] + }, { + code: "with (object) if (a.b) a.b = b", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "with (object) a.b &&= b" + }] + }] + }, + + // Never + { + code: "a ||= b", + output: "a = a || b", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a &&= b", + output: "a = a && b", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "&&=" } }] + }, { + code: "a ??= b", + output: "a = a ?? b", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "??=" } }] + }, { + code: "foo ||= bar", + output: "foo = foo || bar", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, + + // > Suggestions + { + code: "a.b ||= c", + output: null, + options: ["never"], + errors: [{ + messageId: "unexpected", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "separate", + output: "a.b = a.b || c" + }] + }] + }, { + code: "a[b] ||= c", + output: null, + options: ["never"], + errors: [{ + messageId: "unexpected", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "separate", + output: "a[b] = a[b] || c" + }] + }] + }, { + code: "this.prop ||= 0", + output: null, + options: ["never"], + errors: [{ + messageId: "unexpected", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "separate", + output: "this.prop = this.prop || 0" + }] + }] + }, { + code: "with (object) a ||= b", + output: null, + options: ["never"], + errors: [{ + messageId: "unexpected", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "separate", + output: "with (object) a = a || b" + }] + }] + }, + + // > Parenthesis + { + code: "(a) ||= b", + output: "(a) = a || b", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a ||= (b)", + output: "a = a || (b)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "(a ||= b)", + output: "(a = a || b)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, + + // > Comments + { + code: "/* before */ a ||= b", + output: "/* before */ a = a || b", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a ||= b // after", + output: "a = a || b // after", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a /* before */ ||= b", + output: null, + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a ||= /* after */ b", + output: null, + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, + + // > Precedence + { + code: "a ||= b && c", + output: "a = a || b && c", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a &&= b || c", + output: "a = a && (b || c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "&&=" } }] + }, { + code: "a ||= b || c", + output: "a = a || (b || c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "||=" } }] + }, { + code: "a &&= b && c", + output: "a = a && (b && c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "&&=" } }] + }] +}); diff --git a/tools/rule-types.json b/tools/rule-types.json index d69028448a1..9867d6b2f33 100644 --- a/tools/rule-types.json +++ b/tools/rule-types.json @@ -59,6 +59,7 @@ "lines-around-comment": "layout", "lines-around-directive": "layout", "lines-between-class-members": "layout", + "logical-assignment-operators": "suggestion", "max-classes-per-file": "suggestion", "max-depth": "suggestion", "max-len": "layout", From e320bfd5f7e039fbd592e1c2ac301d155e7c6d23 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Mon, 11 Jul 2022 17:57:07 +0200 Subject: [PATCH 02/37] docs: update rule documentation - State the concept of the rule clearer - Increase the heading level for options --- docs/src/rules/logical-assignment-operators.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 5ea6e3158b9..989c349863d 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -12,11 +12,12 @@ Require or disallow assignment logical operator shorthand ## Rule Details This rule requires or disallows logical assignment operator shorthand. -On the one hand the shorthand makes it clear that the variable reference is the same. -On the other hand the combined operator may be seen as "magic". -This is a feature introduced in ES2021. +ES2021 introduces the assignment operator shorthand for the logical operators `||`, `&&` and `??`. +Before this was only allowed for mathematical operations such as `+` or `*` (see the rule [operator-assignment](./operator-assignment)). +The shorthand can be used if the assignment target and the left expression of a logical expression are the same. +For example `a = a || b` can be shortened to `a ||= b`. -## Options +### Options This rule has a string and an object option. String option: @@ -30,7 +31,7 @@ Object option (only available if string option is set to `"always"`): * `"enforceForIfStatements": true` Check for equivalent if statements -### always +#### always Examples of **incorrect** code for this rule with the default `"always"` option: @@ -60,7 +61,7 @@ a += b ::: -### never +#### never Examples of **incorrect** code for this rule with the `"never"` option: @@ -88,7 +89,7 @@ a = a ?? b ::: -### enforceForIfStatements +#### enforceForIfStatements This option checks for additional patterns with if statements which could be expressed with the logical assignment operator. ::: correct From e3e4672f27d8ce03abcc08db0fc8ff1fed42c98c Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Mon, 11 Jul 2022 18:41:33 +0200 Subject: [PATCH 03/37] chore: unify ast-utils import --- lib/rules/logical-assignment-operators.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index f8db60c4dba..0fcf8ca8304 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -4,8 +4,6 @@ */ "use strict"; -const { isSameReference } = require("./utils/ast-utils.js"); - //------------------------------------------------------------------------------ // Requirements //------------------------------------------------------------------------------ @@ -360,7 +358,7 @@ module.exports = { body.expression.type === "AssignmentExpression" && body.expression.operator === "=" && existence !== null && - isSameReference(existence.reference, body.expression.left) + astUtils.isSameReference(existence.reference, body.expression.left) ) { const descriptor = { messageId: "if", From f667a30e7648a6f901e039ba3ecd3ac0e105b32f Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Mon, 11 Jul 2022 20:31:55 +0200 Subject: [PATCH 04/37] fix: only check for void 0 in undefined checks --- lib/rules/logical-assignment-operators.js | 2 +- tests/lib/rules/logical-assignment-operators.js | 14 +++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 0fcf8ca8304..89c1f71be1a 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -22,7 +22,7 @@ const astUtils = require("./utils/ast-utils.js"); */ function isUndefined(expression) { return (expression.type === "Identifier" && expression.name === "undefined") || - (expression.type === "UnaryExpression" && expression.operator === "void"); + (expression.type === "UnaryExpression" && expression.operator === "void" && expression.argument.type === "Literal" && expression.argument.value === 0); } /** diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 7f2d0095de6..e922c0504c8 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -168,6 +168,15 @@ ruleTester.run("logical-assignment-operators", rule, { }, { code: "if (a === undefined || a === void 0) a = b", options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === void void 0) a = b", + output: "a ??= b" + }, { + code: "if (a === null || a === void 'string') a = b", + output: "a ??= b" + }, { + code: "if (a === null || a === void fn()) a = b", + output: "a ??= b" }, // > Reference @@ -563,11 +572,6 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a ??= b", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] - }, { - code: "if (a === null || a === void fn()) a = b", - output: "a ??= b", - options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a === void 0 || a === null) a = b", output: "a ??= b", From b0b978abf75faad8fd3f7871afc59407ee068d7b Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 12 Jul 2022 11:15:52 +0200 Subject: [PATCH 05/37] fix: always fix for the logical pattern --- lib/rules/logical-assignment-operators.js | 58 ++++++++----------- .../lib/rules/logical-assignment-operators.js | 38 +++--------- 2 files changed, 31 insertions(+), 65 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 89c1f71be1a..c98c790c684 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -150,7 +150,6 @@ module.exports = { assignment: "Assignment (=) can be replaced with operator assignment ({{operator}}).", useLogicalOperator: "Convert this assignment to use the operator {{ operator }}.", logical: "Logical expression can be replaced with an assignment ({{ operator }}).", - convertLogical: "Convert this logical expression to an assignment.", if: "If can be replaced with operator assignment.", convertIf: "Replace this if with an logical assignment.", unexpected: "Unexpected logical operator assignment ({{operator}}) shorthand.", @@ -299,43 +298,34 @@ module.exports = { // foo || (foo = bar) 'LogicalExpression[right.type="AssignmentExpression"][right.operator="="]'(logical) { - if (!astUtils.isSameReference(logical.left, logical.right.left)) { - return; - } - - const descriptor = { - messageId: "logical", - node: logical, - data: { operator: logical.operator } - }; - const suggestion = { - messageId: "convertLogical", - data: { operator: logical.right.operator }, - fix(ruleFixer) { - if (sourceCode.getCommentsInside(logical).length > 0) { - return null; - } - - // Right side has to be parenthesized, otherwise would be parsed as (foo || foo) = bar which is illegal - const leftToStartRight = [logical.range[0], logical.right.range[0]]; - const removeStart = ruleFixer.removeRange(leftToStartRight); // Also removes start parenthesis - - const endParenthesis = sourceCode.getTokenAfter(logical.right); - const removeEndParenthesis = ruleFixer.remove(endParenthesis); + if (astUtils.isSameReference(logical.left, logical.right.left)) { + context.report({ + messageId: "logical", + node: logical, + data: { operator: logical.operator }, + fix(ruleFixer) { + if (sourceCode.getCommentsInside(logical).length > 0) { + return null; + } - const operatorToken = getOperatorToken(logical.right); - const changeOperator = ruleFixer.insertTextBefore(operatorToken, logical.operator); + // Right side has to be parenthesized, otherwise would be parsed as (foo || foo) = bar which is illegal + const leftToStartRight = [logical.range[0], logical.right.range[0]]; + const removeStart = ruleFixer.removeRange(leftToStartRight); // Also removes start parenthesis - return [ - removeStart, // -> foo = bar) - removeEndParenthesis, // -> foo = bar - changeOperator // -> foo ||= bar - ]; - } - }; + const endParenthesis = sourceCode.getTokenAfter(logical.right); + const removeEndParenthesis = ruleFixer.remove(endParenthesis); - context.report(createConditionalFixer(descriptor, suggestion, logical.right)); + const operatorToken = getOperatorToken(logical.right); + const changeOperator = ruleFixer.insertTextBefore(operatorToken, logical.operator); + return [ + removeStart, // -> foo = bar) + removeEndParenthesis, // -> foo = bar + changeOperator // -> foo ||= bar + ]; + } + }); + } }, // if (foo) foo = bar diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index e922c0504c8..ce91cd01456 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -482,43 +482,19 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, - // > Suggestions + // > Possible Getter { code: "a.b || (a.b = c)", - output: null, - errors: [{ - messageId: "logical", - type: "LogicalExpression", - data: { operator: "||" }, - suggestions: [{ - messageId: "convertLogical", - output: "a.b ||= c" - }] - }] + output: "a.b ||= c", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, { code: "foo.bar || (foo.bar = baz)", - output: null, - errors: [{ - messageId: "logical", - type: "LogicalExpression", - data: { operator: "||" }, - suggestions: [{ - messageId: "convertLogical", - output: "foo.bar ||= baz" - }] - }] + output: "foo.bar ||= baz", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, { code: "with (object) a.b || (a.b = c)", - output: null, - errors: [{ - messageId: "logical", - type: "LogicalExpression", - data: { operator: "||" }, - suggestions: [{ - messageId: "convertLogical", - output: "with (object) a.b ||= c" - }] - }] + output: "with (object) a.b ||= c", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, // If From 21cc4198255f26f7091fe06d566fa94e70deb709 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 12 Jul 2022 12:51:57 +0200 Subject: [PATCH 06/37] feat: support yoda conditions in if conditions --- lib/rules/logical-assignment-operators.js | 81 +++++--- .../lib/rules/logical-assignment-operators.js | 175 +++++++++++++++++- 2 files changed, 228 insertions(+), 28 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index c98c790c684..6adad8594ab 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -9,7 +9,6 @@ //------------------------------------------------------------------------------ const astUtils = require("./utils/ast-utils.js"); - //------------------------------------------------------------------------------ // Helpers //------------------------------------------------------------------------------ @@ -18,11 +17,18 @@ const astUtils = require("./utils/ast-utils.js"); /** * Returns true iff either "undefined" or a void expression (eg. "void 0") * @param {ASTNode} expression Expression to check + * @param {import('eslint-scope').Scope} scope Scope of the expression * @returns {boolean} True iff "undefined" or "void ..." */ -function isUndefined(expression) { - return (expression.type === "Identifier" && expression.name === "undefined") || - (expression.type === "UnaryExpression" && expression.operator === "void" && expression.argument.type === "Literal" && expression.argument.value === 0); +function isUndefined(expression, scope) { + if (expression.type === "Identifier" && expression.name === "undefined") { + return astUtils.isReferenceToGlobalVariable(scope, expression); + } + + return expression.type === "UnaryExpression" && + expression.operator === "void" && + expression.argument.type === "Literal" && + expression.argument.value === 0; } /** @@ -31,7 +37,7 @@ function isUndefined(expression) { * @returns {boolean} True for identifiers and member expressions */ function isReference(expression) { - return expression.type === "Identifier" || + return (expression.type === "Identifier" && expression.name !== "undefined") || expression.type === "MemberExpression"; } @@ -39,31 +45,54 @@ function isReference(expression) { * Returns true iff the expression checks for nullish with loose equals. * Examples: value == null, value == void 0 * @param {ASTNode} expression Test condition + * @param {import('eslint-scope').Scope} scope Scope of the expression * @returns {boolean} True iff implicit nullish comparison */ -function isImplicitNullishComparison(expression) { - return expression.type === "BinaryExpression" && - expression.operator === "==" && - isReference(expression.left) && - astUtils.isNullOrUndefined(expression.right); +function isImplicitNullishComparison(expression, scope) { + if (expression.type !== "BinaryExpression" || expression.operator !== "==") { + return false; + } + + const reference = isReference(expression.left) ? "left" : "right"; + const nullish = reference === "left" ? "right" : "left"; + + return isReference(expression[reference]) && + (astUtils.isNullLiteral(expression[nullish]) || isUndefined(expression[nullish], scope)); } /** - * Returns true iff the expression checks for undefined and null. - * Example: value === null || value === undefined - * @param {ASTNode} expression Test condition - * @returns {boolean} True iff explicit nullish comparison + * Condition with two equal comparisons. + * @param {ASTNode} expression Condition + * @returns {boolean} True iff matches ? === ? || ? === ? */ -function isExplicitNullishComparison(expression) { +function isDoubleComparison(expression) { return expression.type === "LogicalExpression" && expression.operator === "||" && expression.left.type === "BinaryExpression" && expression.left.operator === "===" && expression.right.type === "BinaryExpression" && - expression.right.operator === "===" && - astUtils.isSameReference(expression.left.left, expression.right.left, true) && - ((astUtils.isNullLiteral(expression.left.right) && isUndefined(expression.right.right)) || - (isUndefined(expression.left.right) && astUtils.isNullLiteral(expression.right.right))); + expression.right.operator === "==="; +} + +/** + * Returns true iff the expression checks for undefined and null. + * Example: value === null || value === undefined + * @param {ASTNode} expression Test condition + * @param {import('eslint-scope').Scope} scope Scope of the expression + * @returns {boolean} True iff explicit nullish comparison + */ +function isExplicitNullishComparison(expression, scope) { + if (!isDoubleComparison(expression)) { + return false; + } + const leftReference = isReference(expression.left.left) ? "left" : "right"; + const leftNullish = leftReference === "left" ? "right" : "left"; + const rightReference = isReference(expression.right.left) ? "left" : "right"; + const rightNullish = rightReference === "left" ? "right" : "left"; + + return astUtils.isSameReference(expression.left[leftReference], expression.right[rightReference], true) && + ((astUtils.isNullLiteral(expression.left[leftNullish]) && isUndefined(expression.right[rightNullish], scope)) || + (isUndefined(expression.left[leftNullish], scope) && astUtils.isNullLiteral(expression.right[rightNullish]))); } /** @@ -72,9 +101,10 @@ function isExplicitNullishComparison(expression) { * falsyness checks: !value, !Boolean(value) * nullish checks: value == null, value === undefined || value === null * @param {ASTNode} expression Test condition + * @param {import('eslint-scope').Scope} scope Scope of the expression * @returns {?{ reference: ASTNode, operator: '??'|'||'|'&&'}} Null if not a known existence */ -function getExistence(expression) { +function getExistence(expression, scope) { const isNegated = expression.type === "UnaryExpression" && expression.operator === "!"; const base = isNegated ? expression.argument : expression; @@ -85,10 +115,10 @@ function getExistence(expression) { return { reference: base.argument, operator: "&&" }; case base.type === "CallExpression" && base.callee.name === "Boolean" && base.arguments.length === 1 && isReference(base.arguments[0]): return { reference: base.arguments[0], operator: isNegated ? "||" : "&&" }; - case isImplicitNullishComparison(expression): - return { reference: expression.left, operator: "??" }; - case isExplicitNullishComparison(expression): - return { reference: expression.left.left, operator: "??" }; + case isImplicitNullishComparison(expression, scope): + return { reference: isReference(expression.left) ? expression.left : expression.right, operator: "??" }; + case isExplicitNullishComparison(expression, scope): + return { reference: isReference(expression.left.left) ? expression.left.left : expression.left.right, operator: "??" }; default: return null; } } @@ -341,7 +371,8 @@ module.exports = { } const body = hasBody ? ifNode.consequent.body[0] : ifNode.consequent; - const existence = getExistence(ifNode.test); + const scope = context.getScope(); + const existence = getExistence(ifNode.test, scope); if ( body.type === "ExpressionStatement" && diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index ce91cd01456..301e21de771 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -141,9 +141,6 @@ ruleTester.run("logical-assignment-operators", rule, { }, { code: "if (a != null) a = b", options: ["always", { enforceForIfStatements: true }] - }, { - code: "if (null == a) a = b", - options: ["always", { enforceForIfStatements: true }] }, { code: "if (a === null && a === undefined) a = b", options: ["always", { enforceForIfStatements: true }] @@ -179,6 +176,113 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a ??= b" }, + // > Test > Yoda + { + code: "if (a == a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a == b) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null == null) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (undefined == undefined) undefined = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null == x) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null == fn()) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null === a || a === 0) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (0 === a || null === a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (1 === a || a === undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (undefined === a || 1 === a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (a === null || a === b) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (b === undefined || a === null) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null === a || b === a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null === null || undefined === undefined) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null === null || a === a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (undefined === undefined || a === a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (null === undefined || a === a) a = b", + options: ["always", { enforceForIfStatements: true }] + }, + + // > Test > Undefined + { + code: [ + "{", + " const undefined = 0;", + " if (a == undefined) a = b", + "}" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] + }, { + code: [ + "(() => {", + " const undefined = 0;", + " if (condition) {", + " if (a == undefined) a = b", + " }", + "})()" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] + }, { + code: [ + "{", + " if (a == undefined) a = b", + "}", + "var undefined = 0;" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] + }, { + code: [ + "{", + " const undefined = 0;", + " if (undefined == null) undefined = b", + "}" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] + }, { + code: [ + "{", + " const undefined = 0;", + " if (a === undefined || a === null) a = b", + "}" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] + }, { + code: [ + "{", + " const undefined = 0;", + " if (undefined === a || null === a) a = b", + "}" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] + }, + // > Reference { code: "if (a) b = c", @@ -558,6 +662,71 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a &&= b", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: [ + "{ const undefined = 0; }", + "if (a == undefined) a = b" + ].join("\n"), + output: [ + "{ const undefined = 0; }", + "a ??= b" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: [ + "if (a == undefined) a = b", + "{ const undefined = 0; }" + ].join("\n"), + output: [ + "a ??= b", + "{ const undefined = 0; }" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Yoda + { + code: "if (null == a) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (undefined == a) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (undefined === a || a === null) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === undefined || null === a) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (undefined === a || null === a) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (null === a || a === undefined) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a === null || undefined === a) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (null === a || undefined === a) a = b", + output: "a ??= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] }, // > Parenthesis From 8a1da4be3dc1947d13bd342df695b4b9530fc739 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 12 Jul 2022 15:43:25 +0200 Subject: [PATCH 07/37] fix: remove parenthesis around assignment target if necessary --- lib/rules/logical-assignment-operators.js | 26 ++++++----- .../lib/rules/logical-assignment-operators.js | 43 +++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 6adad8594ab..66fd3ac2714 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -387,22 +387,28 @@ module.exports = { }; const suggestion = { messageId: "convertIf", - fix(ruleFixer) { + *fix(ruleFixer) { if (sourceCode.getCommentsInside(ifNode).length > 0) { - return null; + return; } const operatorToken = getOperatorToken(body.expression); - const logicalOperator = ruleFixer.insertTextBefore(operatorToken, existence.operator); + const prevToken = sourceCode.getTokenBefore(ifNode); - const removeIf = ruleFixer.removeRange([ifNode.range[0], body.range[0]]); - const removeAfter = ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // Only present if "if" had a body + if (astUtils.isParenthesised(sourceCode, body.expression.left) && (prevToken !== null && !astUtils.isSemicolonToken(prevToken))) { - return [ - removeIf, // -> foo = bar - logicalOperator, // -> foo ||= bar - removeAfter // -> foo ||= bar - ]; + // Remove all parenthesis as otherwise the parser may combine the expression with the previous + const parens = sourceCode.getTokensBetween({ range: [0, body.range[0]] }, operatorToken).filter(token => + astUtils.isOpeningParenToken(token) || astUtils.isClosingParenToken(token)); + + yield* parens.map(paren => ruleFixer.remove(paren)); // if (foo) (foo) = bar --> if (foo) foo = bar + } + + yield ruleFixer.insertTextBefore(operatorToken, existence.operator); // -> if (foo) foo ||= bar + + yield ruleFixer.removeRange([ifNode.range[0], body.range[0]]); // -> foo ||= bar + + yield ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // -> foo ||= bar, only present if "if" had a body } }; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 301e21de771..932d531ebad 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -752,6 +752,49 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, + // > Removing Parenthesis + { + code: ";if (a) (a) = b", + output: ";(a) &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "fn();if (a) (a) = b", + output: "fn();(a) &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "id\nif (a) (a) = b", + output: "id\na &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "fn()\nif (a) (a) = b", + output: "fn()\na &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "fn()\nif (a) ((a)) = b", + output: "fn()\na &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "fn()\nif (a) ( a ) = b", + output: "fn()\n a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "fn()\nif (a) (a)= b", + output: "fn()\na&&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "fn()\nif (a) (a)\n= b", + output: "fn()\na\n&&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + // > Spacing { code: "if (a) a = b", From 9bd6e23601a81bfa748a52bfb93f6cd6ca846b24 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 12 Jul 2022 16:39:08 +0200 Subject: [PATCH 08/37] fix: parenthesize logical pattern if needed --- lib/rules/logical-assignment-operators.js | 26 +++++++---- .../lib/rules/logical-assignment-operators.js | 46 +++++++++++++++++++ 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 66fd3ac2714..2ba6c4607c8 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -305,6 +305,7 @@ module.exports = { return; } + // No need for parenthesis around the assignment based on precedence as the precedence stays the same even with changed operator const assignmentOperatorToken = getOperatorToken(assignment); // -> foo ||= foo || bar @@ -333,26 +334,31 @@ module.exports = { messageId: "logical", node: logical, data: { operator: logical.operator }, - fix(ruleFixer) { + *fix(ruleFixer) { if (sourceCode.getCommentsInside(logical).length > 0) { - return null; + return; + } + + const requiresParenthesis = logical.parent.type !== "ExpressionStatement" && + (astUtils.getPrecedence({ type: "AssignmentExpression" }) < astUtils.getPrecedence(logical.parent)); + + if (!astUtils.isParenthesised(sourceCode, logical) && requiresParenthesis) { + yield ruleFixer.insertTextBefore(logical, "("); + yield ruleFixer.insertTextAfter(logical, ")"); } // Right side has to be parenthesized, otherwise would be parsed as (foo || foo) = bar which is illegal const leftToStartRight = [logical.range[0], logical.right.range[0]]; - const removeStart = ruleFixer.removeRange(leftToStartRight); // Also removes start parenthesis + + yield ruleFixer.removeRange(leftToStartRight); // -> foo = bar) const endParenthesis = sourceCode.getTokenAfter(logical.right); - const removeEndParenthesis = ruleFixer.remove(endParenthesis); + + yield ruleFixer.remove(endParenthesis); // -> foo = bar const operatorToken = getOperatorToken(logical.right); - const changeOperator = ruleFixer.insertTextBefore(operatorToken, logical.operator); - return [ - removeStart, // -> foo = bar) - removeEndParenthesis, // -> foo = bar - changeOperator // -> foo ||= bar - ]; + yield ruleFixer.insertTextBefore(operatorToken, logical.operator); // -> foo ||= bar } }); } diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 932d531ebad..a591fac5944 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -518,6 +518,25 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] }, + // > Context + { + code: "fn(a = a || b)", + output: "fn(a ||= b)", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "fn((a = a || b))", + output: "fn((a ||= b))", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "(a = a || b) ? c : d", + output: "(a ||= b) ? c : d", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = b = b || c", + output: "a = b ||= c", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, + // Logical { code: "a || (a = b)", @@ -601,6 +620,33 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, + // > Context + { + code: "a || (a = 0) || b", + output: "(a ||= 0) || b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "(a || (a = 0)) || b", + output: "(a ||= 0) || b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || (b || (b = 0))", + output: "a || (b ||= 0)", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a = b || (b = c)", + output: "a = b ||= c", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a || (a = 0) ? b : c", + output: "(a ||= 0) ? b : c", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "fn(a || (a = 0))", + output: "fn(a ||= 0)", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, + // If { code: "if (a) a = b", From a55ae4c6776669a4e6d1484a6ac1adb31aabef0c Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 12 Jul 2022 16:53:26 +0200 Subject: [PATCH 09/37] fix: add semicolon for if patterns with a body if needed --- lib/rules/logical-assignment-operators.js | 8 +++- .../lib/rules/logical-assignment-operators.js | 41 +++++++++++++++++-- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 2ba6c4607c8..3a221443c8a 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -411,10 +411,14 @@ module.exports = { } yield ruleFixer.insertTextBefore(operatorToken, existence.operator); // -> if (foo) foo ||= bar - yield ruleFixer.removeRange([ifNode.range[0], body.range[0]]); // -> foo ||= bar - yield ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // -> foo ||= bar, only present if "if" had a body + + const nextToken = sourceCode.getTokenAfter(body.expression); + + if (hasBody && (nextToken !== null && nextToken.value !== ";")) { + yield ruleFixer.insertTextAfter(ifNode, ";"); + } } }; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index a591fac5944..d2ee4ac142c 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -704,8 +704,8 @@ ruleTester.run("logical-assignment-operators", rule, { options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "if (a) { a = b }", - output: "a &&= b", + code: "if (a) { a = b; }", + output: "a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { @@ -841,6 +841,39 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, + // > Adding semicolon + { + code: "if (a) a = b; fn();", + output: "a &&= b; fn();", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) { a = b }", + output: "a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) { a = b; }\nfn();", + output: "a &&= b;\nfn();", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) { a = b }\nfn();", + output: "a &&= b;\nfn();", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) { a = b } fn();", + output: "a &&= b; fn();", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) { a = b\n} fn();", + output: "a &&= b; fn();", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + // > Spacing { code: "if (a) a = b", @@ -853,8 +886,8 @@ ruleTester.run("logical-assignment-operators", rule, { options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "if (a) {\n a = b \n}", - output: "a &&= b", + code: "if (a) {\n a = b; \n}", + output: "a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, From 9a4b7f8fa9f8e22f47d388a7f02633d4c5c5f584 Mon Sep 17 00:00:00 2001 From: fnx Date: Wed, 13 Jul 2022 00:33:57 +0200 Subject: [PATCH 10/37] fix: remove file extension from import Co-authored-by: Milos Djermanovic --- lib/rules/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/index.js b/lib/rules/index.js index 7038dd81471..565648c09e8 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -72,7 +72,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({ "lines-around-comment": () => require("./lines-around-comment"), "lines-around-directive": () => require("./lines-around-directive"), "lines-between-class-members": () => require("./lines-between-class-members"), - "logical-assignment-operators": () => require("./logical-assignment-operators.js"), + "logical-assignment-operators": () => require("./logical-assignment-operators"), "max-classes-per-file": () => require("./max-classes-per-file"), "max-depth": () => require("./max-depth"), "max-len": () => require("./max-len"), From a548dd42d71c76e4c204069a8f8486bc5728645b Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 13 Jul 2022 14:53:02 +0200 Subject: [PATCH 11/37] fix: check strictness of global scope instead of current scope to avoid checking for with --- lib/rules/logical-assignment-operators.js | 3 ++- .../lib/rules/logical-assignment-operators.js | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 3a221443c8a..b3b095470b7 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -191,6 +191,7 @@ module.exports = { const mode = context.options[0] === "never" ? "never" : "always"; const checkIf = mode === "always" && context.options.length > 1 && context.options[1].enforceForIfStatements; const sourceCode = context.getSourceCode(); + const isStrict = context.getScope().isStrict; /** * Do not fix if the access could be a getter @@ -199,7 +200,7 @@ module.exports = { */ function shouldBeFixed(assignment) { return assignment.left.type === "Identifier" && - (context.getScope().isStrict || !isInsideWithBlock(assignment)); + (isStrict || !isInsideWithBlock(assignment)); } diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index d2ee4ac142c..0c65314c390 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -516,6 +516,28 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (condition) a = a || b", output: "if (condition) a ||= b", errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: [ + "with (object) {", + ' "use strict";', + " a = a || b", + "}" + ].join("\n"), + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: [ + "with (object) {", + ' "use strict";', + " a ||= b", + "}" + ].join("\n") + }] + }] }, // > Context From 613c2d3890fa18c0a4012f466bf183b34f7ba341 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 13 Jul 2022 15:04:12 +0200 Subject: [PATCH 12/37] fix: check for mixed ?? and ||/&& operators in the fix for never --- lib/rules/logical-assignment-operators.js | 10 ++++--- .../lib/rules/logical-assignment-operators.js | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index b3b095470b7..3b4481bd3be 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -268,10 +268,12 @@ module.exports = { // -> foo = foo || bar yield ruleFixer.insertTextAfter(operatorToken, ` ${assignmentText} ${operator}`); - if ( - astUtils.getPrecedence(assignment.right) <= astUtils.getPrecedence({ type: "LogicalExpression", operator }) && - !astUtils.isParenthesised(sourceCode, assignment.right) - ) { + const precedence = astUtils.getPrecedence(assignment.right) <= astUtils.getPrecedence({ type: "LogicalExpression", operator }); + + // ?? and || / && cannot be mixed but have same precedence + const mixed = assignment.operator === "??=" && astUtils.isLogicalExpression(assignment.right); + + if (!astUtils.isParenthesised(sourceCode, assignment.right) && (precedence || mixed)) { // -> foo = foo || (bar) yield ruleFixer.insertTextBefore(assignment.right, "("); diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 0c65314c390..d474c412b6e 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -1116,5 +1116,33 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a = a && (b && c)", options: ["never"], errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "&&=" } }] + }, + + // > Mixed + { + code: "a ??= b || c", + output: "a = a ?? (b || c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "??=" } }] + }, { + code: "a ??= b && c", + output: "a = a ?? (b && c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "??=" } }] + }, { + code: "a ??= b ?? c", + output: "a = a ?? (b ?? c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "??=" } }] + }, { + code: "a ??= (b || c)", + output: "a = a ?? (b || c)", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "??=" } }] + }, { + code: "a ??= b + c", + output: "a = a ?? b + c", + options: ["never"], + errors: [{ messageId: "unexpected", type: "AssignmentExpression", data: { operator: "??=" } }] }] }); From ec1e1a567751502fdea2ce18beb60efc0f9390bc Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 13 Jul 2022 17:44:10 +0200 Subject: [PATCH 13/37] fix: check previous token for continuation problems in the if fix --- lib/rules/logical-assignment-operators.js | 20 ++++++++----- .../lib/rules/logical-assignment-operators.js | 28 +++++++++---------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 3b4481bd3be..18cdf44fb9d 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -401,18 +401,24 @@ module.exports = { return; } - const operatorToken = getOperatorToken(body.expression); + const firstBodyToken = sourceCode.getFirstToken(body); const prevToken = sourceCode.getTokenBefore(ifNode); - if (astUtils.isParenthesised(sourceCode, body.expression.left) && (prevToken !== null && !astUtils.isSemicolonToken(prevToken))) { - - // Remove all parenthesis as otherwise the parser may combine the expression with the previous - const parens = sourceCode.getTokensBetween({ range: [0, body.range[0]] }, operatorToken).filter(token => - astUtils.isOpeningParenToken(token) || astUtils.isClosingParenToken(token)); + if ( + prevToken !== null && + prevToken.value !== ";" && + prevToken.value !== "{" && + firstBodyToken.type !== "Identifier" && + firstBodyToken.type !== "Keyword" + ) { - yield* parens.map(paren => ruleFixer.remove(paren)); // if (foo) (foo) = bar --> if (foo) foo = bar + // Do not fix if the fixed statement could be part of the previous statement (eg. fn() if (a == null) (a) = b --> fn()(a) ??= b) + return; } + + const operatorToken = getOperatorToken(body.expression); + yield ruleFixer.insertTextBefore(operatorToken, existence.operator); // -> if (foo) foo ||= bar yield ruleFixer.removeRange([ifNode.range[0], body.range[0]]); // -> foo ||= bar yield ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // -> foo ||= bar, only present if "if" had a body diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index d474c412b6e..661bc760d5f 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -820,45 +820,45 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, - // > Removing Parenthesis + // > Previous statement { code: ";if (a) (a) = b", output: ";(a) &&= b", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "fn();if (a) (a) = b", - output: "fn();(a) &&= b", + code: "{ if (a) (a) = b }", + output: "{ (a) &&= b }", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "id\nif (a) (a) = b", - output: "id\na &&= b", + code: "fn();if (a) (a) = b", + output: "fn();(a) &&= b", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "fn()\nif (a) (a) = b", + code: "fn()\nif (a) a = b", output: "fn()\na &&= b", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "fn()\nif (a) ((a)) = b", - output: "fn()\na &&= b", + code: "id\nif (a) (a) = b", + output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "fn()\nif (a) ( a ) = b", - output: "fn()\n a &&= b", + code: "object.prop\nif (a) (a) = b", + output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "fn()\nif (a) (a)= b", - output: "fn()\na&&= b", + code: "object[computed]\nif (a) (a) = b", + output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { - code: "fn()\nif (a) (a)\n= b", - output: "fn()\na\n&&= b", + code: "fn()\nif (a) (a) = b", + output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, From f964605716af8bf8f3b62a80c151a7a726ee599b Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Sat, 30 Jul 2022 12:51:42 +0200 Subject: [PATCH 14/37] feat: support else if (and fix suggest cases for if) --- lib/rules/logical-assignment-operators.js | 30 ++- .../lib/rules/logical-assignment-operators.js | 196 +++++++++++++----- 2 files changed, 167 insertions(+), 59 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 18cdf44fb9d..4f07c437d10 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -198,7 +198,7 @@ module.exports = { * @param {ASTNode} assignment Assignment expression * @returns {boolean} True iff the fix is safe */ - function shouldBeFixed(assignment) { + function canBeFixed(assignment) { return assignment.left.type === "Identifier" && (isStrict || !isInsideWithBlock(assignment)); } @@ -208,11 +208,11 @@ module.exports = { * Adds a fixer or suggestion whether on the fix is safe. * @param {{ messageId: string, node: ASTNode }} descriptor Report descriptor without fix or suggest * @param {{ messageId: string, fix: Function }} suggestion Adds the fix or the whole suggestion as only element in "suggest" to suggestion - * @param {ASTNode} assignment Used to check if the fix is safe + * @param {boolean} shouldBeFixed Fix iff the condition is true * @returns {Object} Descriptor with either an added fix or suggestion */ - function createConditionalFixer(descriptor, suggestion, assignment) { - if (shouldBeFixed(assignment)) { + function createConditionalFixer(descriptor, suggestion, shouldBeFixed) { + if (shouldBeFixed) { return { ...descriptor, fix: suggestion.fix @@ -282,7 +282,7 @@ module.exports = { } }; - context.report(createConditionalFixer(descriptor, suggestion, assignment)); + context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment))); } }; } @@ -327,7 +327,7 @@ module.exports = { } }; - context.report(createConditionalFixer(descriptor, suggestion, assignment)); + context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment))); }, // foo || (foo = bar) @@ -397,12 +397,17 @@ module.exports = { const suggestion = { messageId: "convertIf", *fix(ruleFixer) { - if (sourceCode.getCommentsInside(ifNode).length > 0) { + const isElseIf = ifNode.parent.type === "IfStatement"; + + if ( + sourceCode.getCommentsInside(ifNode).length > 0 || + (isElseIf && sourceCode.getCommentsBefore(ifNode).length > 0) + ) { return; } const firstBodyToken = sourceCode.getFirstToken(body); - const prevToken = sourceCode.getTokenBefore(ifNode); + const prevToken = sourceCode.getTokenBefore(ifNode, { skip: isElseIf ? 1 : 0 }); if ( prevToken !== null && @@ -420,7 +425,11 @@ module.exports = { const operatorToken = getOperatorToken(body.expression); yield ruleFixer.insertTextBefore(operatorToken, existence.operator); // -> if (foo) foo ||= bar - yield ruleFixer.removeRange([ifNode.range[0], body.range[0]]); // -> foo ||= bar + + const ifNodeStart = isElseIf ? sourceCode.getTokenBefore(ifNode) : ifNode; + + yield ruleFixer.removeRange([ifNodeStart.range[0], body.range[0]]); // -> foo ||= bar + yield ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // -> foo ||= bar, only present if "if" had a body const nextToken = sourceCode.getTokenAfter(body.expression); @@ -430,8 +439,9 @@ module.exports = { } } }; + const shouldBeFixed = ifNode.test.type !== "LogicalExpression"; - context.report(createConditionalFixer(descriptor, suggestion, body.expression)); + context.report(createConditionalFixer(descriptor, suggestion, shouldBeFixed)); } } }; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 661bc760d5f..ba33df721d6 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -84,6 +84,12 @@ ruleTester.run("logical-assignment-operators", rule, { }, { code: "if (a) { a = b } else if (a) {}", options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (unrelated) {} else if (a) a = b; else {}", + options: ["always", { enforceForIfStatements: true }] + }, { + code: "if (unrelated) {} else if (a) a = b; else if (unrelated) {}", + options: ["always", { enforceForIfStatements: true }] }, // > Body @@ -387,6 +393,10 @@ ruleTester.run("logical-assignment-operators", rule, { code: "a /* between */ = a || b", output: null, errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + }, { + code: "a = /** @type */ a || b", + output: null, + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] }, { code: "a = a || /* between */ b", output: null, @@ -643,6 +653,11 @@ ruleTester.run("logical-assignment-operators", rule, { }, // > Context + { + code: "a = a.b || (a.b = {})", + output: "a = a.b ||= {}", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" }, suggestions: [] }] + }, { code: "a || (a = 0) || b", output: "(a ||= 0) || b", @@ -707,24 +722,40 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a === null || a === undefined) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (a === undefined || a === null) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (a === null || a === void 0) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (a === void 0 || a === null) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (a) { a = b; }", output: "a &&= b;", @@ -759,42 +790,66 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (null == a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (undefined == a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (undefined === a || a === null) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (a === undefined || null === a) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (undefined === a || null === a) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (null === a || a === undefined) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (a === null || undefined === a) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, { code: "if (null === a || undefined === a) a = b", - output: "a ??= b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ messageId: "convertIf", output: "a ??= b" }] + }] }, // > Parenthesis @@ -937,43 +992,86 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, - // > Suggestions + // > Members { code: "if (a.b) a.b = b", - output: null, + output: "a.b &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ - messageId: "convertIf", - output: "a.b &&= b" - }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a[b].c) a[b].c = d", - output: null, + output: "a[b].c &&= d", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ - messageId: "convertIf", - output: "a[b].c &&= d" - }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "with (object) if (a.b) a.b = b", + output: "with (object) a.b &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Else if + { + code: "if (unrelated) {} else if (a) a = b;", + output: "if (unrelated) {} a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (a) {} else if (b) {} else if (a) a = b;", + output: "if (a) {} else if (b) {} a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) {} else\nif (a) a = b;", + output: "if (unrelated) {} a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) {\n}\nelse if (a) {\na = b;\n}", + output: "if (unrelated) {\n}\na &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) statement; else if (a) a = b;", + output: "if (unrelated) statement; a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) id\nelse if (a) (a) = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ - messageId: "convertIf", - output: "with (object) a.b &&= b" - }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Else if > Comments + { + code: "if (unrelated) { /* body */ } else if (a) a = b;", + output: "if (unrelated) { /* body */ } a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) {} /* before else */ else if (a) a = b;", + output: "if (unrelated) {} /* before else */ a &&= b;", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) {} else // Line\nif (a) a = b;", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) {} else /* Block */ if (a) a = b;", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] + }, + + // > Patterns + { + code: "if (a.b) a.b = a.b.filter(predicate)", + output: "a.b &&= a.b.filter(predicate)", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] }, // Never From ef42c44f0c51e63b647f3aa895df458f79f1f295 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Mon, 8 Aug 2022 16:24:16 +0200 Subject: [PATCH 15/37] fix: do not remove else keyword for else if --- lib/rules/logical-assignment-operators.js | 13 +++-------- .../lib/rules/logical-assignment-operators.js | 23 +++++++++++-------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 4f07c437d10..e1ed15f649c 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -397,17 +397,12 @@ module.exports = { const suggestion = { messageId: "convertIf", *fix(ruleFixer) { - const isElseIf = ifNode.parent.type === "IfStatement"; - - if ( - sourceCode.getCommentsInside(ifNode).length > 0 || - (isElseIf && sourceCode.getCommentsBefore(ifNode).length > 0) - ) { + if (sourceCode.getCommentsInside(ifNode).length > 0) { return; } const firstBodyToken = sourceCode.getFirstToken(body); - const prevToken = sourceCode.getTokenBefore(ifNode, { skip: isElseIf ? 1 : 0 }); + const prevToken = sourceCode.getTokenBefore(ifNode); if ( prevToken !== null && @@ -426,9 +421,7 @@ module.exports = { yield ruleFixer.insertTextBefore(operatorToken, existence.operator); // -> if (foo) foo ||= bar - const ifNodeStart = isElseIf ? sourceCode.getTokenBefore(ifNode) : ifNode; - - yield ruleFixer.removeRange([ifNodeStart.range[0], body.range[0]]); // -> foo ||= bar + yield ruleFixer.removeRange([ifNode.range[0], body.range[0]]); // -> foo ||= bar yield ruleFixer.removeRange([body.range[1], ifNode.range[1]]); // -> foo ||= bar, only present if "if" had a body diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index ba33df721d6..faeb513454d 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -1013,27 +1013,27 @@ ruleTester.run("logical-assignment-operators", rule, { // > Else if { code: "if (unrelated) {} else if (a) a = b;", - output: "if (unrelated) {} a &&= b;", + output: "if (unrelated) {} else a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a) {} else if (b) {} else if (a) a = b;", - output: "if (a) {} else if (b) {} a &&= b;", + output: "if (a) {} else if (b) {} else a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (unrelated) {} else\nif (a) a = b;", - output: "if (unrelated) {} a &&= b;", + output: "if (unrelated) {} else\na &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (unrelated) {\n}\nelse if (a) {\na = b;\n}", - output: "if (unrelated) {\n}\na &&= b;", + output: "if (unrelated) {\n}\nelse a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (unrelated) statement; else if (a) a = b;", - output: "if (unrelated) statement; a &&= b;", + output: "if (unrelated) statement; else a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { @@ -1041,27 +1041,32 @@ ruleTester.run("logical-assignment-operators", rule, { output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] + }, { + code: "if (unrelated) {} else if (a) a = b; else if (c) c = d", + output: "if (unrelated) {} else if (a) a = b; else c &&= d", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement" }] }, // > Else if > Comments { code: "if (unrelated) { /* body */ } else if (a) a = b;", - output: "if (unrelated) { /* body */ } a &&= b;", + output: "if (unrelated) { /* body */ } else a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (unrelated) {} /* before else */ else if (a) a = b;", - output: "if (unrelated) {} /* before else */ a &&= b;", + output: "if (unrelated) {} /* before else */ else a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (unrelated) {} else // Line\nif (a) a = b;", - output: null, + output: "if (unrelated) {} else // Line\na &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (unrelated) {} else /* Block */ if (a) a = b;", - output: null, + output: "if (unrelated) {} else /* Block */ a &&= b;", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, From 9d8dfc7193478dcdea713718795bf42f1e395223 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Mon, 8 Aug 2022 21:17:09 +0200 Subject: [PATCH 16/37] fix: if cases also suggest based on potential getter --- lib/rules/logical-assignment-operators.js | 14 +- .../lib/rules/logical-assignment-operators.js | 121 ++++++++---------- 2 files changed, 58 insertions(+), 77 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index e1ed15f649c..ed25667edc5 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -195,12 +195,12 @@ module.exports = { /** * Do not fix if the access could be a getter - * @param {ASTNode} assignment Assignment expression + * @param {ASTNode} node Assignment expression * @returns {boolean} True iff the fix is safe */ - function canBeFixed(assignment) { - return assignment.left.type === "Identifier" && - (isStrict || !isInsideWithBlock(assignment)); + function canBeFixed(node) { + return node.type === "Identifier" && + (isStrict || !isInsideWithBlock(node)); } @@ -282,7 +282,7 @@ module.exports = { } }; - context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment))); + context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment.left))); } }; } @@ -327,7 +327,7 @@ module.exports = { } }; - context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment))); + context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment.left))); }, // foo || (foo = bar) @@ -432,7 +432,7 @@ module.exports = { } } }; - const shouldBeFixed = ifNode.test.type !== "LogicalExpression"; + const shouldBeFixed = canBeFixed(existence.reference); context.report(createConditionalFixer(descriptor, suggestion, shouldBeFixed)); } diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index faeb513454d..1b87aac59e8 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -722,40 +722,24 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a === null || a === undefined) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a === undefined || a === null) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a === null || a === void 0) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a === void 0 || a === null) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement" }] }, { code: "if (a) { a = b; }", output: "a &&= b;", @@ -798,58 +782,34 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (undefined === a || a === null) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (a === undefined || null === a) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (undefined === a || null === a) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (null === a || a === undefined) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (a === null || undefined === a) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (null === a || undefined === a) a = b", - output: null, + output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ - messageId: "if", - type: "IfStatement", - suggestions: [{ messageId: "convertIf", output: "a ??= b" }] - }] + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, // > Parenthesis @@ -994,20 +954,41 @@ ruleTester.run("logical-assignment-operators", rule, { // > Members { - code: "if (a.b) a.b = b", - output: "a.b &&= b", + code: "if (a.b) a.b = c", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "a.b &&= c" + }] + }] }, { code: "if (a[b].c) a[b].c = d", - output: "a[b].c &&= d", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "a[b].c &&= d" + }] + }] }, { - code: "with (object) if (a.b) a.b = b", - output: "with (object) a.b &&= b", + code: "with (object) if (a) a = b", + output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "with (object) a &&= b" + }] + }] }, // > Else if @@ -1073,8 +1054,8 @@ ruleTester.run("logical-assignment-operators", rule, { // > Patterns { - code: "if (a.b) a.b = a.b.filter(predicate)", - output: "a.b &&= a.b.filter(predicate)", + code: "if (array) array = array.filter(predicate)", + output: "array &&= array.filter(predicate)", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement" }] }, From 9191a66e162d690aaac1e28b4a50a150520d0763 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Thu, 11 Aug 2022 14:26:43 +0200 Subject: [PATCH 17/37] fix: also fix if only a single property is accessed for the if pattern --- lib/rules/logical-assignment-operators.js | 17 +++++- .../lib/rules/logical-assignment-operators.js | 60 +++++++++++++++++-- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index ed25667edc5..1c3e175639d 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -13,6 +13,7 @@ const astUtils = require("./utils/ast-utils.js"); // Helpers //------------------------------------------------------------------------------ +const baseTypes = new Set(["Identifier", "Super", "ThisExpression"]); /** * Returns true iff either "undefined" or a void expression (eg. "void 0") @@ -203,6 +204,19 @@ module.exports = { (isStrict || !isInsideWithBlock(node)); } + /** + * Check whether only a single property is accessed + * @param {ASTNode} node reference + * @returns {boolean} True iff a single property is accessed + */ + function accessesSingleProperty(node) { + if (!isStrict && isInsideWithBlock(node)) { + return node.type === "Identifier"; + } + + return node.type === "MemberExpression" && + baseTypes.has(node.object.type); + } /** * Adds a fixer or suggestion whether on the fix is safe. @@ -432,7 +446,8 @@ module.exports = { } } }; - const shouldBeFixed = canBeFixed(existence.reference); + const shouldBeFixed = canBeFixed(existence.reference) || + (ifNode.test.type !== "LogicalExpression" && accessesSingleProperty(existence.reference)); context.report(createConditionalFixer(descriptor, suggestion, shouldBeFixed)); } diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 1b87aac59e8..27274ff7119 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -952,9 +952,61 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement" }] }, - // > Members + // > Members > Single Property Access { code: "if (a.b) a.b = c", + output: "a.b &&= c", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + }, { + code: "if (a[b]) a[b] = c", + output: "a[b] &&= c", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + }, { + code: "if (this.prop) this.prop = value", + output: "this.prop &&= value", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + }, { + code: "(class extends SuperClass { method() { if (super.prop) super.prop = value } })", + output: "(class extends SuperClass { method() { super.prop &&= value } })", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + }, { + code: "with (object) if (a) a = b", + output: "with (object) a &&= b", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + }, + + // > Members > Possible Multiple Property Accesses + { + code: "if (a.b === undefined || a.b === null) a.b = c", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "a.b ??= c" + }] + }] + }, { + code: "if (a.b.c) a.b.c = d", + output: null, + options: ["always", { enforceForIfStatements: true }], + errors: [{ + messageId: "if", + type: "IfStatement", + suggestions: [{ + messageId: "convertIf", + output: "a.b.c &&= d" + }] + }] + }, { + code: "if (a.b.c.d) a.b.c.d = e", output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ @@ -962,7 +1014,7 @@ ruleTester.run("logical-assignment-operators", rule, { type: "IfStatement", suggestions: [{ messageId: "convertIf", - output: "a.b &&= c" + output: "a.b.c.d &&= e" }] }] }, { @@ -978,7 +1030,7 @@ ruleTester.run("logical-assignment-operators", rule, { }] }] }, { - code: "with (object) if (a) a = b", + code: "with (object) if (a.b) a.b = c", output: null, options: ["always", { enforceForIfStatements: true }], errors: [{ @@ -986,7 +1038,7 @@ ruleTester.run("logical-assignment-operators", rule, { type: "IfStatement", suggestions: [{ messageId: "convertIf", - output: "with (object) a &&= b" + output: "with (object) a.b &&= c" }] }] }, From 0214ef866299c579796679feef8b50910df7edad Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 31 Aug 2022 14:20:29 +0200 Subject: [PATCH 18/37] fix: do not fix logical patterns with a deeper access --- lib/rules/logical-assignment-operators.js | 22 +++++++++++----- .../lib/rules/logical-assignment-operators.js | 26 ++++++++++++++++--- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 1c3e175639d..040a10ef8c2 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -181,6 +181,7 @@ module.exports = { assignment: "Assignment (=) can be replaced with operator assignment ({{operator}}).", useLogicalOperator: "Convert this assignment to use the operator {{ operator }}.", logical: "Logical expression can be replaced with an assignment ({{ operator }}).", + convertLogical: "Replace this logical expression with an assignment with the operator {{ operator }}.", if: "If can be replaced with operator assignment.", convertIf: "Replace this if with an logical assignment.", unexpected: "Unexpected logical operator assignment ({{operator}}) shorthand.", @@ -195,11 +196,11 @@ module.exports = { const isStrict = context.getScope().isStrict; /** - * Do not fix if the access could be a getter + * Returns false if the access could be a getter * @param {ASTNode} node Assignment expression * @returns {boolean} True iff the fix is safe */ - function canBeFixed(node) { + function cannotBeGetter(node) { return node.type === "Identifier" && (isStrict || !isInsideWithBlock(node)); } @@ -296,7 +297,7 @@ module.exports = { } }; - context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment.left))); + context.report(createConditionalFixer(descriptor, suggestion, cannotBeGetter(assignment.left))); } }; } @@ -341,15 +342,19 @@ module.exports = { } }; - context.report(createConditionalFixer(descriptor, suggestion, canBeFixed(assignment.left))); + context.report(createConditionalFixer(descriptor, suggestion, cannotBeGetter(assignment.left))); }, // foo || (foo = bar) 'LogicalExpression[right.type="AssignmentExpression"][right.operator="="]'(logical) { if (astUtils.isSameReference(logical.left, logical.right.left)) { - context.report({ + const descriptor = { messageId: "logical", node: logical, + data: { operator: logical.operator } + }; + const suggestion = { + messageId: "convertLogical", data: { operator: logical.operator }, *fix(ruleFixer) { if (sourceCode.getCommentsInside(logical).length > 0) { @@ -377,7 +382,10 @@ module.exports = { yield ruleFixer.insertTextBefore(operatorToken, logical.operator); // -> foo ||= bar } - }); + }; + const fix = cannotBeGetter(logical.left) || accessesSingleProperty(logical.left); + + context.report(createConditionalFixer(descriptor, suggestion, fix)); } }, @@ -446,7 +454,7 @@ module.exports = { } } }; - const shouldBeFixed = canBeFixed(existence.reference) || + const shouldBeFixed = cannotBeGetter(existence.reference) || (ifNode.test.type !== "LogicalExpression" && accessesSingleProperty(existence.reference)); context.report(createConditionalFixer(descriptor, suggestion, shouldBeFixed)); diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 27274ff7119..6e24e336f3a 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -637,7 +637,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, - // > Possible Getter + // > Fix Condition { code: "a.b || (a.b = c)", output: "a.b ||= c", @@ -646,10 +646,30 @@ ruleTester.run("logical-assignment-operators", rule, { code: "foo.bar || (foo.bar = baz)", output: "foo.bar ||= baz", errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a.b.c || (a.b.c = d)", + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "convertLogical", + output: "a.b.c ||= d" + }] + }] }, { code: "with (object) a.b || (a.b = c)", - output: "with (object) a.b ||= c", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||" }, + suggestions: [{ + messageId: "convertLogical", + output: "with (object) a.b ||= c" + }] + }] }, // > Context From adc16891aececa019cf6b56158879c3a8cd8b1ad Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:08:46 +0200 Subject: [PATCH 19/37] fix: check whether Boolean references a global --- lib/rules/logical-assignment-operators.js | 15 ++++++++++++++- tests/lib/rules/logical-assignment-operators.js | 7 +++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 040a10ef8c2..f63652d9291 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -96,6 +96,19 @@ function isExplicitNullishComparison(expression, scope) { (isUndefined(expression.left[leftNullish], scope) && astUtils.isNullLiteral(expression.right[rightNullish]))); } +/** + * Returns true for Boolean(arg) calls + * @param {ASTNode} expression Test condition + * @param {import('eslint-scope').Scope} scope Scope of the expression + * @returns {boolean} Whether the expression is a boolean cast + */ +function isBooleanCast(expression, scope) { + return expression.type === "CallExpression" && + expression.callee.name === "Boolean" && + expression.arguments.length === 1 && + astUtils.isReferenceToGlobalVariable(scope, expression.callee); +} + /** * Returns true for: * truthiness checks: value, Boolean(value), !!value @@ -114,7 +127,7 @@ function getExistence(expression, scope) { return { reference: base, operator: isNegated ? "||" : "&&" }; case base.type === "UnaryExpression" && base.operator === "!" && isReference(base.argument): return { reference: base.argument, operator: "&&" }; - case base.type === "CallExpression" && base.callee.name === "Boolean" && base.arguments.length === 1 && isReference(base.arguments[0]): + case isBooleanCast(base, scope) && isReference(base.arguments[0]): return { reference: base.arguments[0], operator: isNegated ? "||" : "&&" }; case isImplicitNullishComparison(expression, scope): return { reference: isReference(expression.left) ? expression.left : expression.right, operator: "??" }; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 6e24e336f3a..945fbf486d2 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -314,6 +314,13 @@ ruleTester.run("logical-assignment-operators", rule, { }, { code: "if (Boolean(a)) b = c", options: ["always", { enforceForIfStatements: true }] + }, { + code: [ + "function fn(Boolean) {", + " if (Boolean(a)) a = b", + "}" + ].join("\n"), + options: ["always", { enforceForIfStatements: true }] }, // Never From 76ac24c0343611713ad3031ad797364c852e8e7c Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:22:22 +0200 Subject: [PATCH 20/37] fix: allow test conditions to access the same static property (a.b <=> a['b']) --- lib/rules/logical-assignment-operators.js | 4 +- .../lib/rules/logical-assignment-operators.js | 58 +++++++++++++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index f63652d9291..bb6a4d8829c 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -91,7 +91,7 @@ function isExplicitNullishComparison(expression, scope) { const rightReference = isReference(expression.right.left) ? "left" : "right"; const rightNullish = rightReference === "left" ? "right" : "left"; - return astUtils.isSameReference(expression.left[leftReference], expression.right[rightReference], true) && + return astUtils.isSameReference(expression.left[leftReference], expression.right[rightReference]) && ((astUtils.isNullLiteral(expression.left[leftNullish]) && isUndefined(expression.right[rightNullish], scope)) || (isUndefined(expression.left[leftNullish], scope) && astUtils.isNullLiteral(expression.right[rightNullish]))); } @@ -319,7 +319,7 @@ module.exports = { // foo = foo || bar "AssignmentExpression[operator='='][right.type='LogicalExpression']"(assignment) { - if (!astUtils.isSameReference(assignment.left, assignment.right.left, true)) { + if (!astUtils.isSameReference(assignment.left, assignment.right.left)) { return; } diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 945fbf486d2..e0bf395cf94 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -466,6 +466,42 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a[b] ??= c" }] }] + }, { + code: "a['b'] = a['b'] ?? c", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "a['b'] ??= c" + }] + }] + }, { + code: "a.b = a['b'] ?? c", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "a.b ??= c" + }] + }] + }, { + code: "a['b'] = a.b ?? c", + output: null, + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??" }, + suggestions: [{ + messageId: "useLogicalOperator", + output: "a['b'] ??= c" + }] + }] }, { code: "this.prop = this.prop ?? {}", output: null, @@ -649,6 +685,10 @@ ruleTester.run("logical-assignment-operators", rule, { code: "a.b || (a.b = c)", output: "a.b ||= c", errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + }, { + code: "a['b'] || (a['b'] = c)", + output: "a['b'] ||= c", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] }, { code: "foo.bar || (foo.bar = baz)", output: "foo.bar ||= baz", @@ -990,6 +1030,11 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a[b] &&= c", options: ["always", { enforceForIfStatements: true }], errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + }, { + code: "if (a['b']) a['b'] = c", + output: "a['b'] &&= c", + options: ["always", { enforceForIfStatements: true }], + errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] }, { code: "if (this.prop) this.prop = value", output: "this.prop &&= value", @@ -1189,6 +1234,19 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a[b] = a[b] || c" }] }] + }, { + code: "a['b'] ||= c", + output: null, + options: ["never"], + errors: [{ + messageId: "unexpected", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "separate", + output: "a['b'] = a['b'] || c" + }] + }] }, { code: "this.prop ||= 0", output: null, From 784fefef0e82c3c36ff5e4baa1efb3e9cc12aca9 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:49:12 +0200 Subject: [PATCH 21/37] fix: use the whole assignment operator with equals in the message and add operator for 'if' messages --- lib/rules/logical-assignment-operators.js | 17 +- .../lib/rules/logical-assignment-operators.js | 251 +++++++++--------- 2 files changed, 137 insertions(+), 131 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index bb6a4d8829c..2ffaad73391 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -195,8 +195,8 @@ module.exports = { useLogicalOperator: "Convert this assignment to use the operator {{ operator }}.", logical: "Logical expression can be replaced with an assignment ({{ operator }}).", convertLogical: "Replace this logical expression with an assignment with the operator {{ operator }}.", - if: "If can be replaced with operator assignment.", - convertIf: "Replace this if with an logical assignment.", + if: "'if' statement can be replaced with a logical operator assignment with operator {{ operator }}.", + convertIf: "Replace this 'if' statement with a logical assignment with operator {{ operator }}.", unexpected: "Unexpected logical operator assignment ({{operator}}) shorthand.", separate: "Separate the logical assignment into an assignment with a logical operator." } @@ -279,7 +279,6 @@ module.exports = { }; const suggestion = { messageId: "separate", - data: { operator: assignment.operator }, *fix(ruleFixer) { if (sourceCode.getCommentsInside(assignment).length > 0) { return; @@ -326,11 +325,11 @@ module.exports = { const descriptor = { messageId: "assignment", node: assignment, - data: { operator: assignment.right.operator } + data: { operator: `${assignment.right.operator}=` } }; const suggestion = { messageId: "useLogicalOperator", - data: { operator: assignment.operator }, + data: { operator: `${assignment.right.operator}=` }, *fix(ruleFixer) { if (sourceCode.getCommentsInside(assignment).length > 0) { return; @@ -364,11 +363,11 @@ module.exports = { const descriptor = { messageId: "logical", node: logical, - data: { operator: logical.operator } + data: { operator: `${logical.operator}=` } }; const suggestion = { messageId: "convertLogical", - data: { operator: logical.operator }, + data: { operator: `${logical.operator}=` }, *fix(ruleFixer) { if (sourceCode.getCommentsInside(logical).length > 0) { return; @@ -427,10 +426,12 @@ module.exports = { ) { const descriptor = { messageId: "if", - node: ifNode + node: ifNode, + data: { operator: `${existence.operator}=` } }; const suggestion = { messageId: "convertIf", + data: { operator: `${existence.operator}=` }, *fix(ruleFixer) { if (sourceCode.getCommentsInside(ifNode).length > 0) { return; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index e0bf395cf94..b4a6fb90755 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -353,80 +353,80 @@ ruleTester.run("logical-assignment-operators", rule, { { code: "a = a || b", output: "a ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a && b", output: "a &&= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "&&" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "&&=" }, suggestions: [] }] }, { code: "a = a ?? b", output: "a ??= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "??" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "??=" }, suggestions: [] }] }, { code: "foo = foo || bar", output: "foo ||= bar", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, // > Right { code: "a = a || fn()", output: "a ||= fn()", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || b && c", output: "a ||= b && c", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || (b || c)", output: "a ||= b || c", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || (b ? c : d)", output: "a ||= b ? c : d", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, // > Comments { code: "/* before */ a = a || b", output: "/* before */ a ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || b // after", output: "a ||= b // after", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a /* between */ = a || b", output: null, - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = /** @type */ a || b", output: null, - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || /* between */ b", output: null, - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, // > Parenthesis { code: "(a) = a || b", output: "(a) ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = (a) || b", output: "a ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || (b)", output: "a ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "(a = a || b)", output: "(a ||= b)", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, // > Suggestions @@ -436,7 +436,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "a.b ??= c" @@ -448,7 +448,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "a.b.c ??= d" @@ -460,7 +460,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "a[b] ??= c" @@ -472,7 +472,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "a['b'] ??= c" @@ -484,7 +484,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "a.b ??= c" @@ -496,7 +496,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "a['b'] ??= c" @@ -508,7 +508,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "??" }, + data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", output: "this.prop ??= {}" @@ -523,7 +523,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "||" }, + data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", output: "with (object) a ||= b" @@ -535,7 +535,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "||" }, + data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", output: "with (object) { a ||= b }" @@ -547,7 +547,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "||" }, + data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", output: "with (object) { if (condition) a ||= b }" @@ -556,19 +556,19 @@ ruleTester.run("logical-assignment-operators", rule, { }, { code: "with (a = a || b) {}", output: "with (a ||= b) {}", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "with (object) {} a = a || b", output: "with (object) {} a ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || b; with (object) {}", output: "a ||= b; with (object) {}", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "if (condition) a = a || b", output: "if (condition) a ||= b", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: [ "with (object) {", @@ -580,7 +580,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", - data: { operator: "||" }, + data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", output: [ @@ -597,109 +597,109 @@ ruleTester.run("logical-assignment-operators", rule, { { code: "fn(a = a || b)", output: "fn(a ||= b)", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "fn((a = a || b))", output: "fn((a ||= b))", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "(a = a || b) ? c : d", output: "(a ||= b) ? c : d", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = b = b || c", output: "a = b ||= c", - errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, // Logical { code: "a || (a = b)", output: "a ||= b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a && (a = b)", output: "a &&= b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "&&" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "&&=" } }] }, { code: "a ?? (a = b)", output: "a ??= b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "??" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "??=" } }] }, { code: "foo ?? (foo = bar)", output: "foo ??= bar", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "??" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "??=" } }] }, // > Right { code: "a || (a = 0)", output: "a ||= 0", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || (a = fn())", output: "a ||= fn()", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || (a = (b || c))", output: "a ||= (b || c)", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, // > Parenthesis { code: "(a) || (a = b)", output: "a ||= b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || ((a) = b)", output: "(a) ||= b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || (a = (b))", output: "a ||= (b)", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, // > Comments { code: "/* before */ a || (a = b)", output: "/* before */ a ||= b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || (a = b) // after", output: "a ||= b // after", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a /* between */ || (a = b)", output: null, - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || /* between */ (a = b)", output: null, - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, // > Fix Condition { code: "a.b || (a.b = c)", output: "a.b ||= c", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a['b'] || (a['b'] = c)", output: "a['b'] ||= c", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "foo.bar || (foo.bar = baz)", output: "foo.bar ||= baz", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a.b.c || (a.b.c = d)", output: null, errors: [{ messageId: "logical", type: "LogicalExpression", - data: { operator: "||" }, + data: { operator: "||=" }, suggestions: [{ messageId: "convertLogical", output: "a.b.c ||= d" @@ -711,7 +711,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "logical", type: "LogicalExpression", - data: { operator: "||" }, + data: { operator: "||=" }, suggestions: [{ messageId: "convertLogical", output: "with (object) a.b ||= c" @@ -723,32 +723,32 @@ ruleTester.run("logical-assignment-operators", rule, { { code: "a = a.b || (a.b = {})", output: "a = a.b ||= {}", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" }, suggestions: [] }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a || (a = 0) || b", output: "(a ||= 0) || b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "(a || (a = 0)) || b", output: "(a ||= 0) || b", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || (b || (b = 0))", output: "a || (b ||= 0)", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a = b || (b = c)", output: "a = b ||= c", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a || (a = 0) ? b : c", output: "(a ||= 0) ? b : c", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "fn(a || (a = 0))", output: "fn(a ||= 0)", - errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||" } }] + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, // If @@ -756,62 +756,62 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (a) a = b", output: "a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (Boolean(a)) a = b", output: "a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (!!a) a = b", output: "a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (!a) a = b", output: "a ||= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "||=" } }] }, { code: "if (!Boolean(a)) a = b", output: "a ||= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "||=" } }] }, { code: "if (a == undefined) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: "if (a == null) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: "if (a === null || a === undefined) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: "if (a === undefined || a === null) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: "if (a === null || a === void 0) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: "if (a === void 0 || a === null) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: "if (a) { a = b; }", output: "a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: [ "{ const undefined = 0; }", @@ -822,7 +822,7 @@ ruleTester.run("logical-assignment-operators", rule, { "a ??= b" ].join("\n"), options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, { code: [ "if (a == undefined) a = b", @@ -833,7 +833,7 @@ ruleTester.run("logical-assignment-operators", rule, { "{ const undefined = 0; }" ].join("\n"), options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" } }] }, // > Yoda @@ -841,42 +841,42 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (null == a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (undefined == a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (undefined === a || a === null) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (a === undefined || null === a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (undefined === a || null === a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (null === a || a === undefined) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (a === null || undefined === a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, { code: "if (null === a || undefined === a) a = b", output: "a ??= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "??=" }, suggestions: [] }] }, // > Parenthesis @@ -884,22 +884,22 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if ((a)) a = b", output: "a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) (a) = b", output: "(a) &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) a = (b)", output: "a &&= (b)", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) (a = b)", output: "(a &&= b)", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Previous statement @@ -907,42 +907,42 @@ ruleTester.run("logical-assignment-operators", rule, { code: ";if (a) (a) = b", output: ";(a) &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "{ if (a) (a) = b }", output: "{ (a) &&= b }", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "fn();if (a) (a) = b", output: "fn();(a) &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "fn()\nif (a) a = b", output: "fn()\na &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "id\nif (a) (a) = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "object.prop\nif (a) (a) = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "object[computed]\nif (a) (a) = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "fn()\nif (a) (a) = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Adding semicolon @@ -950,32 +950,32 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (a) a = b; fn();", output: "a &&= b; fn();", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) { a = b }", output: "a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) { a = b; }\nfn();", output: "a &&= b;\nfn();", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) { a = b }\nfn();", output: "a &&= b;\nfn();", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) { a = b } fn();", output: "a &&= b; fn();", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) { a = b\n} fn();", output: "a &&= b; fn();", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Spacing @@ -1001,22 +1001,22 @@ ruleTester.run("logical-assignment-operators", rule, { code: "/* before */ if (a) a = b", output: "/* before */ a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) a = b /* after */", output: "a &&= b /* after */", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) /* between */ a = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) a = /* between */ b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Members > Single Property Access @@ -1024,17 +1024,17 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (a.b) a.b = c", output: "a.b &&= c", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" }, suggestions: [] }] }, { code: "if (a[b]) a[b] = c", output: "a[b] &&= c", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" }, suggestions: [] }] }, { code: "if (a['b']) a['b'] = c", output: "a['b'] &&= c", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" }, suggestions: [] }] }, { code: "if (this.prop) this.prop = value", output: "this.prop &&= value", @@ -1044,12 +1044,12 @@ ruleTester.run("logical-assignment-operators", rule, { code: "(class extends SuperClass { method() { if (super.prop) super.prop = value } })", output: "(class extends SuperClass { method() { super.prop &&= value } })", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" }, suggestions: [] }] }, { code: "with (object) if (a) a = b", output: "with (object) a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement", suggestions: [] }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" }, suggestions: [] }] }, // > Members > Possible Multiple Property Accesses @@ -1060,6 +1060,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement", + data: { operator: "??=" }, suggestions: [{ messageId: "convertIf", output: "a.b ??= c" @@ -1072,6 +1073,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement", + data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", output: "a.b.c &&= d" @@ -1084,6 +1086,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement", + data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", output: "a.b.c.d &&= e" @@ -1096,6 +1099,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement", + data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", output: "a[b].c &&= d" @@ -1108,6 +1112,7 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "if", type: "IfStatement", + data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", output: "with (object) a.b &&= c" @@ -1120,37 +1125,37 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (unrelated) {} else if (a) a = b;", output: "if (unrelated) {} else a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) {} else if (b) {} else if (a) a = b;", output: "if (a) {} else if (b) {} else a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) {} else\nif (a) a = b;", output: "if (unrelated) {} else\na &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) {\n}\nelse if (a) {\na = b;\n}", output: "if (unrelated) {\n}\nelse a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) statement; else if (a) a = b;", output: "if (unrelated) statement; else a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) id\nelse if (a) (a) = b", output: null, options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) {} else if (a) a = b; else if (c) c = d", output: "if (unrelated) {} else if (a) a = b; else c &&= d", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Else if > Comments @@ -1158,22 +1163,22 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (unrelated) { /* body */ } else if (a) a = b;", output: "if (unrelated) { /* body */ } else a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) {} /* before else */ else if (a) a = b;", output: "if (unrelated) {} /* before else */ else a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) {} else // Line\nif (a) a = b;", output: "if (unrelated) {} else // Line\na &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (unrelated) {} else /* Block */ if (a) a = b;", output: "if (unrelated) {} else /* Block */ a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Patterns @@ -1181,7 +1186,7 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (array) array = array.filter(predicate)", output: "array &&= array.filter(predicate)", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // Never From fa5c0ccb55d804fec35c06a356859fe69c708f5a Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:51:41 +0200 Subject: [PATCH 22/37] docs: remove edit_link --- docs/src/rules/logical-assignment-operators.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 989c349863d..55071284638 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -1,7 +1,6 @@ --- title: logical-assignment-operators layout: doc -edit_link: https://github.com/eslint/eslint/edit/main/docs/src/rules/logical-assignment-operators.md rule_type: suggestion --- From e159e7a2f3a369d60f65981230596eaccd280e55 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:52:38 +0200 Subject: [PATCH 23/37] docs: remove description from docs as it is autogenerated from rule.meta.docs.description --- docs/src/rules/logical-assignment-operators.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 55071284638..28c3ab80d4d 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -6,7 +6,6 @@ rule_type: suggestion -Require or disallow assignment logical operator shorthand ## Rule Details From ef1a3f806fceedd795f580f0be664fcea9d32bda Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:54:27 +0200 Subject: [PATCH 24/37] docs: move introductory text before rule details --- docs/src/rules/logical-assignment-operators.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 28c3ab80d4d..a9d11924f20 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -4,17 +4,15 @@ layout: doc rule_type: suggestion --- - - - -## Rule Details - -This rule requires or disallows logical assignment operator shorthand. ES2021 introduces the assignment operator shorthand for the logical operators `||`, `&&` and `??`. Before this was only allowed for mathematical operations such as `+` or `*` (see the rule [operator-assignment](./operator-assignment)). The shorthand can be used if the assignment target and the left expression of a logical expression are the same. For example `a = a || b` can be shortened to `a ||= b`. +## Rule Details + +This rule requires or disallows logical assignment operator shorthand. + ### Options This rule has a string and an object option. From c211235debc117b718ee80b06742a9059c5b8f85 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:55:51 +0200 Subject: [PATCH 25/37] docs: add missing 'logical' to rule description --- lib/rules/logical-assignment-operators.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 2ffaad73391..7884055a581 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -159,7 +159,7 @@ module.exports = { type: "suggestion", docs: { - description: "Require or disallow assignment logical operator shorthand", + description: "Require or disallow logical assignment logical operator shorthand", recommended: false, url: "https://eslint.org/docs/rules/logical-assignment-operators" }, From aad522c52ab9ad2b6a13ca7bdd1bb0385bac08a0 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:57:43 +0200 Subject: [PATCH 26/37] docs: fix formatting for options --- docs/src/rules/logical-assignment-operators.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index a9d11924f20..60eb2f92766 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -23,9 +23,8 @@ String option: Object option (only available if string option is set to `"always"`): -* `"enforceForIfStatements": false`(default) Do *not* check for equivalent if statements - -* `"enforceForIfStatements": true` Check for equivalent if statements +* `"enforceForIfStatements": false`(default) Do *not* check for equivalent `if` statements +* `"enforceForIfStatements": true` Check for equivalent `if` statements #### always From 863df8e35e85aa9990f3c148fd21865dcdc1a69d Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 10:58:37 +0200 Subject: [PATCH 27/37] docs: include all logical operators for option 'never' --- docs/src/rules/logical-assignment-operators.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 60eb2f92766..f4db3050d12 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -67,6 +67,7 @@ Examples of **incorrect** code for this rule with the `"never"` option: a ||= b a &&= b +a ??= b ``` ::: @@ -79,6 +80,7 @@ Examples of **correct** code for this rule with the `"never"` option: /*eslint logical-assignment-operators: ["error", "never"]*/ a = a || b +a = a && b a = a ?? b ``` From 88c5360dda15fb335af307c41216d388f23e8d7a Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 11:21:54 +0200 Subject: [PATCH 28/37] docs: add examples for option 'always' --- docs/src/rules/logical-assignment-operators.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index f4db3050d12..a295625542c 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -38,6 +38,9 @@ Examples of **incorrect** code for this rule with the default `"always"` option: a = a || b a = a && b a = a ?? b +a || (a = b) +a && (a = b) +a ?? (a = b) ``` ::: @@ -50,8 +53,12 @@ Examples of **correct** code for this rule with the default `"always"` option: /*eslint logical-assignment-operators: ["error", "always"]*/ a = b -a ||= b a += b +a ||= b +a = b || c +a || (b = c) + +if (a) a = b ``` ::: From a99eb629909c5faf1b0deda093e3ebcb637f4eb2 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 11:24:55 +0200 Subject: [PATCH 29/37] docs: add examples of for the 'enforceForIfStatements' option and swap correct with incorrect sections --- .../src/rules/logical-assignment-operators.md | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index a295625542c..dae1f4c0921 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -96,27 +96,32 @@ a = a ?? b #### enforceForIfStatements This option checks for additional patterns with if statements which could be expressed with the logical assignment operator. -::: correct + +::: incorrect + +Examples of **incorrect** code for this rule with the `["always", { enforceIfStatements: true }]` option: ```js /*eslint logical-assignment-operators: ["error", "always", { enforceForIfStatements: true }]*/ -if (a) b = c -if (a === 0) a = b +if (a) a = b // <=> a &&= b +if (!a) a = b // <=> a ||= b + +if (a == null) a = b // <=> a ??= b +if (a === null || a === undefined) a = b // <=> a ??= b ``` ::: -::: incorrect +Examples of **correct** code for this rule with the `["always", { enforceIfStatements: true }]` option: + +::: correct ```js /*eslint logical-assignment-operators: ["error", "always", { enforceForIfStatements: true }]*/ -if (a) a = b // <=> a &&= b -if (!a) a = b // <=> a ||= b - -if (a == null) a = b // <=> a ??= b -if (a === null || a === undefined) a = b // <=> a ??= b +if (a) b = c +if (a === 0) a = b ``` ::: From aa69bbf8a29415567885ec7c89289371a26e93c8 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 11:27:37 +0200 Subject: [PATCH 30/37] fix: disallow optional chaining for the logical pattern --- lib/rules/logical-assignment-operators.js | 2 +- tests/lib/rules/logical-assignment-operators.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 7884055a581..16aa6d63656 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -359,7 +359,7 @@ module.exports = { // foo || (foo = bar) 'LogicalExpression[right.type="AssignmentExpression"][right.operator="="]'(logical) { - if (astUtils.isSameReference(logical.left, logical.right.left)) { + if (isReference(logical.left) && astUtils.isSameReference(logical.left, logical.right.left)) { const descriptor = { messageId: "logical", node: logical, diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index b4a6fb90755..90933cef9ff 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -72,6 +72,7 @@ ruleTester.run("logical-assignment-operators", rule, { "a || (a ||= b)", "fn() || (a = b)", "a.b || (a = b)", + "a?.b || (a.b = b)", // If "if (a) a = b", From 7f2c77f407d0429100e7951d984b2a46ac8b02fb Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 11:37:15 +0200 Subject: [PATCH 31/37] fix: fixer does not delete parenthesis around the right operand in the assignment pattern --- lib/rules/logical-assignment-operators.js | 11 +++-------- tests/lib/rules/logical-assignment-operators.js | 14 +++++++++++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 16aa6d63656..759a946f9c7 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -342,15 +342,10 @@ module.exports = { yield ruleFixer.insertTextBefore(assignmentOperatorToken, assignment.right.operator); // -> foo ||= bar - yield ruleFixer.removeRange([assignment.right.range[0], assignment.right.right.range[0]]); + const logicalOperatorToken = getOperatorToken(assignment.right); + const firstRightOperandToken = sourceCode.getTokenAfter(logicalOperatorToken); - if (astUtils.isParenthesised(sourceCode, assignment.right.right)) { - - // Opening parenthesis already removed - const closingParenthesis = sourceCode.getTokenAfter(assignment.right.right); - - yield ruleFixer.remove(closingParenthesis); - } + yield ruleFixer.removeRange([assignment.right.range[0], firstRightOperandToken.range[0]]); } }; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 90933cef9ff..0ecb7320f6b 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -380,11 +380,11 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || (b || c)", - output: "a ||= b || c", + output: "a ||= (b || c)", errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || (b ? c : d)", - output: "a ||= b ? c : d", + output: "a ||= (b ? c : d)", errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, @@ -422,12 +422,20 @@ ruleTester.run("logical-assignment-operators", rule, { errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "a = a || (b)", - output: "a ||= b", + output: "a ||= (b)", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] + }, { + code: "a = a || ((b))", + output: "a ||= ((b))", errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, { code: "(a = a || b)", output: "(a ||= b)", errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] + }, { + code: "a = a || (f(), b)", + output: "a ||= (f(), b)", + errors: [{ messageId: "assignment", type: "AssignmentExpression", data: { operator: "||=" }, suggestions: [] }] }, // > Suggestions From 6e00a16e275842649ad744428603ef2267f79c89 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 11:53:50 +0200 Subject: [PATCH 32/37] fix: remove multiple parenthesis around the right operand in the logical pattern --- lib/rules/logical-assignment-operators.js | 19 +++++++++---------- .../lib/rules/logical-assignment-operators.js | 12 ++++++++++++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 759a946f9c7..bed9c170a90 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -354,6 +354,8 @@ module.exports = { // foo || (foo = bar) 'LogicalExpression[right.type="AssignmentExpression"][right.operator="="]'(logical) { + + // Right side has to be parenthesized, otherwise would be parsed as (foo || foo) = bar which is illegal if (isReference(logical.left) && astUtils.isSameReference(logical.left, logical.right.left)) { const descriptor = { messageId: "logical", @@ -368,22 +370,19 @@ module.exports = { return; } - const requiresParenthesis = logical.parent.type !== "ExpressionStatement" && - (astUtils.getPrecedence({ type: "AssignmentExpression" }) < astUtils.getPrecedence(logical.parent)); + const requiresOuterParenthesis = logical.parent.type !== "ExpressionStatement" && + (astUtils.getPrecedence({ type: "AssignmentExpression" }) < astUtils.getPrecedence(logical.parent)); - if (!astUtils.isParenthesised(sourceCode, logical) && requiresParenthesis) { + if (!astUtils.isParenthesised(sourceCode, logical) && requiresOuterParenthesis) { yield ruleFixer.insertTextBefore(logical, "("); yield ruleFixer.insertTextAfter(logical, ")"); } - // Right side has to be parenthesized, otherwise would be parsed as (foo || foo) = bar which is illegal - const leftToStartRight = [logical.range[0], logical.right.range[0]]; - - yield ruleFixer.removeRange(leftToStartRight); // -> foo = bar) - - const endParenthesis = sourceCode.getTokenAfter(logical.right); + // Also removes all opening parenthesis + yield ruleFixer.removeRange([logical.range[0], logical.right.range[0]]); // -> foo = bar) - yield ruleFixer.remove(endParenthesis); // -> foo = bar + // Also removes all ending parenthesis + yield ruleFixer.removeRange([logical.right.range[1], logical.range[1]]); // -> foo = bar const operatorToken = getOperatorToken(logical.right); diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 0ecb7320f6b..c5b9c438bf0 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -668,6 +668,18 @@ ruleTester.run("logical-assignment-operators", rule, { code: "a || (a = (b))", output: "a ||= (b)", errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] + }, { + code: "a || ((a = b))", + output: "a ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] + }, { + code: "a || (((a = b)))", + output: "a ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] + }, { + code: "a || ( ( a = b ) )", + output: "a ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, // > Comments From bf51265c488663e576969e32492a730867cb6fd6 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Tue, 6 Sep 2022 11:59:13 +0200 Subject: [PATCH 33/37] test: add data property for suggestions --- .../lib/rules/logical-assignment-operators.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index c5b9c438bf0..50402b6c64b 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -448,6 +448,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "a.b ??= c" }] }] @@ -460,6 +461,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "a.b.c ??= d" }] }] @@ -472,6 +474,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "a[b] ??= c" }] }] @@ -484,6 +487,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "a['b'] ??= c" }] }] @@ -496,6 +500,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "a.b ??= c" }] }] @@ -508,6 +513,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "a['b'] ??= c" }] }] @@ -520,6 +526,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "??=" }, output: "this.prop ??= {}" }] }] @@ -535,6 +542,8 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "||=" }, + output: "with (object) a ||= b" }] }] @@ -547,6 +556,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "||=" }, output: "with (object) { a ||= b }" }] }] @@ -559,6 +569,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "||=" }, output: "with (object) { if (condition) a ||= b }" }] }] @@ -592,6 +603,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "useLogicalOperator", + data: { operator: "||=" }, output: [ "with (object) {", ' "use strict";', @@ -723,6 +735,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "convertLogical", + data: { operator: "||=" }, output: "a.b.c ||= d" }] }] @@ -735,6 +748,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "convertLogical", + data: { operator: "||=" }, output: "with (object) a.b ||= c" }] }] @@ -1084,6 +1098,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "??=" }, suggestions: [{ messageId: "convertIf", + data: { operator: "??=" }, output: "a.b ??= c" }] }] @@ -1097,6 +1112,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", + data: { operator: "&&=" }, output: "a.b.c &&= d" }] }] @@ -1110,6 +1126,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", + data: { operator: "&&=" }, output: "a.b.c.d &&= e" }] }] @@ -1123,6 +1140,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", + data: { operator: "&&=" }, output: "a[b].c &&= d" }] }] @@ -1136,6 +1154,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "&&=" }, suggestions: [{ messageId: "convertIf", + data: { operator: "&&=" }, output: "with (object) a.b &&= c" }] }] @@ -1244,6 +1263,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", + data: { operator: "||=" }, output: "a.b = a.b || c" }] }] @@ -1257,6 +1277,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", + data: { operator: "||=" }, output: "a[b] = a[b] || c" }] }] @@ -1270,6 +1291,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", + data: { operator: "||=" }, output: "a['b'] = a['b'] || c" }] }] @@ -1283,6 +1305,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", + data: { operator: "||=" }, output: "this.prop = this.prop || 0" }] }] @@ -1296,6 +1319,7 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", + data: { operator: "||=" }, output: "with (object) a = a || b" }] }] From 5eb5c6bc016e8fddd73e86bc4c718e65c5966ec4 Mon Sep 17 00:00:00 2001 From: fnx Date: Tue, 6 Sep 2022 14:36:19 +0200 Subject: [PATCH 34/37] docs: add missing comma in description Co-authored-by: Milos Djermanovic --- docs/src/rules/logical-assignment-operators.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index dae1f4c0921..d462d6ef423 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -5,7 +5,7 @@ rule_type: suggestion --- ES2021 introduces the assignment operator shorthand for the logical operators `||`, `&&` and `??`. -Before this was only allowed for mathematical operations such as `+` or `*` (see the rule [operator-assignment](./operator-assignment)). +Before, this was only allowed for mathematical operations such as `+` or `*` (see the rule [operator-assignment](./operator-assignment)). The shorthand can be used if the assignment target and the left expression of a logical expression are the same. For example `a = a || b` can be shortened to `a ||= b`. From 839145fb16d49a3fb3f82220193363a47c0ea325 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 7 Sep 2022 12:09:09 +0200 Subject: [PATCH 35/37] test: clean up unnecessary data, add missing data and pass missing options for test case --- tests/lib/rules/logical-assignment-operators.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 50402b6c64b..c0e81889b74 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -174,13 +174,13 @@ ruleTester.run("logical-assignment-operators", rule, { options: ["always", { enforceForIfStatements: true }] }, { code: "if (a === null || a === void void 0) a = b", - output: "a ??= b" + options: ["always", { enforceForIfStatements: true }] }, { code: "if (a === null || a === void 'string') a = b", - output: "a ??= b" + options: ["always", { enforceForIfStatements: true }] }, { code: "if (a === null || a === void fn()) a = b", - output: "a ??= b" + options: ["always", { enforceForIfStatements: true }] }, // > Test > Yoda @@ -1018,17 +1018,17 @@ ruleTester.run("logical-assignment-operators", rule, { code: "if (a) a = b", output: "a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a)\n a = b", output: "a &&= b", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, { code: "if (a) {\n a = b; \n}", output: "a &&= b;", options: ["always", { enforceForIfStatements: true }], - errors: [{ messageId: "if", type: "IfStatement" }] + errors: [{ messageId: "if", type: "IfStatement", data: { operator: "&&=" } }] }, // > Comments @@ -1263,7 +1263,6 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", - data: { operator: "||=" }, output: "a.b = a.b || c" }] }] @@ -1277,7 +1276,6 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", - data: { operator: "||=" }, output: "a[b] = a[b] || c" }] }] @@ -1291,7 +1289,6 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", - data: { operator: "||=" }, output: "a['b'] = a['b'] || c" }] }] @@ -1305,7 +1302,6 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", - data: { operator: "||=" }, output: "this.prop = this.prop || 0" }] }] @@ -1319,7 +1315,6 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [{ messageId: "separate", - data: { operator: "||=" }, output: "with (object) a = a || b" }] }] From fde5e0fe8b8c8483ff04af15846510bedb5840c7 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 7 Sep 2022 12:30:59 +0200 Subject: [PATCH 36/37] fix: do not allow property accesses in a computed property when checking for a single property access --- lib/rules/logical-assignment-operators.js | 3 +- .../lib/rules/logical-assignment-operators.js | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index bed9c170a90..bd2357acf43 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -229,7 +229,8 @@ module.exports = { } return node.type === "MemberExpression" && - baseTypes.has(node.object.type); + baseTypes.has(node.object.type) && + (!node.computed || (node.property.type !== "MemberExpression" && node.property.type !== "ChainExpression")); } /** diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index c0e81889b74..88d6624e1ac 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -722,6 +722,14 @@ ruleTester.run("logical-assignment-operators", rule, { code: "a['b'] || (a['b'] = c)", output: "a['b'] ||= c", errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] + }, { + code: "a[0] || (a[0] = b)", + output: "a[0] ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] + }, { + code: "a[this] || (a[this] = b)", + output: "a[this] ||= b", + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "foo.bar || (foo.bar = baz)", output: "foo.bar ||= baz", @@ -739,6 +747,32 @@ ruleTester.run("logical-assignment-operators", rule, { output: "a.b.c ||= d" }] }] + }, { + code: "a[b.c] || (a[b.c] = d)", + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "convertLogical", + data: { operator: "||=" }, + output: "a[b.c] ||= d" + }] + }] + }, { + code: "a[b?.c] || (a[b?.c] = d)", + output: null, + errors: [{ + messageId: "logical", + type: "LogicalExpression", + data: { operator: "||=" }, + suggestions: [{ + messageId: "convertLogical", + data: { operator: "||=" }, + output: "a[b?.c] ||= d" + }] + }] }, { code: "with (object) a.b || (a.b = c)", output: null, From 180155d6ce0799af0d0c369ce3e6cbf08344aa68 Mon Sep 17 00:00:00 2001 From: Daniel Martens Date: Wed, 7 Sep 2022 12:32:51 +0200 Subject: [PATCH 37/37] test: add test cases for private identifiers --- tests/lib/rules/logical-assignment-operators.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 88d6624e1ac..ba839b5c6a8 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -73,6 +73,13 @@ ruleTester.run("logical-assignment-operators", rule, { "fn() || (a = b)", "a.b || (a = b)", "a?.b || (a.b = b)", + { + code: "class Class { #prop; constructor() { this.#prop || (this.prop = value) } }", + parserOptions: { ecmaVersion: 2022 } + }, { + code: "class Class { #prop; constructor() { this.prop || (this.#prop = value) } }", + parserOptions: { ecmaVersion: 2022 } + }, // If "if (a) a = b", @@ -718,6 +725,11 @@ ruleTester.run("logical-assignment-operators", rule, { code: "a.b || (a.b = c)", output: "a.b ||= c", errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] + }, { + code: "class Class { #prop; constructor() { this.#prop || (this.#prop = value) } }", + output: "class Class { #prop; constructor() { this.#prop ||= value } }", + parserOptions: { ecmaVersion: 2022 }, + errors: [{ messageId: "logical", type: "LogicalExpression", data: { operator: "||=" } }] }, { code: "a['b'] || (a['b'] = c)", output: "a['b'] ||= c",