diff options
author | dan <dan.abramov@gmail.com> | 2024-04-05 15:09:35 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-04-05 15:09:35 +0100 |
commit | 46c112edfdcb40681a8997ec4f47b413a08fdd14 (patch) | |
tree | 8745de3a743f9231a5151296c2df4fd6e39404e7 /eslint | |
parent | 49266c355ea781cbd7a0b373e64143da7740c91e (diff) | |
download | voidsky-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.js | 339 | ||||
-rw-r--r-- | eslint/avoid-unwrapped-text.js | 194 |
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') { |