From 569b6e14a62c1d480cf8a27680c430feea866e59 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Thu, 5 Jan 2023 14:43:33 -0800 Subject: [PATCH] Add new eslint plugin to check for valid i18n keys --- .eslint/rules/valid-i18n-keys.js | 183 ++++++++++++++++++ .eslint/rules/valid-i18n-keys.test.js | 116 +++++++++++ .eslintrc.js | 5 +- .github/workflows/ci.yml | 1 + _locales/en/messages.json | 4 + eslint-local-rules.js | 7 + package.json | 4 +- sticker-creator/app/stages/AppStage.tsx | 1 + ts/components/CallingHeader.tsx | 16 +- ts/components/ConversationList.tsx | 2 + ts/components/DisappearingTimerSelect.tsx | 8 +- ts/components/GroupV1MigrationDialog.tsx | 1 + ts/components/IncomingCallBar.tsx | 6 +- ts/components/Intl.stories.tsx | 3 +- ts/components/Intl.tsx | 1 + ts/components/LeftPaneSearchInput.tsx | 2 +- ts/components/ProfileEditor.tsx | 2 + ts/components/SafetyNumberViewer.tsx | 8 +- ts/components/ShortcutGuide.tsx | 1 + ts/components/StoriesSettingsModal.tsx | 1 + ts/components/StoryDetailsModal.tsx | 1 + ts/components/WhatsNewModal.tsx | 2 + .../conversation/DeliveryIssueDialog.tsx | 28 +-- .../conversation/GroupNotification.tsx | 44 +++-- ts/components/conversation/GroupV2Change.tsx | 1 + ts/components/conversation/MessageAudio.tsx | 1 + ts/components/conversation/MessageDetail.tsx | 1 + .../RemoveGroupMemberConfirmationDialog.tsx | 30 +-- .../conversation/SafetyNumberNotification.tsx | 1 + .../conversation/TimerNotification.tsx | 1 + .../conversation/UnsupportedMessage.tsx | 1 + .../conversation/VerificationNotification.tsx | 1 + .../ConversationDetails.tsx | 11 +- .../conversation-details/PendingInvites.tsx | 12 +- .../media-gallery/MediaGallery.tsx | 3 +- ts/test-both/types/setupI18n_test.ts | 1 + ts/util/expirationTimer.ts | 2 +- ts/util/timestamp.ts | 7 +- yarn.lock | 5 + 39 files changed, 447 insertions(+), 78 deletions(-) create mode 100644 .eslint/rules/valid-i18n-keys.js create mode 100644 .eslint/rules/valid-i18n-keys.test.js create mode 100644 eslint-local-rules.js diff --git a/.eslint/rules/valid-i18n-keys.js b/.eslint/rules/valid-i18n-keys.js new file mode 100644 index 000000000000..5d6f30659fab --- /dev/null +++ b/.eslint/rules/valid-i18n-keys.js @@ -0,0 +1,183 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +const crypto = require('crypto'); + +const messages = require('../../_locales/en/messages.json'); +const messageKeys = Object.keys(messages).sort((a, b) => { + return a.localeCompare(b); +}); + +const hashSum = crypto.createHash('sha256'); +hashSum.update(messageKeys.join('\n')); +const messagesCacheKey = hashSum.digest('hex'); + +function isI18nCall(node) { + return ( + node.type === 'CallExpression' && + node.callee.type === 'Identifier' && + node.callee.name === 'i18n' + ); +} + +function isIntlElement(node) { + return ( + node.type === 'JSXOpeningElement' && + node.name.type === 'JSXIdentifier' && + node.name.name === 'Intl' + ); +} + +function isStringLiteral(node) { + return node.type === 'Literal' && typeof node.value === 'string'; +} + +function valueToMessageKey(node) { + if (isStringLiteral(node)) { + return node.value; + } + + if (node.type !== 'TemplateLiteral') { + return null; + } + + if (node.quasis.length === 1) { + return node.quasis[0].value.cooked; + } + + const parts = node.quasis.map(element => { + return element.value.cooked; + }); + + return new RegExp(`^${parts.join('(.*)')}$`); +} + +function getI18nCallMessageKey(node) { + if (node.arguments.length < 1) { + return null; + } + + let arg1 = node.arguments[0]; + if (arg1 == null) { + return null; + } + + return valueToMessageKey(arg1); +} + +function getIntlElementMessageKey(node) { + let idAttribute = node.attributes.find(attribute => { + return ( + attribute.type === 'JSXAttribute' && + attribute.name.type === 'JSXIdentifier' && + attribute.name.name === 'id' + ); + }); + + if (idAttribute == null) { + return null; + } + + let value = idAttribute.value; + + if (value.type === 'JSXExpressionContainer') { + value = value.expression; + } + + return valueToMessageKey(value); +} + +function isValidMessageKey(key) { + if (typeof key === 'string') { + if (Object.hasOwn(messages, key)) { + return true; + } + } else if (key instanceof RegExp) { + if (messageKeys.some(k => key.test(k))) { + return true; + } + } + return false; +} + +module.exports = { + messagesCacheKey, + meta: { + type: 'problem', + hasSuggestions: false, + fixable: false, + schema: [ + { + type: 'object', + properties: { + messagesCacheKey: { + type: 'string', + }, + }, + required: ['messagesCacheKey'], + additionalProperties: false, + }, + ], + }, + create(context) { + const messagesCacheKeyOption = context.options[0].messagesCacheKey; + if (messagesCacheKeyOption !== messagesCacheKey) { + throw new Error( + `The cache key for the i18n rule does not match the current messages.json file (expected: ${messagesCacheKey}, received: ${messagesCacheKeyOption})` + ); + } + + return { + JSXOpeningElement(node) { + if (!isIntlElement(node)) { + return; + } + + let key = getIntlElementMessageKey(node); + + if (key == null) { + context.report({ + node, + message: + " must always be provided an 'id' attribute with a literal string", + }); + return; + } + + if (isValidMessageKey(key)) { + return; + } + + context.report({ + node, + message: ` id "${key}" not found in _locales/en/messages.json`, + }); + }, + CallExpression(node) { + if (!isI18nCall(node)) { + return; + } + + let key = getI18nCallMessageKey(node); + + if (key == null) { + context.report({ + node, + message: + "i18n()'s first argument should always be a literal string", + }); + return; + } + + if (isValidMessageKey(key)) { + return; + } + + context.report({ + node, + message: `i18n() key "${key}" not found in _locales/en/messages.json`, + }); + }, + }; + }, +}; diff --git a/.eslint/rules/valid-i18n-keys.test.js b/.eslint/rules/valid-i18n-keys.test.js new file mode 100644 index 000000000000..7b957d21670c --- /dev/null +++ b/.eslint/rules/valid-i18n-keys.test.js @@ -0,0 +1,116 @@ +// Copyright 2023 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +const rule = require('./valid-i18n-keys'); +const RuleTester = require('eslint').RuleTester; + +const messagesCacheKey = rule.messagesCacheKey; + +// Need to load so mocha doesn't complain about polluting the global namespace +require('@typescript-eslint/parser'); + +const ruleTester = new RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + jsx: true, + }, + }, +}); + +ruleTester.run('valid-i18n-keys', rule, { + valid: [ + { + code: `i18n("AddCaptionModal__title")`, + options: [{ messagesCacheKey }], + }, + { + code: 'i18n(`AddCaptionModal__${title}`)', + options: [{ messagesCacheKey }], + }, + { + code: `let jsx = `, + options: [{ messagesCacheKey }], + }, + { + code: `let jsx = `, + options: [{ messagesCacheKey }], + }, + { + code: 'let jsx = ', + options: [{ messagesCacheKey }], + }, + { + code: 'let jsx = ', + options: [{ messagesCacheKey }], + }, + ], + invalid: [ + { + code: `i18n("THIS_KEY_SHOULD_NEVER_EXIST")`, + options: [{ messagesCacheKey }], + errors: [ + { + message: + 'i18n() key "THIS_KEY_SHOULD_NEVER_EXIST" not found in _locales/en/messages.json', + type: 'CallExpression', + }, + ], + }, + { + code: `i18n(cond ? "AddCaptionModal__title" : "AddCaptionModal__title")`, + options: [{ messagesCacheKey }], + errors: [ + { + message: "i18n()'s first argument should always be a literal string", + type: 'CallExpression', + }, + ], + }, + { + code: `i18n(42)`, + options: [{ messagesCacheKey }], + errors: [ + { + message: "i18n()'s first argument should always be a literal string", + type: 'CallExpression', + }, + ], + }, + { + code: `let jsx = `, + options: [{ messagesCacheKey }], + errors: [ + { + message: + ' id "THIS_KEY_SHOULD_NEVER_EXIST" not found in _locales/en/messages.json', + type: 'JSXOpeningElement', + }, + ], + }, + { + code: `let jsx = `, + options: [{ messagesCacheKey }], + errors: [ + { + message: + " must always be provided an 'id' attribute with a literal string", + type: 'JSXOpeningElement', + }, + ], + }, + { + code: `let jsx = `, + options: [{ messagesCacheKey }], + errors: [ + { + message: + " must always be provided an 'id' attribute with a literal string", + type: 'JSXOpeningElement', + }, + ], + }, + ], +}); diff --git a/.eslintrc.js b/.eslintrc.js index 16b743779db4..0bfbdbf56236 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,5 +1,6 @@ // Copyright 2018 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +const { messagesCacheKey } = require('./.eslint/rules/valid-i18n-keys'); // For reference: https://github.com/airbnb/javascript @@ -215,6 +216,8 @@ const typescriptRules = { // TODO: DESKTOP-4655 'import/no-cycle': 'off', + + 'local-rules/valid-i18n-keys': ['error', { messagesCacheKey }], }; module.exports = { @@ -228,7 +231,7 @@ module.exports = { extends: ['airbnb-base', 'prettier'], - plugins: ['mocha', 'more'], + plugins: ['mocha', 'more', 'local-rules'], overrides: [ { diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5c0dea0eb8a5..079047ca6bca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,6 +81,7 @@ jobs: - run: yarn test-release env: NODE_ENV: production + - run: yarn test-eslint linux: needs: lint diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 05a27fdec4ac..de3a56f5e5ce 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -2276,6 +2276,10 @@ "message": "This message was deleted.", "description": "Shown in a message's bubble when the message has been deleted for everyone." }, + "giftBadge--missing": { + "message": "Unable to fetch gift badge details", + "description": "Aria label for gift badge when we can't fetch the details" + }, "message--giftBadge--unopened--incoming": { "message": "View this message on mobile to open it", "description": "Shown in a message's bubble when you've received a gift badge from a contact" diff --git a/eslint-local-rules.js b/eslint-local-rules.js new file mode 100644 index 000000000000..ca387946eddd --- /dev/null +++ b/eslint-local-rules.js @@ -0,0 +1,7 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only +/* eslint-disable global-require */ + +module.exports = { + 'valid-i18n-keys': require('./.eslint/rules/valid-i18n-keys'), +}; diff --git a/package.json b/package.json index 92158f077ddd..5757495cc4c9 100644 --- a/package.json +++ b/package.json @@ -34,11 +34,12 @@ "prepare-staging-build": "node scripts/prepare_staging_build.js", "prepare-windows-cert": "node scripts/prepare_windows_cert.js", "publish-to-apt": "NAME=$npm_package_name VERSION=$npm_package_version ./aptly.sh", - "test": "yarn test-node && yarn test-electron && yarn test-lint-intl", + "test": "yarn test-node && yarn test-electron && yarn test-lint-intl && yarn test-eslint", "test-electron": "node ts/scripts/test-electron.js", "test-release": "node ts/scripts/test-release.js", "test-node": "electron-mocha --timeout 10000 --file test/setup-test-node.js --recursive test/modules ts/test-node ts/test-both", "test-mock": "mocha ts/test-mock/**/*_test.js", + "test-eslint": "mocha .eslint/rules/**/*.test.js", "test-node-coverage": "nyc --reporter=lcov --reporter=text mocha --recursive test/modules ts/test-node ts/test-both", "test-lint-intl": "ts-node ./build/intl-linter/linter.ts --test", "eslint": "eslint --cache . --max-warnings 0", @@ -276,6 +277,7 @@ "eslint-config-airbnb-typescript-prettier": "5.0.0", "eslint-config-prettier": "8.5.0", "eslint-plugin-import": "2.26.0", + "eslint-plugin-local-rules": "1.3.2", "eslint-plugin-mocha": "10.1.0", "eslint-plugin-more": "1.0.5", "eslint-plugin-react": "7.31.10", diff --git a/sticker-creator/app/stages/AppStage.tsx b/sticker-creator/app/stages/AppStage.tsx index cde18961bbf7..642e7ea9aaae 100644 --- a/sticker-creator/app/stages/AppStage.tsx +++ b/sticker-creator/app/stages/AppStage.tsx @@ -95,6 +95,7 @@ export function AppStage(props: Props): JSX.Element { className={styles.toaster} loaf={toasts.map((slice, id) => ({ id, + // eslint-disable-next-line local-rules/valid-i18n-keys text: i18n(slice.key, slice.subs), }))} onDismiss={dismissToast} diff --git a/ts/components/CallingHeader.tsx b/ts/components/CallingHeader.tsx index 3868e6047c4d..39bbe964902a 100644 --- a/ts/components/CallingHeader.tsx +++ b/ts/components/CallingHeader.tsx @@ -91,19 +91,19 @@ export function CallingHeader({ {isGroupCall && participantCount > 2 && toggleSpeakerView && (
); diff --git a/ts/components/DisappearingTimerSelect.tsx b/ts/components/DisappearingTimerSelect.tsx index 08711d987f54..f8e899ba7261 100644 --- a/ts/components/DisappearingTimerSelect.tsx +++ b/ts/components/DisappearingTimerSelect.tsx @@ -65,11 +65,9 @@ export function DisappearingTimerSelect(props: Props): JSX.Element { ...expirationTimerOptions, { value: DurationInSeconds.fromSeconds(-1), - text: i18n( - isCustomTimeSelected - ? 'selectedCustomDisappearingTimeOption' - : 'customDisappearingTimeOption' - ), + text: isCustomTimeSelected + ? i18n('selectedCustomDisappearingTimeOption') + : i18n('customDisappearingTimeOption'), }, ]; diff --git a/ts/components/GroupV1MigrationDialog.tsx b/ts/components/GroupV1MigrationDialog.tsx index a85cbeffb799..8f6d77506f71 100644 --- a/ts/components/GroupV1MigrationDialog.tsx +++ b/ts/components/GroupV1MigrationDialog.tsx @@ -154,6 +154,7 @@ function renderMembers({ return ( <> + {/* eslint-disable-next-line local-rules/valid-i18n-keys */} {i18n(key)} ; - messageNode = i18n( - isVideoCall ? 'incomingVideoCall' : 'incomingAudioCall' - ); + messageNode = isVideoCall + ? i18n('incomingVideoCall') + : i18n('incomingAudioCall'); break; case CallMode.Group: { const { otherMembersRung, ringer } = props; diff --git a/ts/components/Intl.stories.tsx b/ts/components/Intl.stories.tsx index cf3cbba3d212..e7a79249a9f0 100644 --- a/ts/components/Intl.stories.tsx +++ b/ts/components/Intl.stories.tsx @@ -23,7 +23,8 @@ const createProps = (overrideProps: Partial = {}): Props => ({ renderText: overrideProps.renderText, }); -// eslint-disable-next-line react/function-component-definition +// eslint-disable-next-line max-len +// eslint-disable-next-line react/function-component-definition, local-rules/valid-i18n-keys const Template: Story = args => ; export const NoReplacements = Template.bind({}); diff --git a/ts/components/Intl.tsx b/ts/components/Intl.tsx index 23953a1405f0..410bd56d1009 100644 --- a/ts/components/Intl.tsx +++ b/ts/components/Intl.tsx @@ -88,6 +88,7 @@ export class Intl extends React.Component { return intl.formatMessage({ id }, components); } + // eslint-disable-next-line local-rules/valid-i18n-keys const text = i18n(id); const results: Array< string | JSX.Element | Array | null diff --git a/ts/components/LeftPaneSearchInput.tsx b/ts/components/LeftPaneSearchInput.tsx index 4cfb82c73cb8..209b9259d96c 100644 --- a/ts/components/LeftPaneSearchInput.tsx +++ b/ts/components/LeftPaneSearchInput.tsx @@ -87,7 +87,7 @@ export function LeftPaneSearchInput({ inputRef.current?.focus(); }; - const label = i18n(searchConversation ? 'searchIn' : 'search'); + const label = searchConversation ? i18n('searchIn') : i18n('search'); return ( } + // eslint-disable-next-line local-rules/valid-i18n-keys label={i18n(defaultBio.i18nLabel)} onClick={() => { const emojiData = getEmojiData(defaultBio.shortName, skinTone); @@ -425,6 +426,7 @@ export function ProfileEditor({ setStagedProfile(profileData => ({ ...profileData, aboutEmoji: unifiedToEmoji(emojiData.unified), + // eslint-disable-next-line local-rules/valid-i18n-keys aboutText: i18n(defaultBio.i18nLabel), })); }} diff --git a/ts/components/SafetyNumberViewer.tsx b/ts/components/SafetyNumberViewer.tsx index 935d6b4f74f1..884b92362b93 100644 --- a/ts/components/SafetyNumberViewer.tsx +++ b/ts/components/SafetyNumberViewer.tsx @@ -64,7 +64,6 @@ export function SafetyNumberViewer({ ); const { isVerified } = contact; - const verifiedStatusKey = isVerified ? 'isVerified' : 'isNotVerified'; const verifyButtonText = isVerified ? i18n('unverify') : i18n('verify'); return ( @@ -79,7 +78,12 @@ export function SafetyNumberViewer({ ) : ( )} - + {} + {isVerified ? ( + + ) : ( + + )}
); diff --git a/ts/components/conversation/conversation-details/PendingInvites.tsx b/ts/components/conversation/conversation-details/PendingInvites.tsx index 2e5b07fe4687..47a05999a9d9 100644 --- a/ts/components/conversation/conversation-details/PendingInvites.tsx +++ b/ts/components/conversation/conversation-details/PendingInvites.tsx @@ -285,14 +285,12 @@ function getConfirmationMessage({ // Requesting a membership since they weren't added by anyone if (membershipType === StageType.DENY_REQUEST) { - const key = isAccessControlEnabled( - conversation.accessControlAddFromInviteLink - ) - ? 'PendingRequests--deny-for--with-link' - : 'PendingRequests--deny-for'; - return i18n(key, { + const params = { name: firstMembership.member.title, - }); + }; + return isAccessControlEnabled(conversation.accessControlAddFromInviteLink) + ? i18n('PendingRequests--deny-for--with-link', params) + : i18n('PendingRequests--deny-for', params); } if (membershipType === StageType.APPROVE_REQUEST) { diff --git a/ts/components/conversation/media-gallery/MediaGallery.tsx b/ts/components/conversation/media-gallery/MediaGallery.tsx index 29e015ad37c3..15b913ad9942 100644 --- a/ts/components/conversation/media-gallery/MediaGallery.tsx +++ b/ts/components/conversation/media-gallery/MediaGallery.tsx @@ -74,7 +74,8 @@ function MediaSection({ const header = section.type === 'yearMonth' ? date.format(MONTH_FORMAT) - : i18n(section.type); + : // eslint-disable-next-line local-rules/valid-i18n-keys + i18n(section.type); return ( { describe('i18n', () => { it('returns empty string for unknown string', () => { + // eslint-disable-next-line local-rules/valid-i18n-keys assert.strictEqual(i18n('random'), ''); }); it('returns message for given string', () => { diff --git a/ts/util/expirationTimer.ts b/ts/util/expirationTimer.ts index 593050e3701e..c6313900d855 100644 --- a/ts/util/expirationTimer.ts +++ b/ts/util/expirationTimer.ts @@ -35,7 +35,7 @@ export function format( ): string { let seconds = Math.abs(dirtySeconds || 0); if (!seconds) { - return i18n(capitalizeOff ? 'off' : 'disappearingMessages__off'); + return capitalizeOff ? i18n('off') : i18n('disappearingMessages__off'); } seconds = Math.max(Math.floor(seconds), 1); diff --git a/ts/util/timestamp.ts b/ts/util/timestamp.ts index f1ffc8aed634..c95d36114055 100644 --- a/ts/util/timestamp.ts +++ b/ts/util/timestamp.ts @@ -141,11 +141,10 @@ export function formatDate( const m = moment(rawTimestamp); - const formatI18nKey = + const rawFormatString = Math.abs(m.diff(Date.now())) < 6 * MONTH - ? 'TimelineDateHeader--date-in-last-6-months' - : 'TimelineDateHeader--date-older-than-6-months'; - const rawFormatString = i18n(formatI18nKey); + ? i18n('TimelineDateHeader--date-in-last-6-months') + : i18n('TimelineDateHeader--date-older-than-6-months'); const formatString = sanitizeFormatString(rawFormatString, 'LL'); return m.format(formatString); diff --git a/yarn.lock b/yarn.lock index 0254b2c91867..f6e13b6c3c5d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8262,6 +8262,11 @@ eslint-plugin-jsx-a11y@^6.5.1: minimatch "^3.1.2" semver "^6.3.0" +eslint-plugin-local-rules@1.3.2: + version "1.3.2" + resolved "https://registry.yarnpkg.com/eslint-plugin-local-rules/-/eslint-plugin-local-rules-1.3.2.tgz#b9c9522915faeb9e430309fb909fc1dbcd7aedb3" + integrity sha512-X4ziX+cjlCYnZa+GB1ly3mmj44v2PeIld3tQVAxelY6AMrhHSjz6zsgsT6nt0+X5b7eZnvL/O7Q3pSSK2kF/+Q== + eslint-plugin-mocha@10.1.0: version "10.1.0" resolved "https://registry.yarnpkg.com/eslint-plugin-mocha/-/eslint-plugin-mocha-10.1.0.tgz#69325414f875be87fb2cb00b2ef33168d4eb7c8d"