Skip to content

Commit

Permalink
fix rule behavior
Browse files Browse the repository at this point in the history
- Makes the member accesses of parameters danger.
- Makes closure handling relax.
  • Loading branch information
mysticatea committed May 27, 2019
1 parent e0d8a02 commit bc274e0
Showing 1 changed file with 19 additions and 10 deletions.
29 changes: 19 additions & 10 deletions lib/rules/require-atomic-updates.js
Expand Up @@ -55,18 +55,25 @@ function getWriteExpr(reference) {

/**
* Checks if an expression is a variable that can only be observed within the given function.
* @param {escope.Variable} variable The variable to check
* @returns {boolean} `true` if the variable is local to the given function, and is never referenced in a closure.
* @param {Scope} functionScope The function scope of the current function.
* @param {Variable|null} variable The variable to check
* @param {boolean} isMemberAccess If `true` then this is a member access.
* @returns {boolean} `true` if the variable is local to the given function.
*/
function isLocalVariableWithoutEscape(variable) {
function isLocalVariable(functionScope, variable, isMemberAccess) {
if (!variable) {
return false; // A global variable which was not defined.
}

const functionScope = variable.scope.variableScope;
// If the reference is a property access and the variable is a parameter, it handles the variable is not local.
if (isMemberAccess && variable.defs.some(d => d.type === "Parameter")) {
return false;
}

return variable.references.every(reference =>
reference.from.variableScope === functionScope);
return variable.defs.every(def => (
def.node.range[0] >= functionScope.block.range[0] &&
def.node.range[1] <= functionScope.block.range[1]
));
}

class SegmentInfo {
Expand Down Expand Up @@ -184,7 +191,8 @@ module.exports = {
stack = {
upper: stack,
codePath,
referenceMap: shouldVerify ? createReferenceMap(scope) : null
referenceMap: shouldVerify ? createReferenceMap(scope) : null,
scope
};
},
onCodePathEnd() {
Expand All @@ -198,7 +206,7 @@ module.exports = {

// Handle references to prepare verification.
Identifier(node) {
const { codePath, referenceMap } = stack;
const { codePath, referenceMap, scope } = stack;
const reference = referenceMap && referenceMap.get(node);

// Ignore if this is not a valid variable reference.
Expand All @@ -208,6 +216,7 @@ module.exports = {
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 === "=")) {
Expand All @@ -216,11 +225,11 @@ module.exports = {

/*
* 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 clausure.
* if this reference is an assignment to a local variable.
*/
if (writeExpr &&
writeExpr.parent.right === writeExpr && // ← exclude variable declarations.
!isLocalVariableWithoutEscape(variable)
!isLocalVariable(scope, variable, isMemberAccess)
) {
let refs = assignmentReferences.get(writeExpr);

Expand Down

0 comments on commit bc274e0

Please sign in to comment.