about summary refs log tree commit diff
path: root/eslint
diff options
context:
space:
mode:
authordan <dan.abramov@gmail.com>2024-04-05 15:09:35 +0100
committerGitHub <noreply@github.com>2024-04-05 15:09:35 +0100
commit46c112edfdcb40681a8997ec4f47b413a08fdd14 (patch)
tree8745de3a743f9231a5151296c2df4fd6e39404e7 /eslint
parent49266c355ea781cbd7a0b373e64143da7740c91e (diff)
downloadvoidsky-46c112edfdcb40681a8997ec4f47b413a08fdd14.tar.zst
Enforce that text is wrapped in <Text>, remaining cases (#3421)
* Toggle.Button -> Toggle.ButtonWithText

* Simplify Prompt.Cancel/Action

* Move lines down for better diff

* Remove ButtonWithText

* Simplify types

* Enforce Button/ButtonText nesting

* Add suggested wrapper in linter error

* Check <Trans> ancestry too

* Also check literals

* Rm ts-ignore
Diffstat (limited to 'eslint')
-rw-r--r--eslint/__tests__/avoid-unwrapped-text.test.js339
-rw-r--r--eslint/avoid-unwrapped-text.js194
2 files changed, 527 insertions, 6 deletions
diff --git a/eslint/__tests__/avoid-unwrapped-text.test.js b/eslint/__tests__/avoid-unwrapped-text.test.js
index 7c667b4a8..a6762b8fd 100644
--- a/eslint/__tests__/avoid-unwrapped-text.test.js
+++ b/eslint/__tests__/avoid-unwrapped-text.test.js
@@ -199,7 +199,7 @@ describe('avoid-unwrapped-text', () => {
 
       {
         code: `
-<View prop={
+<View propText={
   <Trans><Text>foo</Text></Trans>
 }>
   <Bar />
@@ -281,6 +281,170 @@ function MyText({ foo }) {
 }
         `,
       },
+
+      {
+        code: `
+<View>
+  <Text>{'foo'}</Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  <Text>{foo + 'foo'}</Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  <Text><Trans>{'foo'}</Trans></Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {foo['bar'] && <Bar />}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {(foo === 'bar') && <Bar />}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {(foo !== 'bar') && <Bar />}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  <Text>{\`foo\`}</Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  <Text><Trans>{\`foo\`}</Trans></Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  <Text>{_(msg\`foo\`)}</Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  <Text><Trans>{_(msg\`foo\`)}</Trans></Text>
+</View>
+       `,
+      },
+
+      {
+        code: `
+<Foo>
+  <View prop={stuff('foo')}>
+    <Bar />
+  </View>
+</Foo>
+       `,
+      },
+
+      {
+        code: `
+<Foo>
+  <View onClick={() => stuff('foo')}>
+    <Bar />
+  </View>
+</Foo>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {renderItem('foo')}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {foo === 'foo' && <Bar />}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {foo['foo'] && <Bar />}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {check('foo') && <Bar />}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {foo.bar && <Bar />}
+</View>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  <Trans>{renderItem('foo')}</Trans>
+</Text>
+       `,
+      },
+
+      {
+        code: `
+<View>
+  {null}
+</View>
+       `,
+      },
+
+      {
+        code: `
+<Text>
+  <Trans>{null}</Trans>
+</Text>
+       `,
+      },
     ],
 
     invalid: [
@@ -455,6 +619,179 @@ function MyText({ foo }) {
         `,
         errors: 1,
       },
+
+      {
+        code: `
+<View>
+  {'foo'}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {foo && 'foo'}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{'foo'}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {foo && <Trans>{'foo'}</Trans>}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {10}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{10}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{foo + 10}</Trans>
+</View>
+             `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {\`foo\`}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{\`foo\`}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{foo + \`foo\`}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {_(msg\`foo\`)}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {foo + _(msg\`foo\`)}
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{_(msg\`foo\`)}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{foo + _(msg\`foo\`)}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>foo</Trans>
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans><Trans>foo</Trans></Trans>
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{foo}</Trans>
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>{'foo'}</Trans>
+</View>
+       `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View prop={
+  <Trans><Text>foo</Text></Trans>
+}>
+  <Bar />
+</View>
+        `,
+        errors: 1,
+      },
     ],
   }
 
diff --git a/eslint/avoid-unwrapped-text.js b/eslint/avoid-unwrapped-text.js
index 79d099f00..eef31f795 100644
--- a/eslint/avoid-unwrapped-text.js
+++ b/eslint/avoid-unwrapped-text.js
@@ -33,6 +33,7 @@ exports.create = function create(context) {
   const options = context.options[0] || {}
   const impliedTextProps = options.impliedTextProps ?? []
   const impliedTextComponents = options.impliedTextComponents ?? []
+  const suggestedTextWrappers = options.suggestedTextWrappers ?? {}
   const textProps = [...impliedTextProps]
   const textComponents = ['Text', ...impliedTextComponents]
 
@@ -54,13 +55,13 @@ exports.create = function create(context) {
             return
           }
           if (tagName === 'Trans') {
-            // Skip over it and check above.
+            // Exit and rely on the traversal for <Trans> JSXElement (code below).
             // TODO: Maybe validate that it's present.
-            parent = parent.parent
-            continue
+            return
           }
-          let message = 'Wrap this string in <Text>.'
-          if (tagName !== 'View') {
+          const suggestedWrapper = suggestedTextWrappers[tagName]
+          let message = `Wrap this string in <${suggestedWrapper ?? 'Text'}>.`
+          if (tagName !== 'View' && !suggestedWrapper) {
             message +=
               ' If <' +
               tagName +
@@ -112,6 +113,189 @@ exports.create = function create(context) {
         continue
       }
     },
+    Literal(node) {
+      if (typeof node.value !== 'string' && typeof node.value !== 'number') {
+        return
+      }
+      let parent = node.parent
+      while (parent) {
+        if (parent.type === 'JSXElement') {
+          const tagName = getTagName(parent)
+          if (isTextComponent(tagName)) {
+            // We're good.
+            return
+          }
+          if (tagName === 'Trans') {
+            // Exit and rely on the traversal for <Trans> JSXElement (code below).
+            // TODO: Maybe validate that it's present.
+            return
+          }
+          const suggestedWrapper = suggestedTextWrappers[tagName]
+          let message = `Wrap this string in <${suggestedWrapper ?? 'Text'}>.`
+          if (tagName !== 'View' && !suggestedWrapper) {
+            message +=
+              ' If <' +
+              tagName +
+              '> is guaranteed to render <Text>, ' +
+              'rename it to <' +
+              tagName +
+              'Text> or add it to impliedTextComponents.'
+          }
+          context.report({
+            node,
+            message,
+          })
+          return
+        }
+
+        if (parent.type === 'BinaryExpression' && parent.operator === '+') {
+          parent = parent.parent
+          continue
+        }
+
+        if (
+          parent.type === 'JSXExpressionContainer' ||
+          parent.type === 'LogicalExpression'
+        ) {
+          parent = parent.parent
+          continue
+        }
+
+        // Be conservative for other types.
+        return
+      }
+    },
+    TemplateLiteral(node) {
+      let parent = node.parent
+      while (parent) {
+        if (parent.type === 'JSXElement') {
+          const tagName = getTagName(parent)
+          if (isTextComponent(tagName)) {
+            // We're good.
+            return
+          }
+          if (tagName === 'Trans') {
+            // Exit and rely on the traversal for <Trans> JSXElement (code below).
+            // TODO: Maybe validate that it's present.
+            return
+          }
+          const suggestedWrapper = suggestedTextWrappers[tagName]
+          let message = `Wrap this string in <${suggestedWrapper ?? 'Text'}>.`
+          if (tagName !== 'View' && !suggestedWrapper) {
+            message +=
+              ' If <' +
+              tagName +
+              '> is guaranteed to render <Text>, ' +
+              'rename it to <' +
+              tagName +
+              'Text> or add it to impliedTextComponents.'
+          }
+          context.report({
+            node,
+            message,
+          })
+          return
+        }
+
+        if (
+          parent.type === 'CallExpression' &&
+          parent.callee.type === 'Identifier' &&
+          parent.callee.name === '_'
+        ) {
+          // This is a user-facing string, keep going up.
+          parent = parent.parent
+          continue
+        }
+
+        if (parent.type === 'BinaryExpression' && parent.operator === '+') {
+          parent = parent.parent
+          continue
+        }
+
+        if (
+          parent.type === 'JSXExpressionContainer' ||
+          parent.type === 'LogicalExpression' ||
+          parent.type === 'TaggedTemplateExpression'
+        ) {
+          parent = parent.parent
+          continue
+        }
+
+        // Be conservative for other types.
+        return
+      }
+    },
+    JSXElement(node) {
+      if (getTagName(node) !== 'Trans') {
+        return
+      }
+      let parent = node.parent
+      while (parent) {
+        if (parent.type === 'JSXElement') {
+          const tagName = getTagName(parent)
+          if (isTextComponent(tagName)) {
+            // We're good.
+            return
+          }
+          if (tagName === 'Trans') {
+            // Exit and rely on the traversal for this JSXElement.
+            // TODO: Should nested <Trans> even be allowed?
+            return
+          }
+          const suggestedWrapper = suggestedTextWrappers[tagName]
+          let message = `Wrap this <Trans> in <${suggestedWrapper ?? 'Text'}>.`
+          if (tagName !== 'View' && !suggestedWrapper) {
+            message +=
+              ' If <' +
+              tagName +
+              '> is guaranteed to render <Text>, ' +
+              'rename it to <' +
+              tagName +
+              'Text> or add it to impliedTextComponents.'
+          }
+          context.report({
+            node,
+            message,
+          })
+          return
+        }
+
+        if (
+          parent.type === 'JSXAttribute' &&
+          parent.name.type === 'JSXIdentifier' &&
+          parent.parent.type === 'JSXOpeningElement' &&
+          parent.parent.parent.type === 'JSXElement'
+        ) {
+          const tagName = getTagName(parent.parent.parent)
+          const propName = parent.name.name
+          if (
+            textProps.includes(tagName + ' ' + propName) ||
+            propName === 'text' ||
+            propName.endsWith('Text')
+          ) {
+            // We're good.
+            return
+          }
+          const message =
+            'Wrap this <Trans> in <Text>.' +
+            ' If `' +
+            propName +
+            '` is guaranteed to be wrapped in <Text>, ' +
+            'rename it to `' +
+            propName +
+            'Text' +
+            '` or add it to impliedTextProps.'
+          context.report({
+            node,
+            message,
+          })
+          return
+        }
+
+        parent = parent.parent
+        continue
+      }
+    },
     ReturnStatement(node) {
       let fnScope = context.getScope()
       while (fnScope && fnScope.type !== 'function') {