Revert "DevTools: Added GRD/GRDP files for all localizable strings in the DevTools."
This reverts commit 2f05fdbf2ab3458b4b3a3f2a86f908a3a7d04f79.
Reason for revert: Breaks chrome translation, crbug 966547
Original change's description:
> DevTools: Added GRD/GRDP files for all localizable strings in the DevTools.
>
> This change includes...
> * A main GRD file for the DevTools (front_end/langpacks/devtools_ui_strings.grd)
> * GRDP files organized by subfolder
> * A node script that keeps the GRD/GRDP files in sync with the keys in the DevTools frontend
> * A git cl presubmit --upload check that runs the node script
>
> Note: Subsequent changes will add a build step to generate a .pak file that contains these DevTools strings. You can read about the overall approach here (https://bugs.chromium.org/p/chromium/issues/detail?id=941561).
>
>
> Details of this design:
> ======================
> We followed a similar pattern used by WebUI where strings are encoded in GRIT GRD/P files, which are used by a localization service to perform translations. They are also consumed in the build step to generate a .pak file, which is loaded by the browser's resource system.
>
> Chromium documentation:
> * https://www.chromium.org/developers/tools-we-use-in-chromium/grit/grit-users-guide
> * https://www.chromium.org/developers/design-documents/ui-localization
>
> Frontend strings:
> -----------------
> These are the localizable strings that are displayed to the user.
>
> GRDP <message> strings:
> -----------------------
> Each frontend string has a corresponding <message> entry in a GRDP file. These entries are what the localization service will use to perform localizations. It's also the input to the GRIT compiler, which generates a .pak file, which is loaded by the browser's resource_bundle system.
>
> GRDP <messsage> placeholders:
> ----------------------------
> Frontend strings use placeholders, which are used to substitute in values at runtime.
> For example,
> 'This string has %s two placeholder %.2f.'
>
> Since the order of the string may change in a different language, we need to encode the order of the placeholders. As such, in the GRDP file you'll find %s replaced with $[1-9].
>
> For example,
> 'This string has <ph name="ph1">$1s</ph> two placeholder <ph name="ph2">$2.2f</ph>.'
>
> Also, note that the precision and type of the placeholder is maintained (i.e. .2f).
>
> Detecting changes:
> -----------------
> The check_localizable_resources.js script performs the following check and generates an error if there are any changes that need to be made to a GRDP file.
>
> 1. Parses the frontend strings and hashes them.
> 2. Reads the messages from the GRDP files and hashes them.
> 3. Uses a difference between these two sets to report which strings need to be added and/or removed from GRDP files.
>
> Optionally, the user can specify --autofix and it will automatically update the appropriate GRDP files.
>
> Presubmit check:
> ---------------
> Running git cl presubmit --upload will run the check_localizable_resources.js script with the --autofix argument.
>
> If there are any changes, they reported to the user like this.
>
> ** Presubmit ERRORS **
> Error: Found changes to localizable DevTools strings.
> DevTools localizable resources checker has updated the appropriate grdp file(s).
> Manually write a description for any new <message> entries.
> Use git status to see what has changed.
>
> BUG=941561
>
> Change-Id: I5ac1656a037a6aaffeb4f64b103c4daec28be39a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613921
> Reviewed-by: Joel Einbinder <einbinder@chromium.org>
> Reviewed-by: Alexei Filippov <alph@chromium.org>
> Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#662371}
TBR=alph@chromium.org,einbinder@chromium.org,exterkamp@chromium.org,jeffish@microsoft.com,lomitch@microsoft.com
Change-Id: Ic4ca7d000d10b2bc61b585198469ca30f051dc62
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 941561
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627943
Reviewed-by: Krishna Govind <govind@chromium.org>
Commit-Queue: Krishna Govind <govind@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#662798}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: ceb872b6f6a02d707783563a225c77bb0c62170a
diff --git a/scripts/check_localizability.js b/scripts/check_localizability.js
index 30102a7..963eb49 100644
--- a/scripts/check_localizability.js
+++ b/scripts/check_localizability.js
@@ -11,11 +11,21 @@
// In this case, add it to the excluded errors at the top of the script.
const path = require('path');
-const localizationUtils = require('./localization_utils/localization_utils');
-const esprimaTypes = localizationUtils.esprimaTypes;
-const escodegen = localizationUtils.escodegen;
-const esprima = localizationUtils.esprima;
+// Use modules in third_party/node/node_modules
+const THIRD_PARTY_PATH = path.resolve(__dirname, '..', '..', '..', '..');
+const REPO_NODE_MODULES_PATH = path.resolve(THIRD_PARTY_PATH, 'node', 'node_modules');
+const escodegen = require(path.resolve(REPO_NODE_MODULES_PATH, 'escodegen'));
+const esprima = require(path.resolve(REPO_NODE_MODULES_PATH, 'esprima'));
+
+const fs = require('fs');
+const {promisify} = require('util');
+const readDirAsync = promisify(fs.readdir);
+const readFileAsync = promisify(fs.readFile);
+const statAsync = promisify(fs.stat);
+
+const excludeFiles = ['lighthouse-dt-bundle.js', 'Tests.js'];
+const excludeDirs = ['_test_runner', 'Images', 'node_modules'];
// Exclude known errors
const excludeErrors = [
'Common.UIString(view.title())', 'Common.UIString(setting.title() || \'\')', 'Common.UIString(option.text)',
@@ -24,6 +34,16 @@
'Common.UIString(extension.title())', 'Common.UIString(this._currentValueLabel, value)'
];
+const esprimaTypes = {
+ BI_EXPR: 'BinaryExpression',
+ CALL_EXPR: 'CallExpression',
+ COND_EXPR: 'ConditionalExpression',
+ IDENTIFIER: 'Identifier',
+ MEMBER_EXPR: 'MemberExpression',
+ TAGGED_TEMP_EXPR: 'TaggedTemplateExpression',
+ TEMP_LITERAL: 'TemplateLiteral'
+};
+
const usage = `Usage: node ${path.basename(process.argv[0])} [-a | <.js file path>*]
-a: If present, check all devtools frontend .js files
@@ -42,7 +62,7 @@
let filePaths = [];
if (process.argv[2] === '-a') {
const frontendPath = path.resolve(__dirname, '..', 'front_end');
- await localizationUtils.getFilesFromDirectory(frontendPath, filePaths, ['.js']);
+ await getFilesFromDirectory(frontendPath, filePaths);
} else {
filePaths = process.argv.slice(2);
}
@@ -66,15 +86,73 @@
main();
+function verifyIdentifier(node, name) {
+ return node !== undefined && node.type === esprimaTypes.IDENTIFIER && node.name === name;
+}
+
+/**
+ * Verify callee of objectName.propertyName(), e.g. Common.UIString().
+ */
+function verifyCallExpressionCallee(callee, objectName, propertyName) {
+ return callee !== undefined && callee.type === esprimaTypes.MEMBER_EXPR && callee.computed === false &&
+ verifyIdentifier(callee.object, objectName) && verifyIdentifier(callee.property, propertyName);
+}
+
+function isNodeCallOnObject(node, objectName, propertyName) {
+ return node !== undefined && node.type === esprimaTypes.CALL_EXPR &&
+ verifyCallExpressionCallee(node.callee, objectName, propertyName);
+}
+
+function isNodeCommonUIStringCall(node) {
+ return isNodeCallOnObject(node, 'Common', 'UIString');
+}
+
+function isNodeUIformatLocalized(node) {
+ return isNodeCallOnObject(node, 'UI', 'formatLocalized');
+}
+
+function isNodelsTaggedTemplateExpression(node) {
+ return node !== undefined && node.type === esprimaTypes.TAGGED_TEMP_EXPR && verifyIdentifier(node.tag, 'ls') &&
+ node.quasi !== undefined && node.quasi.type !== undefined && node.quasi.type === esprimaTypes.TEMP_LITERAL;
+}
+
function includesConditionalExpression(listOfElements) {
return listOfElements.filter(ele => ele !== undefined && ele.type === esprimaTypes.COND_EXPR).length > 0;
}
+function getLocalizationCase(node) {
+ if (isNodeCommonUIStringCall(node))
+ return 'Common.UIString';
+ else if (isNodelsTaggedTemplateExpression(node))
+ return 'Tagged Template';
+ else if (isNodeUIformatLocalized(node))
+ return 'UI.formatLocalized';
+ else
+ return null;
+}
+
+function isLocalizationCall(node) {
+ return isNodeCommonUIStringCall(node) || isNodelsTaggedTemplateExpression(node) || isNodeUIformatLocalized(node);
+}
+
function addError(error, errors) {
if (!errors.includes(error))
errors.push(error);
}
+function getLocation(node) {
+ if (node !== undefined && node.loc !== undefined && node.loc.start !== undefined && node.loc.end !== undefined &&
+ node.loc.start.line !== undefined && node.loc.end.line !== undefined) {
+ const startLine = node.loc.start.line;
+ const endLine = node.loc.end.line;
+ if (startLine === endLine)
+ return ` Line ${startLine}`;
+ else
+ return ` Line ${node.loc.start.line}-${node.loc.end.line}`;
+ }
+ return '';
+}
+
function buildConcatenatedNodesList(node, nodes) {
if (!node)
return;
@@ -95,7 +173,7 @@
*/
function checkConcatenation(parentNode, node, filePath, errors) {
function isWord(node) {
- return (node.type === esprimaTypes.LITERAL && !!node.value.match(/[a-z]/i));
+ return (node.type === 'Literal' && !!node.value.match(/[a-z]/i));
}
function isConcatenation(node) {
return (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+');
@@ -105,18 +183,16 @@
return;
if (isConcatenation(node)) {
- const concatenatedNodes = [];
+ let concatenatedNodes = [];
buildConcatenatedNodesList(node, concatenatedNodes);
- const hasLocalizationCall =
- !!concatenatedNodes.find(currentNode => localizationUtils.isLocalizationCall(currentNode));
+ const hasLocalizationCall = !!concatenatedNodes.find(currentNode => isLocalizationCall(currentNode));
if (hasLocalizationCall) {
const hasAlphabeticLiteral = !!concatenatedNodes.find(currentNode => isWord(currentNode));
if (hasAlphabeticLiteral) {
const code = escodegen.generate(node);
addError(
`${filePath}${
- localizationUtils.getLocationMessage(
- node.loc)}: string concatenation should be changed to variable substitution with ls: ${code}`,
+ getLocation(node)}: string concatenation should be changed to variable substitution with ls: ${code}`,
errors);
}
}
@@ -124,18 +200,26 @@
}
/**
+ * Verify if callee is functionName() or object.functionName().
+ */
+function verifyFunctionCallee(callee, functionName) {
+ return callee !== undefined &&
+ ((callee.type === esprimaTypes.IDENTIFIER && callee.name === functionName) ||
+ (callee.type === esprimaTypes.MEMBER_EXPR && verifyIdentifier(callee.property, functionName)));
+}
+
+/**
* Check if an argument of a function is localized.
*/
function checkFunctionArgument(functionName, argumentIndex, node, filePath, errors) {
- if (node !== undefined && node.type === esprimaTypes.CALL_EXPR &&
- localizationUtils.verifyFunctionCallee(node.callee, functionName) && node.arguments !== undefined &&
- node.arguments.length > argumentIndex) {
+ if (node !== undefined && node.type === esprimaTypes.CALL_EXPR && verifyFunctionCallee(node.callee, functionName) &&
+ node.arguments !== undefined && node.arguments.length > argumentIndex) {
const arg = node.arguments[argumentIndex];
// No need to localize empty strings.
- if (arg.type === esprimaTypes.LITERAL && arg.value === '')
+ if (arg.type == 'Literal' && arg.value === '')
return;
- if (!localizationUtils.isLocalizationCall(arg)) {
+ if (!isLocalizationCall(arg)) {
let order = '';
switch (argumentIndex) {
case 0:
@@ -151,8 +235,8 @@
order = `${argumentIndex + 1}th`;
}
addError(
- `${filePath}${localizationUtils.getLocationMessage(node.loc)}: ${order} argument to ${
- functionName}() should be localized: ${escodegen.generate(node)}`,
+ `${filePath}${getLocation(node)}: ${order} argument to ${functionName}() should be localized: ${
+ escodegen.generate(node)}`,
errors);
}
}
@@ -182,22 +266,19 @@
return;
}
- const locCase = localizationUtils.getLocalizationCase(node);
+ const locCase = getLocalizationCase(node);
const code = escodegen.generate(node);
switch (locCase) {
case 'Common.UIString':
case 'UI.formatLocalized':
const firstArgType = node.arguments[0].type;
- if (firstArgType !== esprimaTypes.LITERAL && firstArgType !== esprimaTypes.TEMP_LITERAL &&
- firstArgType !== esprimaTypes.IDENTIFIER && !excludeErrors.includes(code)) {
- addError(
- `${filePath}${localizationUtils.getLocationMessage(node.loc)}: first argument to call should be a string: ${
- code}`,
- errors);
+ if (firstArgType !== 'Literal' && firstArgType !== 'TemplateLiteral' && firstArgType !== 'Identifier' &&
+ !excludeErrors.includes(code)) {
+ addError(`${filePath}${getLocation(node)}: first argument to call should be a string: ${code}`, errors);
}
if (includesConditionalExpression(node.arguments.slice(1))) {
addError(
- `${filePath}${localizationUtils.getLocationMessage(node.loc)}: conditional(s) found in ${
+ `${filePath}${getLocation(node)}: conditional(s) found in ${
code}. Please extract conditional(s) out of the localization call.`,
errors);
}
@@ -205,7 +286,7 @@
case 'Tagged Template':
if (includesConditionalExpression(node.quasi.expressions)) {
addError(
- `${filePath}${localizationUtils.getLocationMessage(node.loc)}: conditional(s) found in ${
+ `${filePath}${getLocation(node)}: conditional(s) found in ${
code}. Please extract conditional(s) out of the localization call.`,
errors);
}
@@ -222,11 +303,42 @@
}
}
-async function auditFileForLocalizability(filePath, errors) {
- const fileContent = await localizationUtils.parseFileContent(filePath);
- const ast = esprima.parse(fileContent, {loc: true});
+function getRelativeFilePathFromSrc(fullFilePath) {
+ return path.relative(path.resolve(THIRD_PARTY_PATH, '..'), fullFilePath);
+}
- const relativeFilePath = localizationUtils.getRelativeFilePathFromSrc(filePath);
+async function auditFileForLocalizability(filePath, errors) {
+ const fileContent = await readFileAsync(filePath);
+ const ast = esprima.parse(fileContent.toString(), {loc: true});
+
+ const relativeFilePath = getRelativeFilePathFromSrc(filePath);
for (const node of ast.body)
analyzeNode(undefined, node, relativeFilePath, errors);
}
+
+function shouldParseDirectory(directoryName) {
+ return !excludeDirs.reduce((result, dir) => result || directoryName.indexOf(dir) !== -1, false);
+}
+
+function shouldParseFile(filePath) {
+ return (path.extname(filePath) === '.js' && !excludeFiles.includes(path.basename(filePath)));
+}
+
+async function getFilesFromItem(itemPath, filePaths) {
+ const stat = await statAsync(itemPath);
+ if (stat.isDirectory() && shouldParseDirectory(itemPath))
+ return await getFilesFromDirectory(itemPath, filePaths);
+
+ if (shouldParseFile(itemPath))
+ filePaths.push(itemPath);
+}
+
+async function getFilesFromDirectory(directoryPath, filePaths) {
+ const itemNames = await readDirAsync(directoryPath);
+ const promises = [];
+ for (const itemName of itemNames) {
+ const itemPath = path.resolve(directoryPath, itemName);
+ promises.push(getFilesFromItem(itemPath, filePaths));
+ }
+ await Promise.all(promises);
+}