diff --git a/docs/rules/require-atomic-updates.md b/docs/rules/require-atomic-updates.md index 8dd313815c8..64fae21d619 100644 --- a/docs/rules/require-atomic-updates.md +++ b/docs/rules/require-atomic-updates.md @@ -38,11 +38,22 @@ Promise.all([getPageLength(1), getPageLength(2)]).then(pageLengths => { ## Rule Details -This rule aims to report assignments to variables or properties where all of the following are true: +This rule aims to report assignments to variables or properties in cases where the assignments may be based on outdated values. -* A variable or property is reassigned to a new value which is based on its old value. -* A `yield` or `await` expression interrupts the assignment after the old value is read, and before the new value is set. -* The rule cannot easily verify that the assignment is safe (e.g. if an assigned variable is local and would not be readable from anywhere else while the function is paused). +### Variables + +This rule reports an assignment to a variable when it detects the following execution flow in a generator or async function: + +1. The variable is read. +2. A `yield` or `await` pauses the function. +3. After the function is resumed, a value is assigned to the variable from step 1. + +The assignment in step 3 is reported because it may be incorrectly resolved because the value of the variable from step 1 may have changed between steps 2 and 3. In particular, if the variable can be accessed from other execution contexts (for example, if it is not a local variable and therefore other functions can change it), the value of the variable may have changed elsewhere while the function was paused in step 2. + +Note that the rule does not report the assignment in step 3 in any of the following cases: + +* If the variable is read again between steps 2 and 3. +* If the variable cannot be accessed while the function is paused (for example, if it's a local variable). Examples of **incorrect** code for this rule: @@ -50,20 +61,27 @@ Examples of **incorrect** code for this rule: /* eslint require-atomic-updates: error */ let result; -async function foo() { - result += await somethingElse; - result = result + await somethingElse; +async function foo() { + result += await something; +} - result = result + doSomething(await somethingElse); +async function bar() { + result = result + await something; } -function* bar() { - result += yield; +async function baz() { + result = result + doSomething(await somethingElse); +} - result = result + (yield somethingElse); +async function qux() { + if (!result) { + result = await initialize(); + } +} - result = result + doSomething(yield somethingElse); +function* generator() { + result += yield; } ``` @@ -73,22 +91,89 @@ Examples of **correct** code for this rule: /* eslint require-atomic-updates: error */ let result; -async function foo() { - result = await somethingElse + result; - let tmp = await somethingElse; - result += tmp; +async function foobar() { + result = await something + result; +} + +async function baz() { + const tmp = doSomething(await somethingElse); + result += tmp; +} + +async function qux() { + if (!result) { + const tmp = await initialize(); + if (!result) { + result = tmp; + } + } +} + +async function quux() { + let localVariable = 0; + localVariable += await something; +} + +function* generator() { + result = (yield) + result; +} +``` + +### Properties + +This rule reports an assignment to a property through a variable when it detects the following execution flow in a generator or async function: + +1. The variable or object property is read. +2. A `yield` or `await` pauses the function. +3. After the function is resumed, a value is assigned to a property. - let localVariable = 0; - localVariable += await somethingElse; +This logic is similar to the logic for variables, but stricter because the property in step 3 doesn't have to be the same as the property in step 1. It is assumed that the flow depends on the state of the object as a whole. + +Example of **incorrect** code for this rule: + +```js +/* eslint require-atomic-updates: error */ + +async function foo(obj) { + if (!obj.done) { + obj.something = await getSomething(); + } } +``` -function* bar() { - result = (yield) + result; +Example of **correct** code for this rule: - result = (yield somethingElse) + result; +```js +/* eslint require-atomic-updates: error */ + +async function foo(obj) { + if (!obj.done) { + const tmp = await getSomething(); + if (!obj.done) { + obj.something = tmp; + } + } +} +``` + +## Options + +This rule has an object option: + +* `"allowProperties"`: When set to `true`, the rule does not report assignments to properties. Default is `false`. + +### allowProperties + +Example of **correct** code for this rule with the `{ "allowProperties": true }` option: + +```js +/* eslint require-atomic-updates: ["error", { "allowProperties": true }] */ - result = doSomething(yield somethingElse, result); +async function foo(obj) { + if (!obj.done) { + obj.something = await getSomething(); + } } ``` diff --git a/lib/rules/require-atomic-updates.js b/lib/rules/require-atomic-updates.js index 9eee4ca38bc..248b0eb11d0 100644 --- a/lib/rules/require-atomic-updates.js +++ b/lib/rules/require-atomic-updates.js @@ -176,7 +176,17 @@ module.exports = { }, fixable: null, - schema: [], + + schema: [{ + type: "object", + properties: { + allowProperties: { + type: "boolean", + default: false + } + }, + additionalProperties: false + }], messages: { nonAtomicUpdate: "Possible race condition: `{{value}}` might be reassigned based on an outdated value of `{{value}}`.", @@ -185,6 +195,8 @@ module.exports = { }, create(context) { + const allowProperties = !!context.options[0] && context.options[0].allowProperties; + const sourceCode = context.getSourceCode(); const assignmentReferences = new Map(); const segmentInfo = new SegmentInfo(); @@ -284,7 +296,7 @@ module.exports = { value: variable.name } }); - } else { + } else if (!allowProperties) { context.report({ node: node.parent, messageId: "nonAtomicObjectUpdate", diff --git a/tests/lib/rules/require-atomic-updates.js b/tests/lib/rules/require-atomic-updates.js index be931b7e1d0..53d85b75289 100644 --- a/tests/lib/rules/require-atomic-updates.js +++ b/tests/lib/rules/require-atomic-updates.js @@ -214,7 +214,29 @@ ruleTester.run("require-atomic-updates", rule, { await a; b = 1; } - ` + `, + + // allowProperties + { + code: ` + async function a(foo) { + if (foo.bar) { + foo.bar = await something; + } + } + `, + options: [{ allowProperties: true }] + }, + { + code: ` + function* g(foo) { + baz = foo.bar; + yield something; + foo.bar = 1; + } + `, + options: [{ allowProperties: true }] + } ], invalid: [ @@ -358,6 +380,99 @@ ruleTester.run("require-atomic-updates", rule, { line: 8 } ] + }, + + // allowProperties + { + code: ` + async function a(foo) { + if (foo.bar) { + foo.bar = await something; + } + } + `, + errors: [STATIC_PROPERTY_ERROR] + }, + { + code: ` + function* g(foo) { + baz = foo.bar; + yield something; + foo.bar = 1; + } + `, + errors: [STATIC_PROPERTY_ERROR] + }, + { + code: ` + async function a(foo) { + if (foo.bar) { + foo.bar = await something; + } + } + `, + options: [{}], + errors: [STATIC_PROPERTY_ERROR] + + }, + { + code: ` + function* g(foo) { + baz = foo.bar; + yield something; + foo.bar = 1; + } + `, + options: [{}], + errors: [STATIC_PROPERTY_ERROR] + }, + { + code: ` + async function a(foo) { + if (foo.bar) { + foo.bar = await something; + } + } + `, + options: [{ allowProperties: false }], + errors: [STATIC_PROPERTY_ERROR] + + }, + { + code: ` + function* g(foo) { + baz = foo.bar; + yield something; + foo.bar = 1; + } + `, + options: [{ allowProperties: false }], + errors: [STATIC_PROPERTY_ERROR] + }, + { + code: ` + let foo; + async function a() { + if (foo) { + foo = await something; + } + } + `, + options: [{ allowProperties: true }], + errors: [VARIABLE_ERROR] + + }, + { + code: ` + let foo; + function* g() { + baz = foo; + yield something; + foo = 1; + } + `, + options: [{ allowProperties: true }], + errors: [VARIABLE_ERROR] } ] });