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.
facebook · Oct 4, 2022 · af9afe9 · af9afe9
1 parent aed33a4
commit af9afe9
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 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
@@ -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
@@ -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),
     };
   },