From 7dda297dcd227938e953ac408a09c3777130da3d Mon Sep 17 00:00:00 2001 From: Zzzen Date: Thu, 24 Feb 2022 19:42:27 +0800 Subject: [PATCH 1/6] feat: `valid-typeof`: always ban `undefined` --- lib/rules/valid-typeof.js | 2 ++ tests/lib/rules/valid-typeof.js | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/lib/rules/valid-typeof.js b/lib/rules/valid-typeof.js index 60463581233..73af3bbfaed 100644 --- a/lib/rules/valid-typeof.js +++ b/lib/rules/valid-typeof.js @@ -72,6 +72,8 @@ module.exports = { if (VALID_TYPES.indexOf(value) === -1) { context.report({ node: sibling, messageId: "invalidValue" }); } + } else if (sibling.type === "Identifier" && sibling.name === "undefined") { + context.report({ node: sibling, messageId: "notString" }); } else if (requireStringLiterals && !isTypeofExpression(sibling)) { context.report({ node: sibling, messageId: "notString" }); } diff --git a/tests/lib/rules/valid-typeof.js b/tests/lib/rules/valid-typeof.js index cd28088a0ed..7baf1072d9e 100644 --- a/tests/lib/rules/valid-typeof.js +++ b/tests/lib/rules/valid-typeof.js @@ -141,6 +141,10 @@ ruleTester.run("valid-typeof", rule, { options: [{ requireStringLiterals: true }], errors: [{ messageId: "notString", type: "Identifier" }] }, + { + code: "if (typeof bar !== undefined) {}", + errors: [{ messageId: "notString", type: "Identifier" }] + }, { code: "typeof foo === undefined", options: [{ requireStringLiterals: true }], From 51073f59f2083d9992aa3df018eeca8543ad4815 Mon Sep 17 00:00:00 2001 From: Zzzen Date: Fri, 25 Feb 2022 00:56:45 +0800 Subject: [PATCH 2/6] change error message and check definition of `undefined` --- lib/rules/valid-typeof.js | 23 +++++++++++++++++++++-- tests/lib/rules/valid-typeof.js | 15 ++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/lib/rules/valid-typeof.js b/lib/rules/valid-typeof.js index 73af3bbfaed..e0536409df5 100644 --- a/lib/rules/valid-typeof.js +++ b/lib/rules/valid-typeof.js @@ -44,6 +44,21 @@ module.exports = { const requireStringLiterals = context.options[0] && context.options[0].requireStringLiterals; + let globalScope; + + /** + * Checks whether the given node represents a reference to a global variable that is not declared in the source code. + * These identifiers will be allowed, as it is assumed that user has no control over the names of external global variables. + * @param {ASTNode} node `Identifier` node to check. + * @returns {boolean} `true` if the node is a reference to a global variable. + */ + function isReferenceToGlobalVariable(node) { + const variable = globalScope.set.get(node.name); + + return variable && variable.defs.length === 0 && + variable.references.some(ref => ref.identifier === node); + } + /** * Determines whether a node is a typeof expression. * @param {ASTNode} node The node @@ -59,6 +74,10 @@ module.exports = { return { + Program() { + globalScope = context.getScope(); + }, + UnaryExpression(node) { if (isTypeofExpression(node)) { const parent = context.getAncestors().pop(); @@ -72,8 +91,8 @@ module.exports = { if (VALID_TYPES.indexOf(value) === -1) { context.report({ node: sibling, messageId: "invalidValue" }); } - } else if (sibling.type === "Identifier" && sibling.name === "undefined") { - context.report({ node: sibling, messageId: "notString" }); + } else if (sibling.type === "Identifier" && sibling.name === "undefined" && isReferenceToGlobalVariable(sibling)) { + context.report({ node: sibling, messageId: "invalidValue" }); } else if (requireStringLiterals && !isTypeofExpression(sibling)) { context.report({ node: sibling, messageId: "notString" }); } diff --git a/tests/lib/rules/valid-typeof.js b/tests/lib/rules/valid-typeof.js index 7baf1072d9e..19e467c9584 100644 --- a/tests/lib/rules/valid-typeof.js +++ b/tests/lib/rules/valid-typeof.js @@ -45,6 +45,7 @@ ruleTester.run("valid-typeof", rule, { "typeof(foo) == 'string'", "typeof(foo) != 'string'", "var oddUse = typeof foo + 'thing'", + "function f(undefined) { typeof x === undefined }", { code: "typeof foo === 'number'", options: [{ requireStringLiterals: true }] @@ -137,28 +138,28 @@ ruleTester.run("valid-typeof", rule, { errors: [{ messageId: "invalidValue", type: "Literal" }] }, { - code: "typeof foo == Object", - options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + code: "if (typeof bar !== undefined) {}", + errors: [{ messageId: "invalidValue", type: "Identifier" }] }, { - code: "if (typeof bar !== undefined) {}", + code: "typeof foo == Object", + options: [{ requireStringLiterals: true }], errors: [{ messageId: "notString", type: "Identifier" }] }, { code: "typeof foo === undefined", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + errors: [{ messageId: "invalidValue", type: "Identifier" }] }, { code: "undefined === typeof foo", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + errors: [{ messageId: "invalidValue", type: "Identifier" }] }, { code: "undefined == typeof foo", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + errors: [{ messageId: "invalidValue", type: "Identifier" }] }, { code: "typeof foo === `undefined${foo}`", From f4a96de8a13b1abf557b3965ae730bf63a33c057 Mon Sep 17 00:00:00 2001 From: Zzzen Date: Sat, 26 Feb 2022 15:45:46 +0800 Subject: [PATCH 3/6] check string literal first --- lib/rules/valid-typeof.js | 4 ++-- tests/lib/rules/valid-typeof.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/rules/valid-typeof.js b/lib/rules/valid-typeof.js index e0536409df5..80ec741621d 100644 --- a/lib/rules/valid-typeof.js +++ b/lib/rules/valid-typeof.js @@ -91,10 +91,10 @@ module.exports = { if (VALID_TYPES.indexOf(value) === -1) { context.report({ node: sibling, messageId: "invalidValue" }); } - } else if (sibling.type === "Identifier" && sibling.name === "undefined" && isReferenceToGlobalVariable(sibling)) { - context.report({ node: sibling, messageId: "invalidValue" }); } else if (requireStringLiterals && !isTypeofExpression(sibling)) { context.report({ node: sibling, messageId: "notString" }); + } else if (sibling.type === "Identifier" && sibling.name === "undefined" && isReferenceToGlobalVariable(sibling)) { + context.report({ node: sibling, messageId: "invalidValue" }); } } } diff --git a/tests/lib/rules/valid-typeof.js b/tests/lib/rules/valid-typeof.js index 19e467c9584..6e3c40872f0 100644 --- a/tests/lib/rules/valid-typeof.js +++ b/tests/lib/rules/valid-typeof.js @@ -149,17 +149,17 @@ ruleTester.run("valid-typeof", rule, { { code: "typeof foo === undefined", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "invalidValue", type: "Identifier" }] + errors: [{ messageId: "notString", type: "Identifier" }] }, { code: "undefined === typeof foo", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "invalidValue", type: "Identifier" }] + errors: [{ messageId: "notString", type: "Identifier" }] }, { code: "undefined == typeof foo", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "invalidValue", type: "Identifier" }] + errors: [{ messageId: "notString", type: "Identifier" }] }, { code: "typeof foo === `undefined${foo}`", From f288a7872659b30baca2587f001b7b87e080b3ad Mon Sep 17 00:00:00 2001 From: Zzzen Date: Sun, 27 Feb 2022 17:22:14 +0800 Subject: [PATCH 4/6] add suggestions --- lib/rules/valid-typeof.js | 19 +++++++++++++++++-- tests/lib/rules/valid-typeof.js | 13 ++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/lib/rules/valid-typeof.js b/lib/rules/valid-typeof.js index 80ec741621d..7e0a37b5a9e 100644 --- a/lib/rules/valid-typeof.js +++ b/lib/rules/valid-typeof.js @@ -19,6 +19,8 @@ module.exports = { url: "https://eslint.org/docs/rules/valid-typeof" }, + hasSuggestions: true, + schema: [ { type: "object", @@ -33,7 +35,8 @@ module.exports = { ], messages: { invalidValue: "Invalid typeof comparison value.", - notString: "Typeof comparisons should be to string literals." + notString: "Typeof comparisons should be to string literals.", + suggestString: 'Use `"{{type}}"` instead of `{{type}}`.' } }, @@ -94,7 +97,19 @@ module.exports = { } else if (requireStringLiterals && !isTypeofExpression(sibling)) { context.report({ node: sibling, messageId: "notString" }); } else if (sibling.type === "Identifier" && sibling.name === "undefined" && isReferenceToGlobalVariable(sibling)) { - context.report({ node: sibling, messageId: "invalidValue" }); + context.report({ + node: sibling, + messageId: "invalidValue", + data: { type: "undefined" }, + suggest: [ + { + messageId: "suggestString", + fix(fixer) { + return fixer.replaceText(sibling, '"undefined"'); + } + } + ] + }); } } } diff --git a/tests/lib/rules/valid-typeof.js b/tests/lib/rules/valid-typeof.js index 6e3c40872f0..d77392a4aa2 100644 --- a/tests/lib/rules/valid-typeof.js +++ b/tests/lib/rules/valid-typeof.js @@ -139,7 +139,18 @@ ruleTester.run("valid-typeof", rule, { }, { code: "if (typeof bar !== undefined) {}", - errors: [{ messageId: "invalidValue", type: "Identifier" }] + errors: [ + { + messageId: "invalidValue", + type: "Identifier", + data: { type: "undefined" }, + suggestions: [ + { + messageId: "suggestString", + output: 'if (typeof bar !== "undefined") {}' + } + ] + }] }, { code: "typeof foo == Object", From bf98c4d396c3a4c8e27329dfe3ffd12133ff3217 Mon Sep 17 00:00:00 2001 From: Zzzen Date: Fri, 4 Mar 2022 22:50:41 +0800 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Milos Djermanovic --- lib/rules/valid-typeof.js | 2 +- tests/lib/rules/valid-typeof.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rules/valid-typeof.js b/lib/rules/valid-typeof.js index 7e0a37b5a9e..8fa75fab73f 100644 --- a/lib/rules/valid-typeof.js +++ b/lib/rules/valid-typeof.js @@ -100,10 +100,10 @@ module.exports = { context.report({ node: sibling, messageId: "invalidValue", - data: { type: "undefined" }, suggest: [ { messageId: "suggestString", + data: { type: "undefined" }, fix(fixer) { return fixer.replaceText(sibling, '"undefined"'); } diff --git a/tests/lib/rules/valid-typeof.js b/tests/lib/rules/valid-typeof.js index d77392a4aa2..522a97e4e9c 100644 --- a/tests/lib/rules/valid-typeof.js +++ b/tests/lib/rules/valid-typeof.js @@ -143,10 +143,10 @@ ruleTester.run("valid-typeof", rule, { { messageId: "invalidValue", type: "Identifier", - data: { type: "undefined" }, suggestions: [ { messageId: "suggestString", + data: { type: "undefined" }, output: 'if (typeof bar !== "undefined") {}' } ] From f79dcde528772d0bd55b612bd5c718c380f58dc8 Mon Sep 17 00:00:00 2001 From: Zzzen Date: Fri, 4 Mar 2022 23:07:31 +0800 Subject: [PATCH 6/6] suggest string when `requireStringLiterals` is on --- lib/rules/valid-typeof.js | 6 ++--- tests/lib/rules/valid-typeof.js | 39 ++++++++++++++++++++++++++++++--- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/lib/rules/valid-typeof.js b/lib/rules/valid-typeof.js index 8fa75fab73f..cb85cd9cb90 100644 --- a/lib/rules/valid-typeof.js +++ b/lib/rules/valid-typeof.js @@ -94,12 +94,10 @@ module.exports = { if (VALID_TYPES.indexOf(value) === -1) { context.report({ node: sibling, messageId: "invalidValue" }); } - } else if (requireStringLiterals && !isTypeofExpression(sibling)) { - context.report({ node: sibling, messageId: "notString" }); } else if (sibling.type === "Identifier" && sibling.name === "undefined" && isReferenceToGlobalVariable(sibling)) { context.report({ node: sibling, - messageId: "invalidValue", + messageId: requireStringLiterals ? "notString" : "invalidValue", suggest: [ { messageId: "suggestString", @@ -110,6 +108,8 @@ module.exports = { } ] }); + } else if (requireStringLiterals && !isTypeofExpression(sibling)) { + context.report({ node: sibling, messageId: "notString" }); } } } diff --git a/tests/lib/rules/valid-typeof.js b/tests/lib/rules/valid-typeof.js index 522a97e4e9c..35a52f0a6c0 100644 --- a/tests/lib/rules/valid-typeof.js +++ b/tests/lib/rules/valid-typeof.js @@ -160,17 +160,50 @@ ruleTester.run("valid-typeof", rule, { { code: "typeof foo === undefined", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + errors: [ + { + messageId: "notString", + type: "Identifier", + suggestions: [ + { + messageId: "suggestString", + data: { type: "undefined" }, + output: 'typeof foo === "undefined"' + } + ] + }] }, { code: "undefined === typeof foo", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + errors: [ + { + messageId: "notString", + type: "Identifier", + suggestions: [ + { + messageId: "suggestString", + data: { type: "undefined" }, + output: '"undefined" === typeof foo' + } + ] + }] }, { code: "undefined == typeof foo", options: [{ requireStringLiterals: true }], - errors: [{ messageId: "notString", type: "Identifier" }] + errors: [ + { + messageId: "notString", + type: "Identifier", + suggestions: [ + { + messageId: "suggestString", + data: { type: "undefined" }, + output: '"undefined" == typeof foo' + } + ] + }] }, { code: "typeof foo === `undefined${foo}`",