DevTools: Autofix out-of-date grdp/grd file paths in presubmit

This patch adds the functionality to autofix grdp/grd file paths that are
out of date.

* <part> file entries that reference non-existent grdp files in
devtools_ui_strings.grd are removed.

* Grdp files with the wrong names are renamed (format should be
<folder_name>_strings.grdp) along with the corresponding part file entries
in devtools_ui_strings.grd.

* If more than one grdp files are under a directory, an error is issued to
ask the user to consolidate these grdp files.

This patch will eliminate the need to manually fix grdp/grd inconsistensies
(e.g. https://crrev.com/c/1637591) after unexpected changes (e.g. audits2
renamed to audits: https://crrev.com/c/1614691).

Bug: 941561
Change-Id: Ie413fe070a2f5b4319a12ce07653ec1a79912215
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1703044Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Mandy Chen <mandy.chen@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#698917}

third_party/blink/renderer/devtools/scripts/check_localizable_resources.js

@@ -13,7 +13,6 @@
  */
 
 const fs = require('fs');
-const path = require('path');
 const {promisify} = require('util');
 const writeFileAsync = promisify(fs.writeFile);
 const appendFileAsync = promisify(fs.appendFile);
@@ -25,12 +24,16 @@ const grdpFileEnd = '</grit-part>';
 
 async function main() {
   try {
-    await checkLocalizedStrings.parseLocalizableResourceMaps();
+    const shouldAutoFix = process.argv.includes('--autofix');
+    const error = await checkLocalizedStrings.validateGrdAndGrdpFiles(shouldAutoFix);
+    if (error !== '' && !shouldAutoFix)
+      throw new Error(error);
 
-    if (process.argv.includes('--autofix'))
-      await autofix();
+    await checkLocalizedStrings.parseLocalizableResourceMaps();
+    if (shouldAutoFix)
+      await autofix(error);
     else
-      await getErrors();
+      await getErrors(error);
   } catch (e) {
     console.log(e.stack);
     console.log(`Error: ${e.message}`);
@@ -40,11 +43,11 @@ async function main() {
 
 main();
 
-async function getErrors() {
+async function getErrors(existingError) {
   const toAddError = await checkLocalizedStrings.getAndReportResourcesToAdd();
   const toModifyError = checkLocalizedStrings.getAndReportIDSKeysToModify();
   const toRemoveError = checkLocalizedStrings.getAndReportResourcesToRemove();
-  let error = `${toAddError || ''}${toModifyError || ''}${toRemoveError || ''}`;
+  let error = `${existingError}\n${toAddError || ''}${toModifyError || ''}${toRemoveError || ''}`;
 
   if (error === '') {
     console.log('DevTools localizable resources checker passed.');
@@ -56,20 +59,25 @@ async function getErrors() {
   throw new Error(error);
 }
 
-async function autofix() {
+async function autofix(existingError) {
   const keysToAddToGRD = checkLocalizedStrings.getMessagesToAdd();
   const keysToRemoveFromGRD = checkLocalizedStrings.getMessagesToRemove();
   const resourceAdded = await addResourcesToGRDP(keysToAddToGRD, keysToRemoveFromGRD);
   const resourceModified = await modifyResourcesInGRDP();
   const resourceRemoved = await removeResourcesFromGRDP(keysToRemoveFromGRD);
 
-  if (!resourceAdded && !resourceRemoved && !resourceModified) {
+  if (!resourceAdded && !resourceRemoved && !resourceModified && existingError === '') {
     console.log('DevTools localizable resources checker passed.');
     return;
   }
 
   let message =
-      'Found changes to localizable DevTools strings.\nDevTools localizable resources checker has updated the appropriate grdp file(s).';
+      'Found changes to localizable DevTools resources.\nDevTools localizable resources checker has updated the appropriate grd/grdp file(s).';
+  if (existingError !== '') {
+    message +=
+        `\nGrd/Grdp files have been updated. Please verify the updated grdp files and/or the <part> file references in ${
+            localizationUtils.getRelativeFilePathFromSrc(localizationUtils.GRD_PATH)} are correct.`;
+  }
   if (resourceAdded)
     message += '\nManually write a description for any new <message> entries.';
   if (resourceRemoved && duplicateRemoved(keysToRemoveFromGRD))
@@ -124,8 +132,7 @@ async function addResourcesToGRDP(keysToAddToGRD, keysToRemoveFromGRD) {
 
       // Create a new grdp file and reference it in the parent grd file
       promises.push(appendFileAsync(grdpFilePath, `${grdpFileStart}${grdpMessagesToAdd}${grdpFileEnd}`));
-      grdpFilePathsToAdd.push(
-          path.relative(path.dirname(localizationUtils.GRD_PATH), grdpFilePath).split(path.sep).join('/'));
+      grdpFilePathsToAdd.push(grdpFilePath);
       continue;
     }
 
@@ -162,45 +169,11 @@ async function addResourcesToGRDP(keysToAddToGRD, keysToRemoveFromGRD) {
 
     promises.push(writeFileAsync(grdpFilePath, newGrdpFileContent));
   }
-  promises.push(addChildGRDPFilePathsToGRD(grdpFilePathsToAdd.sort()));
+  promises.push(localizationUtils.addChildGRDPFilePathsToGRD(grdpFilePathsToAdd.sort()));
   await Promise.all(promises);
   return true;
 }
 
-async function addChildGRDPFilePathsToGRD(relativeGrdpFilePaths) {
-  function createPartFileEntry(relativeGrdpFilePath) {
-    return `      <part file="${relativeGrdpFilePath}" />\n`;
-  }
-
-  const grdFileContent = await localizationUtils.parseFileContent(localizationUtils.GRD_PATH);
-  const grdLines = grdFileContent.split('\n');
-
-  let newGrdFileContent = '';
-  for (let i = 0; i < grdLines.length; i++) {
-    const grdLine = grdLines[i];
-    // match[0]: full match
-    // match[1]: relative grdp file path
-    const match = grdLine.match(/<part file="(.*?)"/);
-    if (match) {
-      const relativeGrdpFilePathsRemaining = [];
-      for (const relativeGrdpFilePath of relativeGrdpFilePaths) {
-        if (relativeGrdpFilePath < match[1])
-          newGrdFileContent += createPartFileEntry(relativeGrdpFilePath);
-        else
-          relativeGrdpFilePathsRemaining.push(relativeGrdpFilePath);
-      }
-      relativeGrdpFilePaths = relativeGrdpFilePathsRemaining;
-    } else if (grdLine.includes('</messages>')) {
-      for (const relativeGrdpFilePath of relativeGrdpFilePaths)
-        newGrdFileContent += createPartFileEntry(relativeGrdpFilePath);
-    }
-    newGrdFileContent += grdLine;
-    if (i < grdLines.length - 1)
-      newGrdFileContent += '\n';
-  }
-  return writeFileAsync(localizationUtils.GRD_PATH, newGrdFileContent);
-}
-
 // Return true if any resources are updated
 async function modifyResourcesInGRDP() {
   const messagesToModify = checkLocalizedStrings.getIDSKeysToModify();

third_party/blink/renderer/devtools/scripts/localization_utils/check_localized_strings.js

@@ -8,7 +8,11 @@
  * files and report error if present.
  */
 
+const fs = require('fs');
 const path = require('path');
+const {promisify} = require('util');
+const writeFileAsync = promisify(fs.writeFile);
+const renameFileAsync = promisify(fs.rename);
 const localizationUtils = require('./localization_utils');
 const escodegen = localizationUtils.escodegen;
 const esprimaTypes = localizationUtils.esprimaTypes;
@@ -57,13 +61,138 @@ const IDSkeys = new Map();
 const fileToGRDPMap = new Map();
 
 const devtoolsFrontendPath = path.resolve(__dirname, '..', '..', 'front_end');
+let devtoolsFrontendDirs;
 
+/**
+ * The following functions validate and update grd/grdp files.
+ */
+
+async function validateGrdAndGrdpFiles(shouldAutoFix) {
+  const grdError = await validateGrdFile(shouldAutoFix);
+  const grdpError = await validateGrdpFiles(shouldAutoFix);
+  if (grdError !== '' || grdpError !== '')
+    return `${grdError}\n${grdpError}`;
+  else
+    return '';
+}
+
+function expectedGrdpFilePath(dir) {
+  return path.resolve(dir, `${path.basename(dir)}_strings.grdp`);
+}
+
+async function validateGrdFile(shouldAutoFix) {
+  const fileContent = await localizationUtils.parseFileContent(localizationUtils.GRD_PATH);
+  const fileLines = fileContent.split('\n');
+  const newLines = [];
+  let errors = '';
+  fileLines.forEach(line => errors += validateGrdLine(line, newLines));
+  if (errors !== '' && shouldAutoFix)
+    await writeFileAsync(localizationUtils.GRD_PATH, newLines.join('\n'));
+  return errors;
+}
+
+function validateGrdLine(line, newLines) {
+  let error = '';
+  const match = line.match(/<part file="([^"]*)" \/>/);
+  if (!match) {
+    newLines.push(line);
+    return error;
+  }
+  // match[0]: full match
+  // match[1]: relative grdp file path
+  const grdpFilePath = localizationUtils.getAbsoluteGrdpPath(match[1]);
+  const expectedGrdpFile = expectedGrdpFilePath(path.dirname(grdpFilePath));
+  if (fs.existsSync(grdpFilePath) &&
+      (grdpFilePath === expectedGrdpFile || grdpFilePath === localizationUtils.SHARED_STRINGS_PATH)) {
+    newLines.push(line);
+    return error;
+  } else if (!fs.existsSync(grdpFilePath)) {
+    error += `${line.trim()} in ${
+                 localizationUtils.getRelativeFilePathFromSrc(
+                     localizationUtils.GRD_PATH)} refers to a grdp file that doesn't exist. ` +
+        `Please verify the grdp file and update the <part file="..."> entry to reference the correct grdp file. ` +
+        `Make sure the grdp file name is ${path.basename(expectedGrdpFile)}.`
+  } else {
+    error += `${line.trim()} in ${
+        localizationUtils.getRelativeFilePathFromSrc(localizationUtils.GRD_PATH)} should reference "${
+        localizationUtils.getRelativeGrdpPath(expectedGrdpFile)}".`;
+  }
+  return error;
+}
+
+async function validateGrdpFiles(shouldAutoFix) {
+  const frontendDirsToGrdpFiles = await mapFrontendDirsToGrdpFiles();
+  const grdFileContent = await localizationUtils.parseFileContent(localizationUtils.GRD_PATH);
+  let errors = '';
+  const renameFilePromises = [];
+  const grdpFilesToAddToGrd = [];
+  frontendDirsToGrdpFiles.forEach(
+      (grdpFiles, dir) => errors +=
+      validateGrdpFile(dir, grdpFiles, grdFileContent, shouldAutoFix, renameFilePromises, grdpFilesToAddToGrd));
+  if (grdpFilesToAddToGrd.length > 0)
+    await localizationUtils.addChildGRDPFilePathsToGRD(grdpFilesToAddToGrd.sort());
+  await Promise.all(renameFilePromises);
+  return errors;
+}
+
+async function mapFrontendDirsToGrdpFiles() {
+  devtoolsFrontendDirs =
+      devtoolsFrontendDirs || await localizationUtils.getChildDirectoriesFromDirectory(devtoolsFrontendPath);
+  const dirToGrdpFiles = new Map();
+  const getGrdpFilePromises = devtoolsFrontendDirs.map(dir => {
+    const files = [];
+    dirToGrdpFiles.set(dir, files);
+    return localizationUtils.getFilesFromDirectory(dir, files, ['.grdp']);
+  });
+  await Promise.all(getGrdpFilePromises);
+  return dirToGrdpFiles;
+}
+
+function validateGrdpFile(dir, grdpFiles, grdFileContent, shouldAutoFix, renameFilePromises, grdpFilesToAddToGrd) {
+  let error = '';
+  const expectedGrdpFile = expectedGrdpFilePath(dir);
+  if (grdpFiles.length === 0)
+    return error;
+  if (grdpFiles.length > 1) {
+    throw new Error(`${grdpFiles.length} GRDP files found under ${
+        localizationUtils.getRelativeFilePathFromSrc(dir)}. Please make sure there's only one GRDP file named ${
+        path.basename(expectedGrdpFile)} under this directory.`);
+  }
+
+  // Only one grdp file is under the directory
+  if (grdpFiles[0] !== expectedGrdpFile) {
+    // Rename grdp file and the reference in the grd file
+    if (shouldAutoFix) {
+      renameFilePromises.push(renameFileAsync(grdpFiles[0], expectedGrdpFile));
+      grdpFilesToAddToGrd.push(expectedGrdpFile);
+    } else {
+      error += `${localizationUtils.getRelativeFilePathFromSrc(grdpFiles[0])} should be renamed to ${
+          localizationUtils.getRelativeFilePathFromSrc(expectedGrdpFile)}.`;
+    }
+    return error;
+  }
+
+  // Only one grdp file and its name follows the naming convention
+  if (!grdFileContent.includes(localizationUtils.getRelativeGrdpPath(grdpFiles[0]))) {
+    if (shouldAutoFix) {
+      grdpFilesToAddToGrd.push(grdpFiles[0]);
+    } else {
+      error += `Please add ${localizationUtils.createPartFileEntry(grdpFiles[0]).trim()} to ${
+          localizationUtils.getRelativeFilePathFromSrc(grdpFiles[0])}.`;
+    }
+  }
+  return error;
+}
+
+/**
+ * Parse localizable resources.
+ */
 async function parseLocalizableResourceMaps() {
   const grdpToFiles = new Map();
-  const dirs = await localizationUtils.getChildDirectoriesFromDirectory(devtoolsFrontendPath);
+  const dirs = devtoolsFrontendDirs || await localizationUtils.getChildDirectoriesFromDirectory(devtoolsFrontendPath);
   const grdpToFilesPromises = dirs.map(dir => {
     const files = [];
-    grdpToFiles.set(path.resolve(dir, `${path.basename(dir)}_strings.grdp`), files);
+    grdpToFiles.set(expectedGrdpFilePath(dir), files);
     return localizationUtils.getFilesFromDirectory(dir, files, ['.js', 'module.json']);
   });
   await Promise.all(grdpToFilesPromises);
@@ -76,7 +205,7 @@ async function parseLocalizableResourceMaps() {
   await Promise.all(promises);
   // Parse grd(p) files after frontend strings are processed so we know
   // what to add or remove based on frontend strings
-  await parseIDSKeys(localizationUtils.GRD_PATH);
+  await parseIDSKeys();
 }
 
 /**
@@ -255,15 +384,15 @@ function addString(str, code, filePath, location, argumentNodes) {
  * devtools frontend grdp files.
  */
 
-async function parseIDSKeys(grdFilePath) {
+async function parseIDSKeys() {
   // NOTE: this function assumes that no <message> tags are present in the parent
-  const grdpFilePaths = await parseGRDFile(grdFilePath);
+  const grdpFilePaths = await parseGRDFile();
   await parseGRDPFiles(grdpFilePaths);
 }
 
-async function parseGRDFile(grdFilePath) {
-  const fileContent = await localizationUtils.parseFileContent(grdFilePath);
-  const grdFileDir = path.dirname(grdFilePath);
+async function parseGRDFile() {
+  const fileContent = await localizationUtils.parseFileContent(localizationUtils.GRD_PATH);
+  const grdFileDir = path.dirname(localizationUtils.GRD_PATH);
   const partFileRegex = /<part file="(.*?)"/g;
 
   let match;
@@ -506,4 +635,5 @@ module.exports = {
   getLongestDescription,
   getMessagesToAdd,
   getMessagesToRemove,
+  validateGrdAndGrdpFiles,
 };

third_party/blink/renderer/devtools/scripts/localization_utils/localization_utils.js

@@ -9,6 +9,7 @@ const path = require('path');
 const readFileAsync = promisify(fs.readFile);
 const readDirAsync = promisify(fs.readdir);
 const statAsync = promisify(fs.stat);
+const writeFileAsync = promisify(fs.writeFile);
 
 const esprimaTypes = {
   BI_EXPR: 'BinaryExpression',
@@ -253,7 +254,7 @@ function createGrdpMessage(ids, stringObj) {
 }
 
 function getIDSKey(str) {
-  return `${IDSPrefix}${md5(str)}`
+  return `${IDSPrefix}${md5(str)}`;
 }
 
 // Get line number in the file of a character at given index
@@ -262,17 +263,67 @@ function lineNumberOfIndex(str, index) {
   return stringToIndex.split('\n').length;
 }
 
+// Relative file path from grdp file with back slash replaced with forward slash
+function getRelativeGrdpPath(grdpPath) {
+  return path.relative(path.dirname(GRD_PATH), grdpPath).split(path.sep).join('/');
+}
+
+function getAbsoluteGrdpPath(relativeGrdpFilePath) {
+  return path.resolve(path.dirname(GRD_PATH), relativeGrdpFilePath);
+}
+
+// Create a <part> entry, given absolute path of a grdp file
+function createPartFileEntry(grdpFilePath) {
+  const relativeGrdpFilePath = getRelativeGrdpPath(grdpFilePath);
+  return `      <part file="${relativeGrdpFilePath}" />\n`;
+}
+
+// grdpFilePaths are sorted and are absolute file paths
+async function addChildGRDPFilePathsToGRD(grdpFilePaths) {
+  const grdFileContent = await parseFileContent(GRD_PATH);
+  const grdLines = grdFileContent.split('\n');
+
+  let newGrdFileContent = '';
+  for (let i = 0; i < grdLines.length; i++) {
+    const grdLine = grdLines[i];
+    // match[0]: full match
+    // match[1]: relative grdp file path
+    const match = grdLine.match(/<part file="(.*?)"/);
+    if (match) {
+      const grdpFilePathsRemaining = [];
+      for (const grdpFilePath of grdpFilePaths) {
+        if (grdpFilePath < getAbsoluteGrdpPath(match[1]))
+          newGrdFileContent += createPartFileEntry(grdpFilePath);
+        else
+          grdpFilePathsRemaining.push(grdpFilePath);
+      }
+      grdpFilePaths = grdpFilePathsRemaining;
+    } else if (grdLine.includes('</messages>')) {
+      for (const grdpFilePath of grdpFilePaths)
+        newGrdFileContent += createPartFileEntry(grdpFilePath);
+    }
+    newGrdFileContent += grdLine;
+    if (i < grdLines.length - 1)
+      newGrdFileContent += '\n';
+  }
+  return writeFileAsync(GRD_PATH, newGrdFileContent);
+}
+
 module.exports = {
+  addChildGRDPFilePathsToGRD,
   createGrdpMessage,
+  createPartFileEntry,
   escodegen,
   esprima,
   esprimaTypes,
+  getAbsoluteGrdpPath,
   getChildDirectoriesFromDirectory,
   getFilesFromDirectory,
   getIDSKey,
   getLocalizationCase,
   getLocationMessage,
   getRelativeFilePathFromSrc,
+  getRelativeGrdpPath,
   GRD_PATH,
   IDSPrefix,
   isLocalizationCall,