From d9c7cfaf54b7fb55022cce6d79b355b052a93267 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Tue, 10 Sep 2019 10:39:14 +0900 Subject: [PATCH 1/6] New: add no-import-assign (fixes #12237) --- docs/rules/no-import-assign.md | 44 ++++++++++++ lib/rules/index.js | 1 + lib/rules/no-import-assign.js | 95 +++++++++++++++++++++++++ tests/lib/rules/no-import-assign.js | 104 ++++++++++++++++++++++++++++ tools/rule-types.json | 1 + 5 files changed, 245 insertions(+) create mode 100644 docs/rules/no-import-assign.md create mode 100644 lib/rules/no-import-assign.js create mode 100644 tests/lib/rules/no-import-assign.js diff --git a/docs/rules/no-import-assign.md b/docs/rules/no-import-assign.md new file mode 100644 index 00000000000..78b228dd828 --- /dev/null +++ b/docs/rules/no-import-assign.md @@ -0,0 +1,44 @@ +# disallow assigning to imported bindings (no-import-assign) + +The updates of imported bindings by ES Modules cause runtime errors. + +## Rule Details + +This rule warns the assignments, increments, and decrements of imported bindings. + +Examples of **incorrect** code for this rule: + +```js +/*eslint no-import-assign: "error"*/ + +import mod, { named } from "./mod.mjs" +import * as mod_ns from "./mod.mjs" + +mod = 1 // ERROR: 'mod' is readonly. +named = 2 // ERROR: 'named' is readonly. +mod_ns.named = 3 // ERROR: the members of 'mod_ns' is readonly. +mod_ns = {} // ERROR: 'mod_ns' is readonly. +``` + +Examples of **correct** code for this rule: + +```js +/*eslint no-import-assign: "error"*/ + +import mod, { named } from "./mod.mjs" +import * as mod_ns from "./mod.mjs" + +mod.prop = 1 +named.prop = 2 +mod_ns.named.prop = 3 + +// Known Limitation +function test(obj) { + obj.named = 4 // Not errored because 'obj' is not namespace objects. +} +test(mod_ns) // Not errored because it doesn't know that 'test' updates the member of the argument. +``` + +## When Not To Use It + +If you don't want to be notified about modifying imported bindings, you can disable this rule. diff --git a/lib/rules/index.js b/lib/rules/index.js index 51d224d219f..efa3d42aafd 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -132,6 +132,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({ "no-implicit-coercion": () => require("./no-implicit-coercion"), "no-implicit-globals": () => require("./no-implicit-globals"), "no-implied-eval": () => require("./no-implied-eval"), + "no-import-assign": () => require("./no-import-assign"), "no-inline-comments": () => require("./no-inline-comments"), "no-inner-declarations": () => require("./no-inner-declarations"), "no-invalid-regexp": () => require("./no-invalid-regexp"), diff --git a/lib/rules/no-import-assign.js b/lib/rules/no-import-assign.js new file mode 100644 index 00000000000..d7e1bbac7fe --- /dev/null +++ b/lib/rules/no-import-assign.js @@ -0,0 +1,95 @@ +/** + * @fileoverview Rule to flag updates of imported bindings. + * @author Toru Nagashima + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +/** + * Check if the identifier node is placed at to update members. + * @param {ASTNode} id The Identifier node to check. + * @returns {boolean} `true` if the member of `id` was updated. + */ +function isMemberUpdate(id) { + const parent = id.parent; + const grandparent = parent.parent; + + return ( + parent.type === "MemberExpression" && + parent.object === id && + ( + ( + grandparent.type === "AssignmentExpression" && + grandparent.left === parent + ) || + ( + grandparent.type === "UpdateExpression" && + grandparent.argument === parent + ) || + ( + grandparent.type === "UnaryExpression" && + grandparent.operator === "delete" && + grandparent.argument === parent + ) + ) + ); +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + type: "problem", + + docs: { + description: "disallow assigning to imported bindings", + category: "Possible Errors", + recommended: false, + url: "https://eslint.org/docs/rules/no-import-assign" + }, + + schema: [], + + messages: { + readonly: "'{{name}}' is read-only.", + readonlyMember: "The members of '{{name}}' are read-only." + } + }, + + create(context) { + return { + ImportDeclaration(node) { + for (const variable of context.getDeclaredVariables(node)) { + const shouldCheckMembers = variable.defs.some( + d => d.node.type === "ImportNamespaceSpecifier" + ); + + for (const reference of variable.references) { + const idNode = reference.identifier; + + if (reference.isWrite()) { + context.report({ + node: idNode.parent, + messageId: "readonly", + data: { name: idNode.name } + }); + } else if (shouldCheckMembers && isMemberUpdate(idNode)) { + context.report({ + node: idNode.parent.parent, + messageId: "readonlyMember", + data: { name: idNode.name } + }); + } + } + } + } + }; + + } +}; diff --git a/tests/lib/rules/no-import-assign.js b/tests/lib/rules/no-import-assign.js new file mode 100644 index 00000000000..e7571fe105a --- /dev/null +++ b/tests/lib/rules/no-import-assign.js @@ -0,0 +1,104 @@ +/** + * @fileoverview Tests for no-import-assign rule. + * @author Toru Nagashima + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const rule = require("../../../lib/rules/no-import-assign"), + { RuleTester } = require("../../../lib/rule-tester"); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2015, + sourceType: "module" + } +}); + +ruleTester.run("no-import-assign", rule, { + valid: [ + "import mod from 'mod'; mod.prop = 0", + "import mod from 'mod'; mod.prop += 0", + "import mod from 'mod'; mod.prop++", + "import mod from 'mod'; delete mod.prop", + "import {named} from 'mod'; named.prop = 0", + "import {named} from 'mod'; named.prop += 0", + "import {named} from 'mod'; named.prop++", + "import {named} from 'mod'; delete named.prop", + "import * as mod from 'mod'; mod.named.prop = 0", + "import * as mod from 'mod'; mod.named.prop += 0", + "import * as mod from 'mod'; mod.named.prop++", + "import * as mod from 'mod'; delete mod.named.prop", + "import * as mod from 'mod'; obj[mod] = 0", + "import * as mod from 'mod'; obj[mod.named] = 0", + "import mod from 'mod'; { let mod = 0; mod = 1 }", + "import * as mod from 'mod'; { let mod = 0; mod = 1 }", + "import {} from 'mod'" + ], + invalid: [ + { + code: "import mod1 from 'mod'; mod1 = 0", + errors: [{ messageId: "readonly", data: { name: "mod1" }, column: 25 }] + }, + { + code: "import mod2 from 'mod'; mod2 += 0", + errors: [{ messageId: "readonly", data: { name: "mod2" }, column: 25 }] + }, + { + code: "import mod3 from 'mod'; mod3++", + errors: [{ messageId: "readonly", data: { name: "mod3" }, column: 25 }] + }, + { + code: "import {named1} from 'mod'; named1 = 0", + errors: [{ messageId: "readonly", data: { name: "named1" }, column: 29 }] + }, + { + code: "import {named2} from 'mod'; named2 += 0", + errors: [{ messageId: "readonly", data: { name: "named2" }, column: 29 }] + }, + { + code: "import {named3} from 'mod'; named3++", + errors: [{ messageId: "readonly", data: { name: "named3" }, column: 29 }] + }, + { + code: "import {named4 as foo} from 'mod'; foo = 0; named4 = 0", + errors: [{ messageId: "readonly", data: { name: "foo" }, column: 36 }] + }, + { + code: "import * as mod1 from 'mod'; mod1 = 0", + errors: [{ messageId: "readonly", data: { name: "mod1" }, column: 30 }] + }, + { + code: "import * as mod2 from 'mod'; mod2 += 0", + errors: [{ messageId: "readonly", data: { name: "mod2" }, column: 30 }] + }, + { + code: "import * as mod3 from 'mod'; mod3++", + errors: [{ messageId: "readonly", data: { name: "mod3" }, column: 30 }] + }, + { + code: "import * as mod4 from 'mod'; mod4.named = 0", + errors: [{ messageId: "readonlyMember", data: { name: "mod4" }, column: 30 }] + }, + { + code: "import * as mod5 from 'mod'; mod5.named += 0", + errors: [{ messageId: "readonlyMember", data: { name: "mod5" }, column: 30 }] + }, + { + code: "import * as mod6 from 'mod'; mod6.named++", + errors: [{ messageId: "readonlyMember", data: { name: "mod6" }, column: 30 }] + }, + { + code: "import * as mod7 from 'mod'; delete mod7.named", + errors: [{ messageId: "readonlyMember", data: { name: "mod7" }, column: 30 }] + } + ] +}); diff --git a/tools/rule-types.json b/tools/rule-types.json index 74cae9391cb..5e5514f5506 100644 --- a/tools/rule-types.json +++ b/tools/rule-types.json @@ -119,6 +119,7 @@ "no-implicit-coercion": "suggestion", "no-implicit-globals": "suggestion", "no-implied-eval": "suggestion", + "no-import-assign": "problem", "no-inline-comments": "suggestion", "no-inner-declarations": "problem", "no-invalid-regexp": "problem", From 3b77e77a8a138faf41dae0fcff263bc8565e977e Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Wed, 11 Sep 2019 06:05:15 +0900 Subject: [PATCH 2/6] add destructuring and for-in/of syntax --- lib/rules/no-import-assign.js | 60 +++++++++- tests/lib/rules/no-import-assign.js | 172 ++++++++++++++++++++++++++-- 2 files changed, 220 insertions(+), 12 deletions(-) diff --git a/lib/rules/no-import-assign.js b/lib/rules/no-import-assign.js index d7e1bbac7fe..37db3c6cea5 100644 --- a/lib/rules/no-import-assign.js +++ b/lib/rules/no-import-assign.js @@ -14,7 +14,7 @@ * @param {ASTNode} id The Identifier node to check. * @returns {boolean} `true` if the member of `id` was updated. */ -function isMemberUpdate(id) { +function isMemberWrite(id) { const parent = id.parent; const grandparent = parent.parent; @@ -34,11 +34,52 @@ function isMemberUpdate(id) { grandparent.type === "UnaryExpression" && grandparent.operator === "delete" && grandparent.argument === parent + ) || + grandparent.type === "ArrayPattern" || + ( + grandparent.type === "Property" && + grandparent.value === parent && + grandparent.parent.type === "ObjectPattern" + ) || + grandparent.type === "RestElement" || + ( + grandparent.type === "AssignmentPattern" && + grandparent.left === parent + ) || + ( + grandparent.type === "ForInStatement" && + grandparent.left === parent + ) || + ( + grandparent.type === "ForOfStatement" && + grandparent.left === parent ) ) ); } +/** + * Get the mutation node. + * @param {ASTNode} id The Identifier node to get. + * @returns {ASTNode} The mutation node. + */ +function getWriteNode(id) { + let node = id.parent; + + while ( + node && + node.type !== "AssignmentExpression" && + node.type !== "UpdateExpression" && + !(node.type === "UnaryExpression" && node.operator === "delete") && + node.type !== "ForInStatement" && + node.type !== "ForOfStatement" + ) { + node = node.parent; + } + + return node || id; +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -69,19 +110,30 @@ module.exports = { const shouldCheckMembers = variable.defs.some( d => d.node.type === "ImportNamespaceSpecifier" ); + let prevIdNode = null; for (const reference of variable.references) { const idNode = reference.identifier; + /* + * AssignmentPattern (e.g. `[a = 0] = b`) makes two write + * references for the same identifier. This should skip + * the one of the two in order to prevent redundant reports. + */ + if (idNode === prevIdNode) { + continue; + } + prevIdNode = idNode; + if (reference.isWrite()) { context.report({ - node: idNode.parent, + node: getWriteNode(idNode), messageId: "readonly", data: { name: idNode.name } }); - } else if (shouldCheckMembers && isMemberUpdate(idNode)) { + } else if (shouldCheckMembers && isMemberWrite(idNode)) { context.report({ - node: idNode.parent.parent, + node: getWriteNode(idNode), messageId: "readonlyMember", data: { name: idNode.name } }); diff --git a/tests/lib/rules/no-import-assign.js b/tests/lib/rules/no-import-assign.js index e7571fe105a..6c224569301 100644 --- a/tests/lib/rules/no-import-assign.js +++ b/tests/lib/rules/no-import-assign.js @@ -18,7 +18,7 @@ const rule = require("../../../lib/rules/no-import-assign"), const ruleTester = new RuleTester({ parserOptions: { - ecmaVersion: 2015, + ecmaVersion: 2018, sourceType: "module" } }); @@ -29,19 +29,47 @@ ruleTester.run("no-import-assign", rule, { "import mod from 'mod'; mod.prop += 0", "import mod from 'mod'; mod.prop++", "import mod from 'mod'; delete mod.prop", + "import mod from 'mod'; for (mod.prop in foo);", + "import mod from 'mod'; for (mod.prop of foo);", + "import mod from 'mod'; [mod.prop] = foo;", + "import mod from 'mod'; [...mod.prop] = foo;", + "import mod from 'mod'; ({ bar: mod.prop } = foo);", + "import mod from 'mod'; ({ ...mod.prop } = foo);", "import {named} from 'mod'; named.prop = 0", "import {named} from 'mod'; named.prop += 0", "import {named} from 'mod'; named.prop++", "import {named} from 'mod'; delete named.prop", + "import {named} from 'mod'; for (named.prop in foo);", + "import {named} from 'mod'; for (named.prop of foo);", + "import {named} from 'mod'; [named.prop] = foo;", + "import {named} from 'mod'; [...named.prop] = foo;", + "import {named} from 'mod'; ({ bar: named.prop } = foo);", + "import {named} from 'mod'; ({ ...named.prop } = foo);", "import * as mod from 'mod'; mod.named.prop = 0", "import * as mod from 'mod'; mod.named.prop += 0", "import * as mod from 'mod'; mod.named.prop++", "import * as mod from 'mod'; delete mod.named.prop", + "import * as mod from 'mod'; for (mod.named.prop in foo);", + "import * as mod from 'mod'; for (mod.named.prop of foo);", + "import * as mod from 'mod'; [mod.named.prop] = foo;", + "import * as mod from 'mod'; [...mod.named.prop] = foo;", + "import * as mod from 'mod'; ({ bar: mod.named.prop } = foo);", + "import * as mod from 'mod'; ({ ...mod.named.prop } = foo);", "import * as mod from 'mod'; obj[mod] = 0", "import * as mod from 'mod'; obj[mod.named] = 0", + "import * as mod from 'mod'; for (var foo in mod.named);", + "import * as mod from 'mod'; for (var foo of mod.named);", + "import * as mod from 'mod'; [bar = mod.named] = foo;", + "import * as mod from 'mod'; ({ bar = mod.named } = foo);", + "import * as mod from 'mod'; ({ bar: baz = mod.named } = foo);", + "import * as mod from 'mod'; ({ [mod.named]: bar } = foo);", + "import * as mod from 'mod'; var obj = { ...mod.named };", + "import * as mod from 'mod'; var obj = { foo: mod.named };", "import mod from 'mod'; { let mod = 0; mod = 1 }", "import * as mod from 'mod'; { let mod = 0; mod = 1 }", - "import {} from 'mod'" + "import * as mod from 'mod'; { let mod = 0; mod.named = 1 }", + "import {} from 'mod'", + "import 'mod'" ], invalid: [ { @@ -56,6 +84,38 @@ ruleTester.run("no-import-assign", rule, { code: "import mod3 from 'mod'; mod3++", errors: [{ messageId: "readonly", data: { name: "mod3" }, column: 25 }] }, + { + code: "import mod4 from 'mod'; for (mod4 in foo);", + errors: [{ messageId: "readonly", data: { name: "mod4" }, column: 25 }] + }, + { + code: "import mod5 from 'mod'; for (mod5 of foo);", + errors: [{ messageId: "readonly", data: { name: "mod5" }, column: 25 }] + }, + { + code: "import mod6 from 'mod'; [mod6] = foo", + errors: [{ messageId: "readonly", data: { name: "mod6" }, column: 25 }] + }, + { + code: "import mod7 from 'mod'; [mod7 = 0] = foo", + errors: [{ messageId: "readonly", data: { name: "mod7" }, column: 25 }] + }, + { + code: "import mod8 from 'mod'; [...mod8] = foo", + errors: [{ messageId: "readonly", data: { name: "mod8" }, column: 25 }] + }, + { + code: "import mod9 from 'mod'; ({ bar: mod9 } = foo)", + errors: [{ messageId: "readonly", data: { name: "mod9" }, column: 26 }] + }, + { + code: "import mod10 from 'mod'; ({ bar: mod10 = 0 } = foo)", + errors: [{ messageId: "readonly", data: { name: "mod10" }, column: 27 }] + }, + { + code: "import mod11 from 'mod'; ({ ...mod11 } = foo)", + errors: [{ messageId: "readonly", data: { name: "mod11" }, column: 27 }] + }, { code: "import {named1} from 'mod'; named1 = 0", errors: [{ messageId: "readonly", data: { name: "named1" }, column: 29 }] @@ -69,8 +129,40 @@ ruleTester.run("no-import-assign", rule, { errors: [{ messageId: "readonly", data: { name: "named3" }, column: 29 }] }, { - code: "import {named4 as foo} from 'mod'; foo = 0; named4 = 0", - errors: [{ messageId: "readonly", data: { name: "foo" }, column: 36 }] + code: "import {named4} from 'mod'; for (named4 in foo);", + errors: [{ messageId: "readonly", data: { name: "named4" }, column: 29 }] + }, + { + code: "import {named5} from 'mod'; for (named5 of foo);", + errors: [{ messageId: "readonly", data: { name: "named5" }, column: 29 }] + }, + { + code: "import {named6} from 'mod'; [named6] = foo", + errors: [{ messageId: "readonly", data: { name: "named6" }, column: 29 }] + }, + { + code: "import {named7} from 'mod'; [named7 = 0] = foo", + errors: [{ messageId: "readonly", data: { name: "named7" }, column: 29 }] + }, + { + code: "import {named8} from 'mod'; [...named8] = foo", + errors: [{ messageId: "readonly", data: { name: "named8" }, column: 29 }] + }, + { + code: "import {named9} from 'mod'; ({ bar: named9 } = foo)", + errors: [{ messageId: "readonly", data: { name: "named9" }, column: 30 }] + }, + { + code: "import {named10} from 'mod'; ({ bar: named10 = 0 } = foo)", + errors: [{ messageId: "readonly", data: { name: "named10" }, column: 31 }] + }, + { + code: "import {named11} from 'mod'; ({ ...named11 } = foo)", + errors: [{ messageId: "readonly", data: { name: "named11" }, column: 31 }] + }, + { + code: "import {named12 as foo} from 'mod'; foo = 0; named12 = 0", + errors: [{ messageId: "readonly", data: { name: "foo" }, column: 37 }] }, { code: "import * as mod1 from 'mod'; mod1 = 0", @@ -85,20 +177,84 @@ ruleTester.run("no-import-assign", rule, { errors: [{ messageId: "readonly", data: { name: "mod3" }, column: 30 }] }, { - code: "import * as mod4 from 'mod'; mod4.named = 0", + code: "import * as mod4 from 'mod'; for (mod4 in foo);", + errors: [{ messageId: "readonly", data: { name: "mod4" }, column: 30 }] + }, + { + code: "import * as mod5 from 'mod'; for (mod5 of foo);", + errors: [{ messageId: "readonly", data: { name: "mod5" }, column: 30 }] + }, + { + code: "import * as mod6 from 'mod'; [mod6] = foo", + errors: [{ messageId: "readonly", data: { name: "mod6" }, column: 30 }] + }, + { + code: "import * as mod7 from 'mod'; [mod7 = 0] = foo", + errors: [{ messageId: "readonly", data: { name: "mod7" }, column: 30 }] + }, + { + code: "import * as mod8 from 'mod'; [...mod8] = foo", + errors: [{ messageId: "readonly", data: { name: "mod8" }, column: 30 }] + }, + { + code: "import * as mod9 from 'mod'; ({ bar: mod9 } = foo)", + errors: [{ messageId: "readonly", data: { name: "mod9" }, column: 31 }] + }, + { + code: "import * as mod10 from 'mod'; ({ bar: mod10 = 0 } = foo)", + errors: [{ messageId: "readonly", data: { name: "mod10" }, column: 32 }] + }, + { + code: "import * as mod11 from 'mod'; ({ ...mod11 } = foo)", + errors: [{ messageId: "readonly", data: { name: "mod11" }, column: 32 }] + }, + { + code: "import * as mod1 from 'mod'; mod1.named = 0", + errors: [{ messageId: "readonlyMember", data: { name: "mod1" }, column: 30 }] + }, + { + code: "import * as mod2 from 'mod'; mod2.named += 0", + errors: [{ messageId: "readonlyMember", data: { name: "mod2" }, column: 30 }] + }, + { + code: "import * as mod3 from 'mod'; mod3.named++", + errors: [{ messageId: "readonlyMember", data: { name: "mod3" }, column: 30 }] + }, + { + code: "import * as mod4 from 'mod'; for (mod4.named in foo);", errors: [{ messageId: "readonlyMember", data: { name: "mod4" }, column: 30 }] }, { - code: "import * as mod5 from 'mod'; mod5.named += 0", + code: "import * as mod5 from 'mod'; for (mod5.named of foo);", errors: [{ messageId: "readonlyMember", data: { name: "mod5" }, column: 30 }] }, { - code: "import * as mod6 from 'mod'; mod6.named++", + code: "import * as mod6 from 'mod'; [mod6.named] = foo", errors: [{ messageId: "readonlyMember", data: { name: "mod6" }, column: 30 }] }, { - code: "import * as mod7 from 'mod'; delete mod7.named", + code: "import * as mod7 from 'mod'; [mod7.named = 0] = foo", errors: [{ messageId: "readonlyMember", data: { name: "mod7" }, column: 30 }] + }, + { + code: "import * as mod8 from 'mod'; [...mod8.named] = foo", + errors: [{ messageId: "readonlyMember", data: { name: "mod8" }, column: 30 }] + }, + { + code: "import * as mod9 from 'mod'; ({ bar: mod9.named } = foo)", + errors: [{ messageId: "readonlyMember", data: { name: "mod9" }, column: 31 }] + }, + { + code: "import * as mod10 from 'mod'; ({ bar: mod10.named = 0 } = foo)", + errors: [{ messageId: "readonlyMember", data: { name: "mod10" }, column: 32 }] + }, + { + code: "import * as mod11 from 'mod'; ({ ...mod11.named } = foo)", + errors: [{ messageId: "readonlyMember", data: { name: "mod11" }, column: 32 }] + }, + { + code: "import * as mod12 from 'mod'; delete mod12.named", + errors: [{ messageId: "readonlyMember", data: { name: "mod12" }, column: 31 }] } ] }); From 0e9f2e34f17fa7b6a3988ef62a1bc0c2a1520225 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Wed, 11 Sep 2019 07:12:04 +0900 Subject: [PATCH 3/6] add mutation functions --- lib/rules/no-import-assign.js | 168 +++++++++++++++++++++------- tests/lib/rules/no-import-assign.js | 61 +++++++++- 2 files changed, 190 insertions(+), 39 deletions(-) diff --git a/lib/rules/no-import-assign.js b/lib/rules/no-import-assign.js index 37db3c6cea5..d19fb4de926 100644 --- a/lib/rules/no-import-assign.js +++ b/lib/rules/no-import-assign.js @@ -9,52 +9,141 @@ // Helpers //------------------------------------------------------------------------------ +const { findVariable, getPropertyName } = require("eslint-utils"); + +const MutationMethods = { + Object: new Set([ + "assign", "defineProperties", "defineProperty", "freeze", + "preventExtensions", "seal", "setPrototypeOf" + ]), + Reflect: new Set([ + "defineProperty", "deleteProperty", "preventExtensions", "set", + "setPrototypeOf" + ]) +}; + +/** + * Check if a given node is LHS of an assignment node. + * @param {ASTNode} node The node to check. + * @returns {boolean} `true` if the node is LHS. + */ +function isAssignmentLeft(node) { + const { parent } = node; + + return ( + ( + parent.type === "AssignmentExpression" && + parent.left === node + ) || + + // Destructuring assignments + parent.type === "ArrayPattern" || + ( + parent.type === "Property" && + parent.value === node && + parent.parent.type === "ObjectPattern" + ) || + parent.type === "RestElement" || + ( + parent.type === "AssignmentPattern" && + parent.left === node + ) + ); +} + +/** + * Check if a given node is the operand of mutation unary operator. + * @param {ASTNode} node The node to check. + * @returns {boolean} `true` if the node is the operand of mutation unary operator. + */ +function isOperandOfMutationUnaryOperator(node) { + const { parent } = node; + + return ( + ( + parent.type === "UpdateExpression" && + parent.argument === node + ) || + ( + parent.type === "UnaryExpression" && + parent.operator === "delete" && + parent.argument === node + ) + ); +} + +/** + * Check if a given node is the iteration variable of `for-in`/`for-of` syntax. + * @param {ASTNode} node The node to check. + * @returns {boolean} `true` if the node is the iteration variable. + */ +function isIterationVariable(node) { + const { parent } = node; + + return ( + ( + parent.type === "ForInStatement" && + parent.left === node + ) || + ( + parent.type === "ForOfStatement" && + parent.left === node + ) + ); +} + +/** + * Check if a given node is the iteration variable of `for-in`/`for-of` syntax. + * @param {ASTNode} node The node to check. + * @param {Scope} scope A `escope.Scope` object to find variable (whichever). + * @returns {boolean} `true` if the node is the iteration variable. + */ +function isArgumentOfWellKnownMutationFunction(node, scope) { + const { parent } = node; + + if ( + parent.type === "CallExpression" && + parent.arguments[0] === node && + parent.callee.type === "MemberExpression" && + parent.callee.object.type === "Identifier" + ) { + const { callee } = parent; + const { object } = callee; + + if (Object.keys(MutationMethods).includes(object.name)) { + const variable = findVariable(scope, object); + + return ( + variable !== null && + variable.scope.type === "global" && + MutationMethods[object.name].has(getPropertyName(callee, scope)) + ); + } + } + + return false; +} + /** * Check if the identifier node is placed at to update members. * @param {ASTNode} id The Identifier node to check. + * @param {Scope} scope A `escope.Scope` object to find variable (whichever). * @returns {boolean} `true` if the member of `id` was updated. */ -function isMemberWrite(id) { - const parent = id.parent; - const grandparent = parent.parent; +function isMemberWrite(id, scope) { + const { parent } = id; return ( - parent.type === "MemberExpression" && - parent.object === id && ( + parent.type === "MemberExpression" && + parent.object === id && ( - grandparent.type === "AssignmentExpression" && - grandparent.left === parent - ) || - ( - grandparent.type === "UpdateExpression" && - grandparent.argument === parent - ) || - ( - grandparent.type === "UnaryExpression" && - grandparent.operator === "delete" && - grandparent.argument === parent - ) || - grandparent.type === "ArrayPattern" || - ( - grandparent.type === "Property" && - grandparent.value === parent && - grandparent.parent.type === "ObjectPattern" - ) || - grandparent.type === "RestElement" || - ( - grandparent.type === "AssignmentPattern" && - grandparent.left === parent - ) || - ( - grandparent.type === "ForInStatement" && - grandparent.left === parent - ) || - ( - grandparent.type === "ForOfStatement" && - grandparent.left === parent + isAssignmentLeft(parent) || + isOperandOfMutationUnaryOperator(parent) || + isIterationVariable(parent) ) - ) + ) || + isArgumentOfWellKnownMutationFunction(id, scope) ); } @@ -70,7 +159,8 @@ function getWriteNode(id) { node && node.type !== "AssignmentExpression" && node.type !== "UpdateExpression" && - !(node.type === "UnaryExpression" && node.operator === "delete") && + node.type !== "UnaryExpression" && + node.type !== "CallExpression" && node.type !== "ForInStatement" && node.type !== "ForOfStatement" ) { @@ -106,6 +196,8 @@ module.exports = { create(context) { return { ImportDeclaration(node) { + const scope = context.getScope(); + for (const variable of context.getDeclaredVariables(node)) { const shouldCheckMembers = variable.defs.some( d => d.node.type === "ImportNamespaceSpecifier" @@ -131,7 +223,7 @@ module.exports = { messageId: "readonly", data: { name: idNode.name } }); - } else if (shouldCheckMembers && isMemberWrite(idNode)) { + } else if (shouldCheckMembers && isMemberWrite(idNode, scope)) { context.report({ node: getWriteNode(idNode), messageId: "readonlyMember", diff --git a/tests/lib/rules/no-import-assign.js b/tests/lib/rules/no-import-assign.js index 6c224569301..c491c0cdd99 100644 --- a/tests/lib/rules/no-import-assign.js +++ b/tests/lib/rules/no-import-assign.js @@ -20,6 +20,9 @@ const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2018, sourceType: "module" + }, + globals: { + Reflect: "readonly" } }); @@ -69,7 +72,15 @@ ruleTester.run("no-import-assign", rule, { "import * as mod from 'mod'; { let mod = 0; mod = 1 }", "import * as mod from 'mod'; { let mod = 0; mod.named = 1 }", "import {} from 'mod'", - "import 'mod'" + "import 'mod'", + "import mod from 'mod'; Object.assign(mod, obj);", + "import {named} from 'mod'; Object.assign(named, obj);", + "import * as mod from 'mod'; Object.assign(mod.prop, obj);", + "import * as mod from 'mod'; Object.assign(obj, mod, other);", + "import * as mod from 'mod'; Object[assign](mod, obj);", + "import * as mod from 'mod'; Object.getPrototypeOf(mod);", + "import * as mod from 'mod'; Reflect.set(obj, key, mod);", + "import * as mod from 'mod'; { var Object; Object.assign(mod, obj); }" ], invalid: [ { @@ -255,6 +266,54 @@ ruleTester.run("no-import-assign", rule, { { code: "import * as mod12 from 'mod'; delete mod12.named", errors: [{ messageId: "readonlyMember", data: { name: "mod12" }, column: 31 }] + }, + { + code: "import * as mod from 'mod'; Object.assign(mod, obj)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Object.defineProperty(mod, key, d)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Object.defineProperties(mod, d)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Object.setPrototypeOf(mod, proto)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Object.freeze(mod)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Object.seal(mod, obj)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Object.preventExtensions(mod)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Reflect.defineProperty(mod, key, d)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Reflect.deleteProperty(mod, key)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Reflect.set(mod, key, value)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Reflect.setPrototypeOf(mod, proto)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import * as mod from 'mod'; Reflect.preventExtensions(mod)", + errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] } ] }); From c20208f83742da12e678db8936ba00e1c431537b Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Wed, 11 Sep 2019 09:20:56 +0900 Subject: [PATCH 4/6] add a more test --- tests/lib/rules/no-import-assign.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/lib/rules/no-import-assign.js b/tests/lib/rules/no-import-assign.js index c491c0cdd99..fca07cdb806 100644 --- a/tests/lib/rules/no-import-assign.js +++ b/tests/lib/rules/no-import-assign.js @@ -80,7 +80,8 @@ ruleTester.run("no-import-assign", rule, { "import * as mod from 'mod'; Object[assign](mod, obj);", "import * as mod from 'mod'; Object.getPrototypeOf(mod);", "import * as mod from 'mod'; Reflect.set(obj, key, mod);", - "import * as mod from 'mod'; { var Object; Object.assign(mod, obj); }" + "import * as mod from 'mod'; { var Object; Object.assign(mod, obj); }", + "import * as mod from 'mod'; var Object; Object.assign(mod, obj);" ], invalid: [ { From d1bec8a135ec24356c7ad2c7d8a5a3f0aaccb519 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 12 Sep 2019 07:14:51 +0900 Subject: [PATCH 5/6] allow seal and preventExtensions --- lib/rules/no-import-assign.js | 5 ++--- tests/lib/rules/no-import-assign.js | 17 ++++------------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/lib/rules/no-import-assign.js b/lib/rules/no-import-assign.js index d19fb4de926..0865cf9a977 100644 --- a/lib/rules/no-import-assign.js +++ b/lib/rules/no-import-assign.js @@ -14,11 +14,10 @@ const { findVariable, getPropertyName } = require("eslint-utils"); const MutationMethods = { Object: new Set([ "assign", "defineProperties", "defineProperty", "freeze", - "preventExtensions", "seal", "setPrototypeOf" + "setPrototypeOf" ]), Reflect: new Set([ - "defineProperty", "deleteProperty", "preventExtensions", "set", - "setPrototypeOf" + "defineProperty", "deleteProperty", "set", "setPrototypeOf" ]) }; diff --git a/tests/lib/rules/no-import-assign.js b/tests/lib/rules/no-import-assign.js index fca07cdb806..10c387c958c 100644 --- a/tests/lib/rules/no-import-assign.js +++ b/tests/lib/rules/no-import-assign.js @@ -81,7 +81,10 @@ ruleTester.run("no-import-assign", rule, { "import * as mod from 'mod'; Object.getPrototypeOf(mod);", "import * as mod from 'mod'; Reflect.set(obj, key, mod);", "import * as mod from 'mod'; { var Object; Object.assign(mod, obj); }", - "import * as mod from 'mod'; var Object; Object.assign(mod, obj);" + "import * as mod from 'mod'; var Object; Object.assign(mod, obj);", + "import * as mod from 'mod'; Object.seal(mod, obj)", + "import * as mod from 'mod'; Object.preventExtensions(mod)", + "import * as mod from 'mod'; Reflect.preventExtensions(mod)" ], invalid: [ { @@ -288,14 +291,6 @@ ruleTester.run("no-import-assign", rule, { code: "import * as mod from 'mod'; Object.freeze(mod)", errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] }, - { - code: "import * as mod from 'mod'; Object.seal(mod, obj)", - errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] - }, - { - code: "import * as mod from 'mod'; Object.preventExtensions(mod)", - errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] - }, { code: "import * as mod from 'mod'; Reflect.defineProperty(mod, key, d)", errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] @@ -311,10 +306,6 @@ ruleTester.run("no-import-assign", rule, { { code: "import * as mod from 'mod'; Reflect.setPrototypeOf(mod, proto)", errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] - }, - { - code: "import * as mod from 'mod'; Reflect.preventExtensions(mod)", - errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] } ] }); From 3e25a00398286f5f51cea27f7eecea161dbf0ea5 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Thu, 12 Sep 2019 13:21:01 +0900 Subject: [PATCH 6/6] add a test --- tests/lib/rules/no-import-assign.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/lib/rules/no-import-assign.js b/tests/lib/rules/no-import-assign.js index 10c387c958c..ab65451e33a 100644 --- a/tests/lib/rules/no-import-assign.js +++ b/tests/lib/rules/no-import-assign.js @@ -306,6 +306,10 @@ ruleTester.run("no-import-assign", rule, { { code: "import * as mod from 'mod'; Reflect.setPrototypeOf(mod, proto)", errors: [{ messageId: "readonlyMember", data: { name: "mod" }, column: 29 }] + }, + { + code: "import mod, * as mod_ns from 'mod'; mod.prop = 0; mod_ns.prop = 0", + errors: [{ messageId: "readonlyMember", data: { name: "mod_ns" }, column: 51 }] } ] });