Skip to content

Commit 796b88b

Browse files
chrisbobbechrisbobbe
committed
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.158.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)
1 parent b1eba83 commit 796b88b

File tree

3 files changed

+159
-0
lines changed

3 files changed

+159
-0
lines changed

‎package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@
9090
"@rollup/plugin-node-resolve": "^13.0.4",
9191
"@types/react-native": "^0.67.7",
9292
"@vusion/webfonts-generator": "^0.8.0",
93+
"ast-types": "^0.15.2",
9394
"core-js": "^3.1.4",
9495
"deep-freeze": "^0.0.1",
9596
"eslint": "^8.15.0",
@@ -104,6 +105,7 @@
104105
"eslint-plugin-react-hooks": "^4.5.0",
105106
"flow-bin": "^0.162.0",
106107
"flow-coverage-report": "^0.8.0",
108+
"flow-parser": "<0.159.0",
107109
"flow-typed": "^3.3.1",
108110
"hermes-eslint": "^0.9.0",
109111
"immutable-devtools": "^0.1.5",
@@ -120,6 +122,7 @@
120122
"prettier-eslint-cli": "^6.0.1",
121123
"react-native-cli": "^2.0.1",
122124
"react-test-renderer": "17.0.2",
125+
"recast": "^0.21.2",
123126
"redux-mock-store": "^1.5.1",
124127
"rollup": "^2.26.5",
125128
"sqlite3": "^5.0.2",

‎tools/check-messages-en.js‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
const fs = require('fs');
2+
const path = require('path');
3+
const flowParser = require('flow-parser');
4+
5+
const { parse } = require('recast');
6+
const { namedTypes: n, visit } = require('ast-types');
7+
8+
const messages_en = require('../static/translations/messages_en.json');
9+
10+
// Make a list of files that might contain UI strings, by recursing in src/.
11+
const possibleUIStringFilePaths = [];
12+
const kSrcDirName = 'src/';
13+
function walk(dir, _dirName = '') {
14+
let dirent;
15+
// eslint-disable-next-line no-cond-assign
16+
while ((dirent = dir.readSync())) {
17+
// To reduce false negatives, `continue` when nothing in `dirent` can
18+
// cause UI strings to appear in the app.
19+
20+
if (dirent.isFile()) {
21+
// Non-JS code, and Flow type definitions in .js.flow files.
22+
if (!dirent.name.endsWith('.js')) {
23+
continue;
24+
}
25+
26+
possibleUIStringFilePaths.push(path.join(kSrcDirName, _dirName, dirent.name));
27+
} else if (dirent.isDirectory()) {
28+
const subdirName = path.join(_dirName, dirent.name);
29+
30+
// Test code.
31+
if (subdirName.endsWith('__tests__')) {
32+
continue;
33+
}
34+
35+
walk(fs.opendirSync(path.join(kSrcDirName, subdirName)), subdirName);
36+
} else {
37+
// Something we don't expect to find under src/, probably containing
38+
// no UI strings. (symlinks? fifos, sockets, devices??)
39+
continue;
40+
}
41+
}
42+
}
43+
walk(fs.opendirSync(kSrcDirName));
44+
45+
const parseOptions = {
46+
parser: {
47+
parse(src) {
48+
return flowParser.parse(src, {
49+
// Comments can't cause UI strings to appear in the app; ignore them.
50+
all_comments: false,
51+
comments: false,
52+
53+
// We plan to use Flow enums; the parser shouldn't crash on them.
54+
enums: true,
55+
56+
// Set `tokens: true` just to work around a mysterious error.
57+
//
58+
// From the doc for this option:
59+
//
60+
// > include a list of all parsed tokens in a top-level tokens
61+
// > property
62+
//
63+
// We don't actually want this list of tokens. String literals do
64+
// get represented in the list, but as tokens, i.e., meaningful
65+
// chunks of the literal source code. They come with surrounding
66+
// quotes, escape syntax, etc:
67+
//
68+
// 'doesn\'t'
69+
// "doesn't"
70+
//
71+
// What we really want is the *value* of a string literal:
72+
//
73+
// doesn't
74+
//
75+
// and we get that from the AST.
76+
//
77+
// Anyway, we set `true` for this because otherwise I've been seeing
78+
// `parse` throw an error:
79+
//
80+
// Error: Line 72: Invalid regular expression: missing /
81+
//
82+
// TODO: Debug and/or file an issue upstream.
83+
tokens: true,
84+
});
85+
},
86+
},
87+
};
88+
89+
// Look at all files in possibleUIStringFilePaths, and collect all string
90+
// literals that might represent UI strings.
91+
const possibleUiStringLiterals = new Set();
92+
possibleUIStringFilePaths.forEach(filePath => {
93+
const source = fs.readFileSync(filePath).toString();
94+
const ast = parse(source, parseOptions);
95+
96+
visit(ast, {
97+
// Find nodes with type "Literal" in the AST.
98+
/* eslint-disable no-shadow */
99+
visitLiteral(path) {
100+
// To reduce false negatives, return false when `path` definitely
101+
// doesn't represent a string literal for a UI string in the app.
102+
103+
const { value } = path.value;
104+
105+
// Non-string literals: numbers, booleans, etc.
106+
if (typeof value !== 'string') {
107+
return false;
108+
}
109+
110+
// String literals like 'react' in lines like
111+
// import React from 'react';
112+
if (n.ImportDeclaration.check(path.parent.value)) {
113+
return false;
114+
}
115+
116+
possibleUiStringLiterals.add(value);
117+
118+
return this.traverse(path);
119+
},
120+
});
121+
});
122+
123+
// Check each key ("message ID" in formatjs's lingo) against
124+
// possibleUiStringLiterals, and make a list of any that aren't found.
125+
const danglingMessageIds = Object.keys(messages_en).filter(
126+
messageId => !possibleUiStringLiterals.has(messageId),
127+
);
128+
129+
if (danglingMessageIds.length > 0) {
130+
console.warn(
131+
"Found message IDs in static/translations/messages_en.json that don't seem to be used in the app:",
132+
);
133+
console.warn(danglingMessageIds);
134+
}

