Lint against strings without wrapping <Text> (#3398)

* Add a rudimentary rule

* Get the rule passing

* Support special-casing text props

* More tests

.eslintrc.js

@@ -1,3 +1,5 @@
+const bskyEslint = require('./eslint')
+
 module.exports = {
   root: true,
   extends: [
@@ -13,12 +15,43 @@ module.exports = {
     'react',
     'lingui',
     'simple-import-sort',
+    'bsky-internal',
   ],
   rules: {
     // Temporary until https://github.com/facebook/react-native/pull/43756 gets into a release.
     'prettier/prettier': 0,
     'react/no-unescaped-entities': 0,
     'react-native/no-inline-styles': 0,
+    'bsky-internal/avoid-unwrapped-text': [
+      'error',
+      {
+        impliedTextComponents: [
+          'Button', // TODO: Not always safe.
+          'ButtonText',
+          'DateField.Label',
+          'Description',
+          'H1',
+          'H2',
+          'H3',
+          'H4',
+          'H5',
+          'H6',
+          'InlineLink',
+          'Label',
+          'P',
+          'Prompt.Title',
+          'Prompt.Description',
+          'Prompt.Cancel', // TODO: Not always safe.
+          'Prompt.Action', // TODO: Not always safe.
+          'TextField.Label',
+          'TextField.Suffix',
+          'Title',
+          'Toggle.Label',
+          'ToggleButton.Button', // TODO: Not always safe.
+        ],
+        impliedTextProps: ['FormContainer title'],
+      },
+    ],
     'simple-import-sort/imports': [
       'warn',
       {

eslint/__tests__/avoid-unwrapped-text.test.js

@@ -0,0 +1,423 @@
+const {RuleTester} = require('eslint')
+const avoidUnwrappedText = require('../avoid-unwrapped-text')
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {
+    ecmaFeatures: {
+      jsx: true,
+    },
+    ecmaVersion: 6,
+    sourceType: 'module',
+  },
+})
+
+describe('avoid-unwrapped-text', () => {
+  const tests = {
+    valid: [
+      {
+        code: `
+<Text>
+  foo
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  <Trans>
+    foo
+  </Trans>
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  <>
+    foo
+  </>
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  {foo && <Trans>foo</Trans>}
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  {foo ? <Trans>foo</Trans> : <Trans>bar</Trans>}
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<Trans>
+  <Text>
+    foo
+  </Text>
+</Trans>
+        `,
+      },
+
+      {
+        code: `
+<Trans>
+  {foo && <Text>foo</Text>}
+</Trans>
+              `,
+      },
+
+      {
+        code: `
+<Trans>
+  {foo ? <Text>foo</Text> : <Text>bar</Text>}
+</Trans>
+              `,
+      },
+
+      {
+        code: `
+<CustomText>
+  foo
+</CustomText>
+        `,
+      },
+
+      {
+        code: `
+<CustomText>
+  <Trans>
+    foo
+  </Trans>
+</CustomText>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  {bar}
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<View>
+  {bar}
+</View>
+        `,
+      },
+
+      {
+        code: `
+<Text>
+  foo {bar}
+</Text>
+        `,
+      },
+
+      {
+        code: `
+<View>
+  <Text>
+    foo
+  </Text>
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View>
+  <Text>
+    {bar}
+  </Text>
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View>
+  <Text>
+    foo {bar}
+  </Text>
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View>
+  <CustomText>
+    foo
+  </CustomText>
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View prop={
+  <Text>foo</Text>
+}>
+  <Bar />
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View prop={
+  foo && <Text>foo</Text>
+}>
+  <Bar />
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View prop={
+  foo ? <Text>foo</Text> : <Text>bar</Text>
+}>
+  <Bar />
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View prop={
+  <Trans><Text>foo</Text></Trans>
+}>
+  <Bar />
+</View>
+        `,
+      },
+
+      {
+        code: `
+<View prop={
+  <Text><Trans>foo</Trans></Text>
+}>
+  <Bar />
+</View>
+        `,
+      },
+
+      {
+        code: `
+<Foo propText={
+  <Trans>foo</Trans>
+}>
+  <Bar />
+</Foo>
+        `,
+      },
+
+      {
+        code: `
+<Foo propText={
+  foo && <Trans>foo</Trans>
+}>
+  <Bar />
+</Foo>
+              `,
+      },
+
+      {
+        code: `
+<Foo propText={
+  foo ? <Trans>foo</Trans> : <Trans>bar</Trans>
+}>
+  <Bar />
+</Foo>
+              `,
+      },
+    ],
+
+    invalid: [
+      {
+        code: `
+<View> </View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  foo
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <>
+    foo
+  </>
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <Trans>
+    foo
+  </Trans>
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {foo && <Trans>foo</Trans>}
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  {foo ? <Trans>foo</Trans> : <Trans>bar</Trans>}
+</View>
+        `,
+        errors: 2,
+      },
+
+      {
+        code: `
+<Trans>
+  <View>
+    foo
+  </View>
+</Trans>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  foo {bar}
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<View>
+  <View>
+    foo
+  </View>
+</View>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<Text>
+  <View>
+    foo
+  </View>
+</Text>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<Text prop={
+  <View>foo</View>
+}>
+  <Bar />
+</Text>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<Text prop={
+  foo && <View>foo</View>
+}>
+  <Bar />
+</Text>
+        `,
+        errors: 1,
+      },
+
+      {
+        code: `
+<Text prop={
+  foo ? <View>foo</View> : <View>bar</View>
+}>
+  <Bar />
+</Text>
+        `,
+        errors: 2,
+      },
+
+      {
+        code: `
+<Foo prop={
+  <Trans>foo</Trans>
+}>
+  <Bar />
+</Foo>
+        `,
+        errors: 1,
+      },
+    ],
+  }
+
+  // For easier local testing
+  if (!process.env.CI) {
+    let only = []
+    let skipped = []
+    ;[...tests.valid, ...tests.invalid].forEach(t => {
+      if (t.skip) {
+        delete t.skip
+        skipped.push(t)
+      }
+      if (t.only) {
+        delete t.only
+        only.push(t)
+      }
+    })
+    const predicate = t => {
+      if (only.length > 0) {
+        return only.indexOf(t) !== -1
+      }
+      if (skipped.length > 0) {
+        return skipped.indexOf(t) === -1
+      }
+      return true
+    }
+    tests.valid = tests.valid.filter(predicate)
+    tests.invalid = tests.invalid.filter(predicate)
+  }
+  ruleTester.run('avoid-unwrapped-text', avoidUnwrappedText, tests)
+})

eslint/avoid-unwrapped-text.js

@@ -0,0 +1,111 @@
+'use strict'
+
+// Partially based on eslint-plugin-react-native.
+// Portions of code by Alex Zhukov, MIT license.
+
+function hasOnlyLineBreak(value) {
+  return /^[\r\n\t\f\v]+$/.test(value.replace(/ /g, ''))
+}
+
+function getTagName(node) {
+  const reversedIdentifiers = []
+  if (
+    node.type === 'JSXElement' &&
+    node.openingElement.type === 'JSXOpeningElement'
+  ) {
+    let object = node.openingElement.name
+    while (object.type === 'JSXMemberExpression') {
+      if (object.property.type === 'JSXIdentifier') {
+        reversedIdentifiers.push(object.property.name)
+      }
+      object = object.object
+    }
+
+    if (object.type === 'JSXIdentifier') {
+      reversedIdentifiers.push(object.name)
+    }
+  }
+
+  return reversedIdentifiers.reverse().join('.')
+}
+
+exports.create = function create(context) {
+  const options = context.options[0] || {}
+  const impliedTextProps = options.impliedTextProps ?? []
+  const impliedTextComponents = options.impliedTextComponents ?? []
+  const textProps = [...impliedTextProps]
+  const textComponents = ['Text', ...impliedTextComponents]
+  return {
+    JSXText(node) {
+      if (typeof node.value !== 'string' || hasOnlyLineBreak(node.value)) {
+        return
+      }
+      let parent = node.parent
+      while (parent) {
+        if (parent.type === 'JSXElement') {
+          const tagName = getTagName(parent)
+          if (textComponents.includes(tagName) || tagName.endsWith('Text')) {
+            // We're good.
+            return
+          }
+          if (tagName === 'Trans') {
+            // Skip over it and check above.
+            // TODO: Maybe validate that it's present.
+            parent = parent.parent
+            continue
+          }
+          let message = 'Wrap this string in <Text>.'
+          if (tagName !== 'View') {
+            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 string 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
+      }
+    },
+  }
+}

eslint/index.js

@@ -0,0 +1,7 @@
+'use strict'
+
+module.exports = {
+  rules: {
+    'avoid-unwrapped-text': require('./avoid-unwrapped-text'),
+  },
+}

package.json

@@ -236,6 +236,7 @@
     "babel-preset-expo": "^10.0.0",
     "detox": "^20.14.8",
     "eslint": "^8.19.0",
+    "eslint-plugin-bsky-internal": "link:./eslint",
     "eslint-plugin-detox": "^1.0.0",
     "eslint-plugin-ft-flow": "^2.0.3",
     "eslint-plugin-lingui": "^0.2.0",

yarn.lock

@@ -11404,6 +11404,10 @@ eslint-module-utils@^2.8.0:
   dependencies:
     debug "^3.2.7"
 
+"eslint-plugin-bsky-internal@link:./eslint":
+  version "0.0.0"
+  uid ""
+
 eslint-plugin-detox@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/eslint-plugin-detox/-/eslint-plugin-detox-1.0.0.tgz#2d9c0130e8ebc4ced56efb6eeaf0d0f5c163398d"