Skip to content

Commit

Permalink
Rework & store variables in segmentInfo instead of variable names
Browse files Browse the repository at this point in the history
  • Loading branch information
patriscus committed Apr 10, 2021
1 parent f5b4d6a commit 41af7af
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 25 deletions.
46 changes: 21 additions & 25 deletions lib/rules/require-atomic-updates.js
Expand Up @@ -86,42 +86,42 @@ class SegmentInfo {
* @returns {void}
*/
initialize(segment) {
const outdatedReadVariableNames = new Set();
const freshReadVariableNames = new Set();
const outdatedReadVariables = new Set();
const freshReadVariables = new Set();

for (const prevSegment of segment.prevSegments) {
const info = this.info.get(prevSegment);

if (info) {
info.outdatedReadVariableNames.forEach(Set.prototype.add, outdatedReadVariableNames);
info.freshReadVariableNames.forEach(Set.prototype.add, freshReadVariableNames);
info.outdatedReadVariables.forEach(Set.prototype.add, outdatedReadVariables);
info.freshReadVariables.forEach(Set.prototype.add, freshReadVariables);
}
}

this.info.set(segment, { outdatedReadVariableNames, freshReadVariableNames });
this.info.set(segment, { outdatedReadVariables, freshReadVariables });
}

/**
* Mark a given variable as read on given segments.
* @param {PathSegment[]} segments The segments that it read the variable on.
* @param {string} variableName The variable name to be read.
* @param {Variable} variable The variable to be read.
* @returns {void}
*/
markAsRead(segments, variableName) {
markAsRead(segments, variable) {
for (const segment of segments) {
const info = this.info.get(segment);

if (info) {
info.freshReadVariableNames.add(variableName);
info.freshReadVariables.add(variable);

// If a variable is freshly read again, then it's no more out-dated.
info.outdatedReadVariableNames.delete(variableName);
info.outdatedReadVariables.delete(variable);
}
}
}

/**
* Move `freshReadVariableNames` to `outdatedReadVariableNames`.
* Move `freshReadVariables` to `outdatedReadVariables`.
* @param {PathSegment[]} segments The segments to process.
* @returns {void}
*/
Expand All @@ -130,23 +130,23 @@ class SegmentInfo {
const info = this.info.get(segment);

if (info) {
info.freshReadVariableNames.forEach(Set.prototype.add, info.outdatedReadVariableNames);
info.freshReadVariableNames.clear();
info.freshReadVariables.forEach(Set.prototype.add, info.outdatedReadVariables);
info.freshReadVariables.clear();
}
}
}

/**
* Check if a given variable is outdated on the current segments.
* @param {PathSegment[]} segments The current segments.
* @param {string} variableName The variable name to check.
* @param {Variable} variable The variable to check.
* @returns {boolean} `true` if the variable is outdated on the segments.
*/
isOutdated(segments, variableName) {
isOutdated(segments, variable) {
for (const segment of segments) {
const info = this.info.get(segment);

if (info && info.outdatedReadVariableNames.has(variableName)) {
if (info && info.outdatedReadVariables.has(variable)) {
return true;
}
}
Expand Down Expand Up @@ -214,26 +214,22 @@ module.exports = {
if (!reference) {
return;
}
const name = reference.identifier.name;
const variable = reference.resolved;
const writeExpr = getWriteExpr(reference);
const isMemberAccess = reference.identifier.parent.type === "MemberExpression";

// Add a fresh read variable.
if (reference.isRead() && !(writeExpr && writeExpr.parent.operator === "=")) {
segmentInfo.markAsRead(codePath.currentSegments, name);
segmentInfo.markAsRead(codePath.currentSegments, variable);
}

const isVariableDeclaration = isLocalVariableWithoutEscape(variable, isMemberAccess) ||
node.parent.type === "VariableDeclarator";

/*
* Register the variable to verify after ESLint traversed the `writeExpr` node
* if this reference is an assignment to a variable which is referred from other closure.
*/
if (writeExpr &&
writeExpr.parent.right === writeExpr &&
!isVariableDeclaration
writeExpr.parent.right === writeExpr && // ← exclude variable declarations.
!isLocalVariableWithoutEscape(variable, isMemberAccess)
) {
let refs = assignmentReferences.get(writeExpr);

Expand All @@ -248,7 +244,7 @@ module.exports = {

/*
* Verify assignments.
* If the reference exists in `outdatedReadVariableNames` list, report it.
* If the reference exists in `outdatedReadVariables` list, report it.
*/
":expression:exit"(node) {
const { codePath, referenceMap } = stack;
Expand All @@ -270,9 +266,9 @@ module.exports = {
assignmentReferences.delete(node);

for (const reference of references) {
const name = reference.identifier.name;
const variable = reference.resolved;

if (segmentInfo.isOutdated(codePath.currentSegments, name)) {
if (segmentInfo.isOutdated(codePath.currentSegments, variable)) {
context.report({
node: node.parent,
messageId: "nonAtomicUpdate",
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/require-atomic-updates.js
Expand Up @@ -186,6 +186,21 @@ ruleTester.run("require-atomic-updates", rule, {
const prop = props.find(a => a.id === entry2.id) || null;
}
}
`,

`
async function run() {
{
let entry;
await entry;
}
{
let entry;
() => entry;
entry = 1; // false positive
}
}
`
],

Expand Down

0 comments on commit 41af7af

Please sign in to comment.