‎yarn.lock‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3019,6 +3019,13 @@ [email protected]:
30193019
dependencies:
30203020
tslib "^2.0.1"
30213021

3022+
[email protected], ast-types@^0.15.2:
3023+
version "0.15.2"
3024+
resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.15.2.tgz#39ae4809393c4b16df751ee563411423e85fb49d"
3025+
integrity sha512-c27loCv9QkZinsa5ProX751khO9DJl/AcB5c2KNtA6NRvHKS0PgLfcftz72KVq504vB0Gku5s2kUZzDBvQWvHg==
3026+
dependencies:
3027+
tslib "^2.0.1"
3028+
30223029
"ast-types@npm:@gregprice/ast-types@^0.15.3-0.tsflower.5":
30233030
version "0.15.3-0.tsflower.5"
30243031
resolved "https://registry.yarnpkg.com/@gregprice/ast-types/-/ast-types-0.15.3-0.tsflower.5.tgz#59bfaf5b776dc8bf3e9de7d94d5a9546f3a1b6df"
@@ -5495,6 +5502,11 @@ flow-parser@0.*:
54955502
resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.180.0.tgz#05d49a88715ceca0457607499a018e2bf5908d72"
54965503
integrity sha512-kkzsuGAhckWgn/G+JfCyEa6BYslGrjlH4CJL0LZhdn9of9ukvi7SzVQSFsrEhuhh/zQUghfUEoaeZy1wjQXpUg==
54975504

5505+
flow-parser@<0.159.0:
5506+
version "0.158.0"
5507+
resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.158.0.tgz#d845f167c722babe880110fc3681c44f21823399"
5508+
integrity sha512-0hMsPkBTRrkII/0YiG9ehOxFXy4gOWdk8RSRze5WbfeKAQpL5kC2K4BmumyTfU9o5gr7/llgElF3UpSSrjzQAA==
5509+
54985510
flow-parser@^0.121.0:
54995511
version "0.121.0"
55005512
resolved "https://registry.yarnpkg.com/flow-parser/-/flow-parser-0.121.0.tgz#9f9898eaec91a9f7c323e9e992d81ab5c58e618f"
@@ -10052,6 +10064,16 @@ recast@^0.20.3:
1005210064
source-map "~0.6.1"
1005310065
tslib "^2.0.1"
1005410066

10067+
recast@^0.21.2:
10068+
version "0.21.2"
10069+
resolved "https://registry.yarnpkg.com/recast/-/recast-0.21.2.tgz#f1995f76397da403e8ed9beafb01a6ff8c29ed10"
10070+
integrity sha512-jUR1+NtaBQdKDqBJ+qxwMm5zR8aLSNh268EfgAbY+EP4wcNEWb6hZFhFeYjaYanwgDahx5t47CH8db7X2NfKdQ==
10071+
dependencies:
10072+
ast-types "0.15.2"
10073+
esprima "~4.0.0"
10074+
source-map "~0.6.1"
10075+
tslib "^2.0.1"
10076+
1005510077
"recast@npm:@gregprice/recast@^0.21.2-0.tsflower.8":
1005610078
version "0.21.2-0.tsflower.8"
1005710079
resolved "https://registry.yarnpkg.com/@gregprice/recast/-/recast-0.21.2-0.tsflower.8.tgz#576b7329a3cb6fcd1e406ebded7daf3cd5d9f573"

0 commit comments

Comments
 (0)