From af9afe9b3f622aa5eb4caf873025423ba8163c9f Mon Sep 17 00:00:00 2001 From: lauren Date: Tue, 4 Oct 2022 12:29:36 -0700 Subject: [PATCH] Update safe-string-coercion to handle additions of string literals (#25286) * Update safe-string-coercion to handle additions of string literals Adding strings shouldn't trigger a lint violation of this rule, since adding strings are always safe. --- .../safe-string-coercion-test.internal.js | 31 ++++++++++++++--- scripts/eslint-rules/safe-string-coercion.js | 34 ++++++++++++++++--- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/scripts/eslint-rules/__tests__/safe-string-coercion-test.internal.js b/scripts/eslint-rules/__tests__/safe-string-coercion-test.internal.js index 32c58a4351..e1dfc0c69e 100644 --- a/scripts/eslint-rules/__tests__/safe-string-coercion-test.internal.js +++ b/scripts/eslint-rules/__tests__/safe-string-coercion-test.internal.js @@ -11,6 +11,15 @@ const rule = require('../safe-string-coercion'); const {RuleTester} = require('eslint'); + +RuleTester.setDefaultConfig({ + parser: require.resolve('babel-eslint'), + parserOptions: { + ecmaVersion: 6, + sourceType: 'module', + }, +}); + const ruleTester = new RuleTester(); const missingDevCheckMessage = @@ -57,7 +66,7 @@ ruleTester.run('eslint-rules/safe-string-coercion', rule, { } `, ` - if (__DEV__) { checkFormFieldValueStringCoercion (obj) } + if (__DEV__) { checkFormFieldValueStringCoercion (obj) } '' + obj; `, ` @@ -87,6 +96,9 @@ ruleTester.run('eslint-rules/safe-string-coercion', rule, { // doesn't violate this rule. "if (typeof obj === 'string') { if (typeof obj === 'string' && obj.length) {} else {'' + obj} }", "if (typeof obj === 'string') if (typeof obj === 'string' && obj.length) {} else {'' + obj}", + "'' + ''", + "'' + '' + ''", + "`test${foo}` + ''", ], invalid: [ { @@ -145,7 +157,7 @@ ruleTester.run('eslint-rules/safe-string-coercion', rule, { }, { code: ` - if (__D__) { checkFormFieldValueStringCoercion (obj) } + if (__D__) { checkFormFieldValueStringCoercion (obj) } '' + obj; `, errors: [ @@ -156,7 +168,7 @@ ruleTester.run('eslint-rules/safe-string-coercion', rule, { }, { code: ` - if (__DEV__) { checkFormFieldValueStringCoercion (obj) } + if (__DEV__) { checkFormFieldValueStringCoercion (obj) } '' + notobjj; `, errors: [ @@ -172,7 +184,7 @@ ruleTester.run('eslint-rules/safe-string-coercion', rule, { code: ` if (__DEV__) { checkFormFieldValueStringCoercion (obj) } // must be right before the check call - someOtherCode(); + someOtherCode(); '' + objj; `, errors: [ @@ -261,5 +273,16 @@ ruleTester.run('eslint-rules/safe-string-coercion', rule, { }, ], }, + { + code: `'' + obj + ''`, + errors: [ + {message: missingDevCheckMessage + '\n' + message}, + {message: missingDevCheckMessage + '\n' + message}, + ], + }, + { + code: `foo\`text\` + ""`, + errors: [{message: missingDevCheckMessage + '\n' + message}], + }, ], }); diff --git a/scripts/eslint-rules/safe-string-coercion.js b/scripts/eslint-rules/safe-string-coercion.js index 3a6c8ebb14..39259f8974 100644 --- a/scripts/eslint-rules/safe-string-coercion.js +++ b/scripts/eslint-rules/safe-string-coercion.js @@ -17,6 +17,15 @@ function isEmptyLiteral(node) { ); } +function isStringLiteral(node) { + return ( + // TaggedTemplateExpressions can return non-strings + (node.type === 'TemplateLiteral' && + node.parent.type !== 'TaggedTemplateExpression') || + (node.type === 'Literal' && typeof node.value === 'string') + ); +} + // Symbols and Temporal.* objects will throw when using `'' + value`, but that // pattern can be faster than `String(value)` because JS engines can optimize // `+` better in some cases. Therefore, in perf-sensitive production codepaths @@ -120,9 +129,9 @@ function isSafeTypeofExpression(originalValueNode, node) { return false; } -/** +/** Returns true if the code is inside an `if` block that validates the value - excludes symbols and objects. Examples: + excludes symbols and objects. Examples: * if (typeof value === 'string') { } * if (typeof value === 'string' || typeof value === 'number') { } * if (typeof value === 'string' || someOtherTest) { } @@ -259,7 +268,24 @@ function hasCoercionCheck(node) { } } -function plusEmptyString(context, node) { +function isOnlyAddingStrings(node) { + if (node.operator !== '+') { + return; + } + if (isStringLiteral(node.left) && isStringLiteral(node.right)) { + // It's always safe to add string literals + return true; + } + if (node.left.type === 'BinaryExpression' && isStringLiteral(node.right)) { + return isOnlyAddingStrings(node.left); + } +} + +function checkBinaryExpression(context, node) { + if (isOnlyAddingStrings(node)) { + return; + } + if ( node.operator === '+' && (isEmptyLiteral(node.left) || isEmptyLiteral(node.right)) @@ -337,7 +363,7 @@ module.exports = { }, create(context) { return { - BinaryExpression: node => plusEmptyString(context, node), + BinaryExpression: node => checkBinaryExpression(context, node), CallExpression: node => coerceWithStringConstructor(context, node), }; },