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
This commit is contained in:
dan 2024-04-05 15:09:35 +01:00 committed by GitHub
parent 49266c355e
commit 46c112edfd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 589 additions and 75 deletions

View file

@ -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,
},
],
}

View file

@ -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') {