From a47d7d55afb181b9221a34a5d93353a31e1117f9 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Aug 2022 12:48:33 -0700 Subject: [PATCH] tools: Add check-messages-en, to find dangling messages in messages_en.json As discussed on CZO: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Strings.20in.20messages_en.2Ejson.20but.20not.20in.20the.20app/near/1425641 Run with: tools/check-messages-en This gives just one result, which we'll address next. We add flow-parser at <0.159.0. We'd like a later version -- probably ideal would be to sync it with flow-bin, which is ^0.170.0 right now -- but facebook/flow@5de4ea57e, released in v0.159.0, was a change that isn't yet handled by any version of ast-types [1]. A fix for ast-types is open as the issue benjamn/ast-types#728 and PR benjamn/ast-types#727. [1] I get this error output: /Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/types.js:669 throw new Error("did not recognize object of type " + ^ Error: did not recognize object of type "PropertyDefinition" at Object.getFieldNames (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/types.js:669:19) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:185:36) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:167:20) at NodePath.each (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path.js:88:26) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:179:18) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:167:20) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:204:25) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:167:20) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:204:25) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:167:20) --- package.json | 3 + tools/check-messages-en | 181 ++++++++++++++++++++++++++++++++++++++++ yarn.lock | 79 +++++++++++++++++- 3 files changed, 259 insertions(+), 4 deletions(-) create mode 100755 tools/check-messages-en diff --git a/package.json b/package.json index e4b7426abba..cf0ee736482 100644 --- a/package.json +++ b/package.json @@ -92,6 +92,7 @@ "@rollup/plugin-node-resolve": "^13.0.4", "@types/react-native": "~0.67.6", "@vusion/webfonts-generator": "^0.8.0", + "ast-types": "^0.16.1", "babel-plugin-transform-flow-enums": "^0.0.2", "core-js": "^3.1.4", "deep-freeze": "^0.0.1", @@ -107,6 +108,7 @@ "eslint-plugin-react-hooks": "^4.5.0", "flow-bin": "^0.170.0", "flow-coverage-report": "^0.8.0", + "flow-parser": "<0.159.0", "flow-typed": "^3.3.1", "hermes-eslint": "^0.9.0", "immutable-devtools": "^0.1.5", @@ -124,6 +126,7 @@ "prettier-eslint-cli": "^6.0.1", "react-native-cli": "^2.0.1", "react-test-renderer": "17.0.2", + "recast": "^0.22.0", "redux-mock-store": "^1.5.1", "rollup": "^2.26.5", "sqlite3": "^5.0.2", diff --git a/tools/check-messages-en b/tools/check-messages-en new file mode 100755 index 00000000000..ed32f3c6bef --- /dev/null +++ b/tools/check-messages-en @@ -0,0 +1,181 @@ +#!/usr/bin/env node + +// TODO: Type-check this file + +// Sadly our auto-format doesn't seem to run eslint on this file; give in to +// Prettier here. +// TODO: debug +/* eslint-disable operator-linebreak */ + +const fs = require('fs'); +const path = require('path'); +const { namedTypes: n, visit } = require('ast-types'); +const flowParser = require('flow-parser'); +const { parse } = require('recast'); + +const messages_en = require('../static/translations/messages_en.json'); + +/** + * Make a list of files that might contain UI strings, by recursing in src/. + */ +function getPossibleUiStringFilePaths() { + const result = []; + const kSrcDirName = 'src/'; + function walk(dir, _dirName = '') { + let dirent; + // eslint-disable-next-line no-cond-assign + while ((dirent = dir.readSync())) { + // To reduce false negatives, `continue` when nothing in `dirent` can + // cause UI strings to appear in the app. + + if (dirent.isFile()) { + if (!dirent.name.endsWith('.js')) { + // Non-JS code, and Flow type definitions in .js.flow files. + continue; + } + + result.push(path.join(kSrcDirName, _dirName, dirent.name)); + } else if (dirent.isDirectory()) { + const subdirName = path.join(_dirName, dirent.name); + + // e.g., …/__tests__ and …/__flow-tests__ + if (subdirName.endsWith('tests__')) { + // Test code. + continue; + } + + walk(fs.opendirSync(path.join(kSrcDirName, subdirName)), subdirName); + } else { + // Something we don't expect to find under src/, probably containing + // no UI strings. (symlinks? fifos, sockets, devices??) + continue; + } + } + } + walk(fs.opendirSync(kSrcDirName)); + return result; +} + +const parseOptions = { + parser: { + parse(src) { + return flowParser.parse(src, { + // Comments can't cause UI strings to appear in the app; ignore them. + all_comments: false, + comments: false, + + // We use Flow enums; the parser shouldn't crash on them. + enums: true, + + // Set `tokens: true` just to work around a mysterious error. + // + // From the doc for this option: + // + // > include a list of all parsed tokens in a top-level tokens + // > property + // + // We don't actually want this list of tokens. String literals do + // get represented in the list, but as tokens, i.e., meaningful + // chunks of the literal source code. They come with surrounding + // quotes, escape syntax, etc: + // + // 'doesn\'t' + // "doesn't" + // + // What we really want is the *value* of a string literal: + // + // doesn't + // + // and we get that from the AST. + // + // Anyway, we set `true` for this because otherwise I've been seeing + // `parse` throw an error: + // + // Error: Line 72: Invalid regular expression: missing / + // + // TODO: Debug and/or file an issue upstream. + tokens: true, + }); + }, + }, +}; + +/** + * Look at all given files and collect all strings that might be UI strings. + * + * The result will include non-UI strings because we can't realistically + * filter them all out. That means, when the caller checks messages_en + * against these strings, it could get false negatives: perhaps messages_en + * has a string 'message-empty' (why would it, though), and that string + * won't be flagged because it appears as an enum value in ComposeBox. + * + * To reduce these false negatives, we filter out low-hanging fruit, like + * the string 'foo' in `import Foo from 'foo'`. + */ +function getPossibleUiStrings(possibleUiStringFilePaths) { + const result = new Set(); + possibleUiStringFilePaths.forEach(filePath => { + const source = fs.readFileSync(filePath).toString(); + const ast = parse(source, parseOptions); + + visit(ast, { + // Find nodes with type "Literal" in the AST. + /* eslint-disable no-shadow */ + visitLiteral(path) { + const { value } = path.value; + + if ( + // (Non-string literals include numbers, booleans, etc.) + typeof value === 'string' && + // This string isn't like 'react' in `import React from 'react'`. + !n.ImportDeclaration.check(path.parent.value) + ) { + result.add(value); + } + + // A literal is always a leaf, right? No need to call this.traverse. + return false; + }, + + // Find nodes with type "TemplateLiteral" in the AST. We sometimes use + // template literals in UI strings for readability. + /* eslint-disable no-shadow */ + visitTemplateLiteral(path) { + if ( + // Translatable UI strings are unlikely to contain + // sub-expressions. + // + // Also, if a template literal has nontrivial sub-expressions, we + // can't reasonably interpret them here anyway. + path.value.quasis.length === 1 && + path.value.expressions.length === 0 + ) { + result.add(path.value.quasis[0].value.cooked); + } + + return this.traverse(path); + }, + }); + }); + return result; +} + +function main() { + const possibleUiStrings = getPossibleUiStrings(getPossibleUiStringFilePaths()); + + // Check each key ("message ID" in formatjs's lingo) against + // possibleUiStrings, and make a list of any that aren't found. + const danglingMessageIds = Object.keys(messages_en).filter( + messageId => !possibleUiStrings.has(messageId), + ); + + if (danglingMessageIds.length > 0) { + console.error( + "Found message IDs in static/translations/messages_en.json that don't seem to be used in the app:", + ); + console.error(danglingMessageIds); + process.exit(1); + } +} + +main(); diff --git a/yarn.lock b/yarn.lock index f736d0e60cb..761f53a35af 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3199,6 +3199,16 @@ asap@~2.0.3, asap@~2.0.6: resolved "https://registry.yarnpkg.com/asap/-/asap-2.0.6.tgz#e50347611d7e690943208bbdafebcbc2fb866d46" integrity sha512-BSHWgDSAiKs50o2Re8ppvp3seVHXSRM44cdSsT9FfNEUUZLOGWVCsiWaRPWM1Znn+mqZ1OfVZ3z3DWEzSp7hRA== +assert@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/assert/-/assert-2.0.0.tgz#95fc1c616d48713510680f2eaf2d10dd22e02d32" + integrity sha512-se5Cd+js9dXJnu6Ag2JFc00t+HmHOen+8Q+L7O9zI0PqQXr20uk2J0XQqMxZEeo5U50o8Nvmmx7dZrl+Ufr35A== + dependencies: + es6-object-assign "^1.1.0" + is-nan "^1.2.1" + object-is "^1.0.1" + util "^0.12.0" + assign-symbols@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/assign-symbols/-/assign-symbols-1.0.0.tgz#59667f41fadd4f20ccbc2bb96b8d4f7f78ec0367" @@ -3216,6 +3226,20 @@ ast-types@0.14.2: dependencies: tslib "^2.0.1" +ast-types@0.15.2: + version "0.15.2" + resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.15.2.tgz#39ae4809393c4b16df751ee563411423e85fb49d" + integrity sha512-c27loCv9QkZinsa5ProX751khO9DJl/AcB5c2KNtA6NRvHKS0PgLfcftz72KVq504vB0Gku5s2kUZzDBvQWvHg== + dependencies: + tslib "^2.0.1" + +ast-types@^0.16.1: + version "0.16.1" + resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.16.1.tgz#7a9da1617c9081bc121faafe91711b4c8bb81da2" + integrity sha512-6t10qk83GOG8p0vKmaCr8eiilZwO171AvbROMtvvNiwrTly62t+7XkA8RdIIVbpMhCASAsxgAzdRSwh6nw/5Dg== + dependencies: + tslib "^2.0.1" + "ast-types@npm:@gregprice/ast-types@^0.15.3-0.tsflower.5": version "0.15.3-0.tsflower.5" resolved "https://registry.yarnpkg.com/@gregprice/ast-types/-/ast-types-0.15.3-0.tsflower.5.tgz#59bfaf5b776dc8bf3e9de7d94d5a9546f3a1b6df" @@ -4934,6 +4958,11 @@ es-to-primitive@^1.2.1: is-date-object "^1.0.1" is-symbol "^1.0.2" +es6-object-assign@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/es6-object-assign/-/es6-object-assign-1.1.0.tgz#c2c3582656247c39ea107cb1e6652b6f9f24523c" + integrity sha512-MEl9uirslVwqQU369iHNWZXsI8yaZYGg/D65aOgZkeyFJwHYSxilf7rQzXKI7DdDuBPrBXbfk3sl9hJhmd5AUw== + escalade@^3.1.1: version "3.1.1" resolved "https://registry.yarnpkg.com/escalade/-/escalade-3.1.1.tgz#d8cfdc7000965c5a0174b4a82eaa5c0552742e40" @@ -5823,6 +5852,11 @@ flow-parser@0.*: resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.192.0.tgz#e2aa03e0c6a844c4d6ccdb4af2bc83cc589d9c8c" integrity sha512-FLyei0ikf4ab9xlg+05WNmdpOODiH9XVBuw7iI9OZyjIo+cX2L2OUPTovjbWLYLlI41oGTcprbKdB/f9XwBnKw== +flow-parser@<0.159.0: + version "0.158.0" + resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.158.0.tgz#d845f167c722babe880110fc3681c44f21823399" + integrity sha512-0hMsPkBTRrkII/0YiG9ehOxFXy4gOWdk8RSRze5WbfeKAQpL5kC2K4BmumyTfU9o5gr7/llgElF3UpSSrjzQAA== + flow-parser@^0.121.0: version "0.121.0" resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.121.0.tgz#9f9898eaec91a9f7c323e9e992d81ab5c58e618f" @@ -6741,7 +6775,7 @@ is-accessor-descriptor@^1.0.0: dependencies: kind-of "^6.0.0" -is-arguments@^1.1.0, is-arguments@^1.1.1: +is-arguments@^1.0.4, is-arguments@^1.1.0, is-arguments@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/is-arguments/-/is-arguments-1.1.1.tgz#15b3f88fda01f2a97fec84ca761a560f123efa9b" integrity sha512-8Q7EARjzEnKpt/PCD7e1cgUS0a6X8u5tdSiMqXhojOdoV9TsMsiO+9VLC5vAmO8N7/GmXn7yjR8qnA6bVAEzfA== @@ -6898,6 +6932,13 @@ is-generator-fn@^2.0.0: resolved "https://registry.yarnpkg.com/is-generator-fn/-/is-generator-fn-2.1.0.tgz#7d140adc389aaf3011a8f2a2a4cfa6faadffb118" integrity sha512-cTIB4yPYL/Grw0EaSzASzg6bBy9gqCofvWN8okThAYIxKJZC+udlRAmGbM0XLeniEJSs8uEgHPGuHSe1XsOLSQ== +is-generator-function@^1.0.7: + version "1.0.10" + resolved "https://registry.yarnpkg.com/is-generator-function/-/is-generator-function-1.0.10.tgz#f1558baf1ac17e0deea7c0415c438351ff2b3c72" + integrity sha512-jsEjy9l3yiXEQ+PsXdmBwEPcOxaXWLspKdplFUVI9vq1iZgIekeC0L167qeu86czQaxed3q/Uzuw0swL0irL8A== + dependencies: + has-tostringtag "^1.0.0" + is-glob@^2.0.0: version "2.0.1" resolved "https://registry.yarnpkg.com/is-glob/-/is-glob-2.0.1.tgz#d096f926a3ded5600f3fdfd91198cb0888c2d863" @@ -6939,6 +6980,14 @@ is-module@^1.0.0: resolved "https://registry.yarnpkg.com/is-module/-/is-module-1.0.0.tgz#3258fb69f78c14d5b815d664336b4cffb6441591" integrity sha512-51ypPSPCoTEIN9dy5Oy+h4pShgJmPCygKfyRCISBI+JoWT/2oJvK8QPxmwv7b/p239jXrm9M1mlQbyKJ5A152g== +is-nan@^1.2.1: + version "1.3.2" + resolved "https://registry.yarnpkg.com/is-nan/-/is-nan-1.3.2.tgz#043a54adea31748b55b6cd4e09aadafa69bd9e1d" + integrity sha512-E+zBKpQ2t6MEo1VsonYmluk9NxGrbzpeeLC2xIViuO2EjU2xsXsBPwTr3Ykv9l08UYEVEdWeRZNouaZqF6RN0w== + dependencies: + call-bind "^1.0.0" + define-properties "^1.1.3" + is-negative-zero@^2.0.2: version "2.0.2" resolved "https://registry.yarnpkg.com/is-negative-zero/-/is-negative-zero-2.0.2.tgz#7bf6f03a28003b8b3965de3ac26f664d765f3150" @@ -7049,7 +7098,7 @@ is-symbol@^1.0.2, is-symbol@^1.0.3: dependencies: has-symbols "^1.0.2" -is-typed-array@^1.1.10: +is-typed-array@^1.1.10, is-typed-array@^1.1.3: version "1.1.10" resolved "https://registry.yarnpkg.com/is-typed-array/-/is-typed-array-1.1.10.tgz#36a5b5cb4189b575d1a3e4b08536bfb485801e3f" integrity sha512-PJqgEHiWZvMpaFZ3uTc8kHPM4+4ADTlDniuQL7cU/UDA0Ql7F70yGfHph3cLNe+c9toaigv+DFzTJKhc2CtO6A== @@ -9379,7 +9428,7 @@ object-inspect@^1.12.2, object-inspect@^1.9.0: resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.12.2.tgz#c0641f26394532f28ab8d796ab954e43c009a8ea" integrity sha512-z+cPxW0QGUp0mcqcsgQyLVRDoXFQbXOwBaqyF7VIgI4TWNQsDHrBpUQslRmIfAoYWdYzs6UlKJtB2XJpTaNSpQ== -object-is@^1.1.5: +object-is@^1.0.1, object-is@^1.1.5: version "1.1.5" resolved "https://registry.yarnpkg.com/object-is/-/object-is-1.1.5.tgz#b9deeaa5fc7f1846a0faecdceec138e5778f53ac" integrity sha512-3cyDsyHgtmi7I7DfSSI2LDp6SK2lwvtbg0p0R1e0RvTqF5ceGx+K2dfSjm1bKDMVCFEDAQvy+o8c6a7VujOddw== @@ -10453,6 +10502,17 @@ recast@^0.20.4: source-map "~0.6.1" tslib "^2.0.1" +recast@^0.22.0: + version "0.22.0" + resolved "https://registry.yarnpkg.com/recast/-/recast-0.22.0.tgz#1dd3bf1b86e5eb810b044221a1a734234ed3e9c0" + integrity sha512-5AAx+mujtXijsEavc5lWXBPQqrM4+Dl5qNH96N2aNeuJFUzpiiToKPsxQD/zAIJHspz7zz0maX0PCtCTFVlixQ== + dependencies: + assert "^2.0.0" + ast-types "0.15.2" + esprima "~4.0.0" + source-map "~0.6.1" + tslib "^2.0.1" + "recast@npm:@gregprice/recast@^0.21.2-0.tsflower.8": version "0.21.2-0.tsflower.8" resolved "https://registry.yarnpkg.com/@gregprice/recast/-/recast-0.21.2-0.tsflower.8.tgz#576b7329a3cb6fcd1e406ebded7daf3cd5d9f573" @@ -12230,6 +12290,17 @@ util.promisify@~1.0.0: has-symbols "^1.0.1" object.getownpropertydescriptors "^2.1.0" +util@^0.12.0: + version "0.12.5" + resolved "https://registry.yarnpkg.com/util/-/util-0.12.5.tgz#5f17a6059b73db61a875668781a1c2b136bd6fbc" + integrity sha512-kZf/K6hEIrWHI6XqOFUiiMa+79wE/D8Q+NCNAWclkyg3b4d2k7s0QGepNjiABc+aR3N1PAyHL7p6UcLY6LmrnA== + dependencies: + inherits "^2.0.3" + is-arguments "^1.0.4" + is-generator-function "^1.0.7" + is-typed-array "^1.1.3" + which-typed-array "^1.1.2" + utile@0.2.x: version "0.2.1" resolved "https://registry.yarnpkg.com/utile/-/utile-0.2.1.tgz#930c88e99098d6220834c356cbd9a770522d90d7" @@ -12435,7 +12506,7 @@ which-module@^2.0.0: resolved "https://registry.yarnpkg.com/which-module/-/which-module-2.0.0.tgz#d9ef07dce77b9902b8a3a8fa4b31c3e3f7e6e87a" integrity sha512-B+enWhmw6cjfVC7kS8Pj9pCrKSc5txArRyaYGe088shv/FGWH+0Rjx/xPgtsWfsUtS27FkP697E4DDhgrgoc0Q== -which-typed-array@^1.1.8: +which-typed-array@^1.1.2, which-typed-array@^1.1.8: version "1.1.9" resolved "https://registry.yarnpkg.com/which-typed-array/-/which-typed-array-1.1.9.tgz#307cf898025848cf995e795e8423c7f337efbde6" integrity sha512-w9c4xkx6mPidwp7180ckYWfMmvxpjlZuIudNtDf4N/tTAUB8VJbX25qZoAsrtGuYNnGw3pa0AXgbGKRB8/EceA==