From 1c921c6dfd7ddfb0308c8103e53d32c1241475f0 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 14 Sep 2019 09:30:53 +0900 Subject: [PATCH] New: add no-import-assign (fixes #12237) (#12252) * New: add no-import-assign (fixes #12237) * add destructuring and for-in/of syntax * add mutation functions * add a more test * allow seal and preventExtensions * add a test --- docs/rules/no-import-assign.md | 44 ++++ lib/rules/index.js | 1 + lib/rules/no-import-assign.js | 238 +++++++++++++++++++++ tests/lib/rules/no-import-assign.js | 315 ++++++++++++++++++++++++++++ tools/rule-types.json | 1 + 5 files changed, 599 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 dcfe2e1c3c6..8b0abc4ee7a 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..0865cf9a977 --- /dev/null +++ b/lib/rules/no-import-assign.js @@ -0,0 +1,238 @@ +/** + * @fileoverview Rule to flag updates of imported bindings. + * @author Toru Nagashima + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Helpers +//------------------------------------------------------------------------------ + +const { findVariable, getPropertyName } = require("eslint-utils"); + +const MutationMethods = { + Object: new Set([ + "assign", "defineProperties", "defineProperty", "freeze", + "setPrototypeOf" + ]), + Reflect: new Set([ + "defineProperty", "deleteProperty", "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, scope) { + const { parent } = id; + + return ( + ( + parent.type === "MemberExpression" && + parent.object === id && + ( + isAssignmentLeft(parent) || + isOperandOfMutationUnaryOperator(parent) || + isIterationVariable(parent) + ) + ) || + isArgumentOfWellKnownMutationFunction(id, scope) + ); +} + +/** + * 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.type !== "CallExpression" && + node.type !== "ForInStatement" && + node.type !== "ForOfStatement" + ) { + node = node.parent; + } + + return node || id; +} + +//------------------------------------------------------------------------------ +// 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) { + const scope = context.getScope(); + + for (const variable of context.getDeclaredVariables(node)) { + 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: getWriteNode(idNode), + messageId: "readonly", + data: { name: idNode.name } + }); + } else if (shouldCheckMembers && isMemberWrite(idNode, scope)) { + context.report({ + 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 new file mode 100644 index 00000000000..ab65451e33a --- /dev/null +++ b/tests/lib/rules/no-import-assign.js @@ -0,0 +1,315 @@ +/** + * @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: 2018, + sourceType: "module" + }, + globals: { + Reflect: "readonly" + } +}); + +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 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 * as mod from 'mod'; { let mod = 0; mod.named = 1 }", + "import {} from '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); }", + "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: [ + { + 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 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 }] + }, + { + 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} 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", + 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'; 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'; for (mod5.named of foo);", + errors: [{ messageId: "readonlyMember", data: { name: "mod5" }, column: 30 }] + }, + { + code: "import * as mod6 from 'mod'; [mod6.named] = foo", + errors: [{ messageId: "readonlyMember", data: { name: "mod6" }, column: 30 }] + }, + { + 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 }] + }, + { + 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'; 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 mod, * as mod_ns from 'mod'; mod.prop = 0; mod_ns.prop = 0", + errors: [{ messageId: "readonlyMember", data: { name: "mod_ns" }, column: 51 }] + } + ] +}); diff --git a/tools/rule-types.json b/tools/rule-types.json index ea15a2cfd4c..6c72abb6cb2 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",