From 34921691b2be1fae46a05ea688791871080b94c3 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 26 Aug 2022 12:48:33 -0700 Subject: [PATCH] wip tool to find dangling message IDs in messages_en.json TODO: - Incorporate into tools/test, perhaps to run only when --all-files requested - Type-check this file with Flow? - Think of any more ways to keep non-UI strings out of possibleUiStringLiterals - See discussion: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/Strings.20in.20messages_en.2Ejson.20but.20not.20in.20the.20app/near/1425315 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.162.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:664 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:664:19) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:184:36) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:166:20) at NodePath.each (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path.js:87:26) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:178:18) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:166:20) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:203:25) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:166:20) at visitChildren (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:203:25) at Visitor.PVp.visitWithoutReset (/Users/chrisbobbe/dev/zulip-mobile/node_modules/ast-types/lib/path-visitor.js:166:20) --- package.json | 3 + tools/check-messages-en.js | 133 +++++++++++++++++++++++++++++++++++++ yarn.lock | 22 ++++++ 3 files changed, 158 insertions(+) create mode 100644 tools/check-messages-en.js diff --git a/package.json b/package.json index e4b7426abba..4d2ae2caeeb 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,7 @@ "@types/react-native": "~0.67.6", "@vusion/webfonts-generator": "^0.8.0", "babel-plugin-transform-flow-enums": "^0.0.2", + "ast-types": "^0.15.2", "core-js": "^3.1.4", "deep-freeze": "^0.0.1", "eslint": "^8.15.0", @@ -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.21.2", "redux-mock-store": "^1.5.1", "rollup": "^2.26.5", "sqlite3": "^5.0.2", diff --git a/tools/check-messages-en.js b/tools/check-messages-en.js new file mode 100644 index 00000000000..95f971ee780 --- /dev/null +++ b/tools/check-messages-en.js @@ -0,0 +1,133 @@ +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/. +const possibleUIStringFilePaths = []; +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()) { + // Non-JS code, and Flow type definitions in .js.flow files. + if (!dirent.name.endsWith('.js')) { + continue; + } + + possibleUIStringFilePaths.push(path.join(kSrcDirName, _dirName, dirent.name)); + } else if (dirent.isDirectory()) { + const subdirName = path.join(_dirName, dirent.name); + + // Test code. + if (subdirName.endsWith('__tests__')) { + 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)); + +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 plan to 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 files in possibleUIStringFilePaths, and collect all string +// literals that might represent UI strings. +const possibleUiStringLiterals = 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) { + // To reduce false negatives, return false when `path` definitely + // doesn't represent a string literal for a UI string in the app. + + const { value } = path.value; + + // Non-string literals: numbers, booleans, etc. + if (typeof value !== 'string') { + return false; + } + + // String literals like 'react' in lines like + // import React from 'react'; + if (n.ImportDeclaration.check(path.parent.value)) { + return false; + } + + possibleUiStringLiterals.add(value); + + return this.traverse(path); + }, + }); +}); + +// Check each key ("message ID" in formatjs's lingo) against +// possibleUiStringLiterals, and make a list of any that aren't found. +const danglingMessageIds = Object.keys(messages_en).filter( + messageId => !possibleUiStringLiterals.has(messageId), +); + +if (danglingMessageIds.length > 0) { + console.warn( + "Found message IDs in static/translations/messages_en.json that don't seem to be used in the app:", + ); + console.warn(danglingMessageIds); +} diff --git a/yarn.lock b/yarn.lock index f736d0e60cb..974dfb6aa89 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3216,6 +3216,13 @@ ast-types@0.14.2: dependencies: tslib "^2.0.1" +ast-types@0.15.2, 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@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" @@ -5823,6 +5830,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" @@ -10453,6 +10465,16 @@ recast@^0.20.4: source-map "~0.6.1" tslib "^2.0.1" +recast@^0.21.2: + version "0.21.2" + resolved "https://registry.yarnpkg.com/recast/-/recast-0.21.2.tgz#f1995f76397da403e8ed9beafb01a6ff8c29ed10" + integrity sha512-jUR1+NtaBQdKDqBJ+qxwMm5zR8aLSNh268EfgAbY+EP4wcNEWb6hZFhFeYjaYanwgDahx5t47CH8db7X2NfKdQ== + dependencies: + 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"