commit 7aa2c6c30fa407dc2bad994c8ba8143adcd64c3b
author Lorne Mitchell <lomitch@microsoft.com> Wed Apr 03 03:50:10 2019 +0000
committer Commit Bot <commit-bot@chromium.org> Wed Apr 03 03:50:10 2019 +0000
tree 5ead15ddc8c6131f78f582cecd76f478a6272224
parent 85d37f05a9d3ee7bb57996ef668255521e146f0d

DevTools: Enabled check_localization presubmit script

* Enabled the check_localization presubmit script.
* Fixed localization issues caught by the check_localization presubmit script.
* Updated the check_localization script to allow contatenation of non-alphabetic strings with localized strings.
  * For example, ls`Status Code` + ": " is a valid concatenation. This allows for decorations to be concatenated with localized strings.

Change-Id: I741940c9ebdac363ac0ccad3f7de20d508204e2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1544878
Auto-Submit: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: Joel Einbinder <einbinder@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#647134}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7a329cb73b0b5d5cddb89a34fd7da6cb1a3c87dd

scripts/check_localizability.js

diff --git a/scripts/check_localizability.js b/scripts/check_localizability.js
index 3cbe973..be7fe4c 100644
--- a/scripts/check_localizability.js
+++ b/scripts/check_localizability.js
@@ -153,19 +153,48 @@
   return '';
 }
 
+function buildConcatenatedNodesList(node, nodes) {
+  if (!node)
+    return;
+  if (node.left === undefined && node.right === undefined) {
+    nodes.push(node);
+    return;
+  }
+  buildConcatenatedNodesList(node.left, nodes);
+  buildConcatenatedNodesList(node.right, nodes);
+}
+
 /**
  * Recursively check if there is concatenation to localization call.
+ * Concatenation is allowed between localized strings and non-alphabetic strings.
+ * It is not allowed between a localized string and a word.
+ * Example (allowed): ls`Status Code` + ": "
+ * Example (disallowed): ls`Status` + " Code" + ": "
  */
-function checkConcatenation(node, filePath, errors) {
-  if (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+') {
-    const code = escodegen.generate(node);
-    if (isLocalizationCall(node.left) || isLocalizationCall(node.right)) {
-      addError(
-          `${filePath}${getLocation(node)}: string concatenation should be changed to variable substitution with ls: ${
-              code}`,
-          errors);
-    } else {
-      [node.left, node.right].forEach(node => checkConcatenation(node, filePath, errors));
+function checkConcatenation(parentNode, node, filePath, errors) {
+  function isWord(node) {
+    return (node.type === 'Literal' && !!node.value.match(/[a-z]/i));
+  }
+  function isConcatenation(node) {
+    return (node !== undefined && node.type === esprimaTypes.BI_EXPR && node.operator === '+');
+  }
+
+  if (isConcatenation(parentNode))
+    return;
+
+  if (isConcatenation(node)) {
+    let concatenatedNodes = [];
+    buildConcatenatedNodesList(node, concatenatedNodes);
+    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}${
+                getLocation(node)}: string concatenation should be changed to variable substitution with ls: ${code}`,
+            errors);
+      }
     }
   }
 }
@@ -186,6 +215,10 @@
   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 == 'Literal' && arg.value === '')
+      return;
+
     if (!isLocalizationCall(arg)) {
       let order = '';
       switch (argumentIndex) {
@@ -213,13 +246,13 @@
  * Check esprima node object that represents the AST of code
  * to see if there is any localization error.
  */
-function analyzeNode(node, filePath, errors) {
+function analyzeNode(parentNode, node, filePath, errors) {
   if (node === undefined || node === null)
     return;
 
   if (node instanceof Array) {
     for (const child of node)
-      analyzeNode(child, filePath, errors);
+      analyzeNode(node, child, filePath, errors);
 
     return;
   }
@@ -260,7 +293,7 @@
       break;
     default:
       // String concatenation to localization call(s) should be changed
-      checkConcatenation(node, filePath, errors);
+      checkConcatenation(parentNode, node, filePath, errors);
       // 3rd argument to createInput() should be localized
       checkFunctionArgument('createInput', 2, node, filePath, errors);
       break;
@@ -268,7 +301,7 @@
 
   for (const key of objKeys) {
     // recursively parse all the child nodes
-    analyzeNode(node[key], filePath, errors);
+    analyzeNode(node, node[key], filePath, errors);
   }
 }
 
@@ -282,7 +315,7 @@
 
   const relativeFilePath = getRelativeFilePathFromSrc(filePath);
   for (const node of ast.body)
-    analyzeNode(node, relativeFilePath, errors);
+    analyzeNode(undefined, node, relativeFilePath, errors);
 }
 
 function shouldParseDirectory(directoryName) {