From 4cc57f4bfdf16fd627ad63bf20ff8193e7d0a12a Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 4 Apr 2024 17:32:50 +0100 Subject: Lint against strings without wrapping (#3398) * Add a rudimentary rule * Get the rule passing * Support special-casing text props * More tests --- eslint/__tests__/avoid-unwrapped-text.test.js | 423 ++++++++++++++++++++++++++ eslint/avoid-unwrapped-text.js | 111 +++++++ eslint/index.js | 7 + 3 files changed, 541 insertions(+) create mode 100644 eslint/__tests__/avoid-unwrapped-text.test.js create mode 100644 eslint/avoid-unwrapped-text.js create mode 100644 eslint/index.js (limited to 'eslint') diff --git a/eslint/__tests__/avoid-unwrapped-text.test.js b/eslint/__tests__/avoid-unwrapped-text.test.js new file mode 100644 index 000000000..0fbc01123 --- /dev/null +++ b/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: ` + + foo + + `, + }, + + { + code: ` + + + foo + + + `, + }, + + { + code: ` + + <> + foo + + + `, + }, + + { + code: ` + + {foo && foo} + + `, + }, + + { + code: ` + + {foo ? foo : bar} + + `, + }, + + { + code: ` + + + foo + + + `, + }, + + { + code: ` + + {foo && foo} + + `, + }, + + { + code: ` + + {foo ? foo : bar} + + `, + }, + + { + code: ` + + foo + + `, + }, + + { + code: ` + + + foo + + + `, + }, + + { + code: ` + + {bar} + + `, + }, + + { + code: ` + + {bar} + + `, + }, + + { + code: ` + + foo {bar} + + `, + }, + + { + code: ` + + + foo + + + `, + }, + + { + code: ` + + + {bar} + + + `, + }, + + { + code: ` + + + foo {bar} + + + `, + }, + + { + code: ` + + + foo + + + `, + }, + + { + code: ` +foo +}> + + + `, + }, + + { + code: ` +foo +}> + + + `, + }, + + { + code: ` +foo : bar +}> + + + `, + }, + + { + code: ` +foo +}> + + + `, + }, + + { + code: ` +foo +}> + + + `, + }, + + { + code: ` +foo +}> + + + `, + }, + + { + code: ` +foo +}> + + + `, + }, + + { + code: ` +foo : bar +}> + + + `, + }, + ], + + invalid: [ + { + code: ` + + `, + errors: 1, + }, + + { + code: ` + + foo + + `, + errors: 1, + }, + + { + code: ` + + <> + foo + + + `, + errors: 1, + }, + + { + code: ` + + + foo + + + `, + errors: 1, + }, + + { + code: ` + + {foo && foo} + + `, + errors: 1, + }, + + { + code: ` + + {foo ? foo : bar} + + `, + errors: 2, + }, + + { + code: ` + + + foo + + + `, + errors: 1, + }, + + { + code: ` + + foo {bar} + + `, + errors: 1, + }, + + { + code: ` + + + foo + + + `, + errors: 1, + }, + + { + code: ` + + + foo + + + `, + errors: 1, + }, + + { + code: ` +foo +}> + + + `, + errors: 1, + }, + + { + code: ` +foo +}> + + + `, + errors: 1, + }, + + { + code: ` +foo : bar +}> + + + `, + errors: 2, + }, + + { + code: ` +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) +}) diff --git a/eslint/avoid-unwrapped-text.js b/eslint/avoid-unwrapped-text.js new file mode 100644 index 000000000..c9e72386e --- /dev/null +++ b/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 .' + if (tagName !== 'View') { + message += + ' If <' + + tagName + + '> is guaranteed to render , ' + + '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 .' + + ' If `' + + propName + + '` is guaranteed to be wrapped in , ' + + 'rename it to `' + + propName + + 'Text' + + '` or add it to impliedTextProps.' + context.report({ + node, + message, + }) + return + } + + parent = parent.parent + continue + } + }, + } +} diff --git a/eslint/index.js b/eslint/index.js new file mode 100644 index 000000000..daf5bd81d --- /dev/null +++ b/eslint/index.js @@ -0,0 +1,7 @@ +'use strict' + +module.exports = { + rules: { + 'avoid-unwrapped-text': require('./avoid-unwrapped-text'), + }, +} -- cgit 1.4.1