Add layout-test-tidy to LayoutTests/webaudio/tools

third_party/WebKit/LayoutTests/webaudio/tools/layout-test-tidy.js

+#!/usr/bin/env node
+
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+'use strict';
+
+const os = require('os');
+const fs = require('fs');
+const path = require('path');
+const glob = require("glob")
+const libtidy = require('libtidy');
+const cp = require('child_process');
+const jsdom = require('jsdom');
+const { JSDOM } = jsdom;
+
+
+/**
+ * Options for sub modules.
+ */
+const OPTIONS = {
+
+  HTMLTidy: {
+    'indent': 'yes',
+    'indent-spaces': '2',
+    'wrap': '80',
+    'tidy-mark': 'no'
+  },
+
+  ClangFormat: [
+    '-style=Chromium',
+    '-assume-filename=a.js'
+  ],
+
+  // RegExp text swap collection (ordered key-value pair) for post-processing.
+  RegExpSwapCollection: [
+    {regexp: /var /, replace: 'let '},

[Raymond Toy, 2017/05/10 21:28:44]

I think you need to tighten up the regexp here:

let message = "This var should be safe to use"

will become "This let should be safe to use".

[hongchan, 2017/05/11 21:13:02]

Please advise if you have a better idea.

+    // Move one line up the dangling closing script tags.
+    {regexp: /\>\n\s{2,}\<\/script\>\n/, replace: '></script>\n'},
+    // Clean up redundant braces at the beginning/end.

[Raymond Toy, 2017/05/10 21:28:44]

Expand on why this is needed. Presumably for clang format?

How do you prevent code containing three braces from getting deleted? 

[hongchan, 2017/05/11 21:13:02]

This is a hack for clang-format module. The result of clang-format always starts
from 0 col, no matter when the original code was at. So the clang-format modules
adds 3 braces top and bottom, and this takes them away at the end of process.

+    {regexp: /test\-code\"\>(\n\s*\{){3}/, replace: 'test-code">'},

[Raymond Toy, 2017/05/10 21:28:44]

Why is the replacement "test-code>"?

[hongchan, 2017/05/11 21:13:01]

Fixed with more specific patterns.

[hongchan, 2017/05/11 21:13:02]

Basically |test-code>| is the beginning of the test code. Just trying to be as
specific as possible. Otherwise, this will be applied to all the script
elements.

+    // Remove extra braces that are added for indented JS code.
+    {regexp: /(\}\n\s*){3}\<\/script\>/, replace: '</script>'},
+    // Remove all the empty lines in html.
+    {regexp: /\>\n{2,}/, replace: '>\n'}
+  ]
+};
+
+
+const Bar80 = {

[Raymond Toy, 2017/05/10 21:28:44]

What is this for?

[hongchan, 2017/05/11 21:13:01]

This prints out the 80 col ruler top and bottom, only for --dryrun. Now the
latest patch has the auto detection of overflowed lines, so I am removing this.

+  Start: '+.........' + '.'.repeat(60) + '.80.COLS.+',
+  End: '+.........' + '.'.repeat(60) + '.....EOF.+'
+};
+
+
+/**
+ * Basic utilities.
+ */
+const Util = {
+
+  logAndExit: (moduleName, messageString) => {
+    console.error('[layout-test-tidy::' + moduleName + '] ' + messageString);
+    process.exit(1);
+  },
+
+  loadFileToStringSync: (filePath) => {
+    return fs.readFileSync(filePath);
+  },
+
+  writeStringToFileSync: (pageString, filePath) => {
+    fs.writeFileSync(filePath, pageString);
+  }
+
+};
+
+
+/**
+ * Wrapper for external modules like HTMLTidy and clang format.
+ * @type {Object}
+ */
+const Module = {
+
+  /**
+   * Perform a batch RegExp string substution.
+   * @param {String} targetString Target string.
+   * @param {Array} swapCollection Array of key-value pairs. {regexp: replace}

[Raymond Toy, 2017/05/10 21:28:44]

"{regexp: replace}" looks weird for what you're trying to describe?

[hongchan, 2017/05/11 21:13:02]

Done.

+   * @return {String}
+   */
+  runRegExpSwapSync: (targetString, regExpSwapCollection) => {
+    let tempString = targetString;
+    regExpSwapCollection.forEach((item) => {
+      let re = new RegExp(item.regexp, 'g');
+      tempString = tempString.replace(re, item.replace);
+    });
+
+    return tempString;
+  },
+
+  /**
+   * Run HTMLTidy on input string with options.
+   * @param  {String} pageString [description]
+   * @param  {Object} options HTMLTidy option as key-value pair.
+   * @param  {Task} task Associated Task object.
+   * @return {String}
+   */
+  runHTMLTidySync: (pageString, options, task) => {
+    let tidyDoc = new libtidy.TidyDoc();
+    for (let option in options)
+      tidyDoc.optSet(option, options[option]);
+
+    // This actually process the data inside of |tidyDoc|.
+    let logs = '';
+    logs += tidyDoc.parseBufferSync(Buffer(pageString));
+    logs += tidyDoc.cleanAndRepairSync();
+    logs += tidyDoc.runDiagnosticsSync();
+
+    task.addLog('Module.runHTMLTidySync', logs.split('\n'));
+
+    return tidyDoc.saveBufferSync().toString();
+  },
+
+  /**
+   * Run clang-format and return a promise.
+   * @param {String} codeString JS code to apply clang-format.
+   * @param {Array} clangFormatOption optios array for clang-format.
+   * @param {Task} task Associated Task object.
+   * @return {Promise} Processed code as string.
+   * @resolve {String} clang-formatted JS code as string.
+   * @reject {Error}
+   */
+  runClangFormat: (codeString, clangFormatOption, task) => {
+    let clangFormatBinary = __dirname + '/node_modules/clang-format/bin/';

[Raymond Toy, 2017/05/10 21:28:44]

Does this mean we actually install our own copy of clang-format?  If so, does it
matter if the installed version doesn't match the version from depot_tools?

I think we should just use depot_tools/clang-format, if possible.

[hongchan, 2017/05/11 21:13:02]

Until the weird depot_tools issue is resolved, I prefer to use clang-format that
we can control the version of it.

https://bugs.chromium.org/p/chromium/issues/detail?id=558447

+    clangFormatBinary += (os.platform() === 'win32')
+        ? 'win32/clang-format.exe'
+        : os.platform() + "_" + os.arch() + '/clang-format';
+
+    return new Promise((resolve, reject) => {
+      let echo = cp.spawn('echo', [codeString]);

[Raymond Toy, 2017/05/10 21:28:44]

IIUC, this basically runs a shell with the echo command whose argument is the
code string. If so, I think this is wrong.  Shell quoting will mess things up
and command lines have a relatively short line length limit.

[hongchan, 2017/05/11 21:13:02]

Well, that I don't understand and this worked perfectly on both my mac machines.
Feel free to advise alternatives if you have!

+      let result = '';
+
+      // Be sure to pipe the result, not to stdio.
+      let clangFormat = cp.spawn(clangFormatBinary,
+                                 clangFormatOption,
+                                 {stdio: ['pipe', 'pipe', process.stderr]});
+
+      echo.stdout.pipe(clangFormat.stdin);
+
+      // For debug purpose:
+      // clangFormat.stdout.pipe(process.stdout);
+
+      clangFormat.stdout.on('data', (data) => {
+        // Capture the data streamed.
+        result += data;
+      });
+
+      clangFormat.stdout.on('close', (exitCode) => {
+        if (exitCode) {
+          Util.logAndExit('Module.runClangFormat', 'exit code = 1');
+        } else {
+          task.addLog('Module.runClangFormat', 'clang-format was successful.');
+          resolve(result);
+        }
+      });
+    });
+  },
+
+  /**
+   * Detect line overflow and record the line number to the task log.
+   * @param {String} pageOrCodeString HTML page or JS code data in string.
+   * @param {TidyTask} task Associated TidyTask object.
+   */
+  detectLineOverflow: (pageOrCodeString, task) => {
+    let currentLineNumber = 0;
+    let index0 = 0;
+    let index1 = 0;
+    while (index0 < pageOrCodeString.length - 1) {
+      index1 = pageOrCodeString.indexOf('\n', index0);
+      if (index1 - index0 > 80) {
+        task.addLog('Module.detectLineOverflow',
+                    'Overflow (> 80 cols.) at line ' + currentLineNumber + '.');
+      }
+      currentLineNumber++;
+      index0 = index1 + 1;
+    }
+  }
+
+};
+
+
+/**
+ * DOM utilities. Process DOM processing after parsing the string by JSDOM.
+ */
+const DOMUtil = {
+
+  /**
+   * Parse string, generate JSDOM object and return |document| element.
+   * @param {String} pageString An HTML page in string.
+   * @return {Document} A |document| object.
+   */
+  getJSDOMFromStringSync: (pageString) => {
+    return new JSDOM(`${pageString}`);
+    // return jsdom_.window.document;
+  },
+
+  /**
+   * In-place tidy up head element.
+   * @param {Document} document A |document| object.
+   * @param {Task} task An associated Task object.
+   * @return {Void}
+   */
+  tidyHeadElementSync: (document, task) => {
+    try {
+      // If the title is missing, add one with the file name.

[Raymond Toy, 2017/05/10 21:28:43]

"with the" -> "from the"

[hongchan, 2017/05/11 21:13:01]

Done.

+      let titleElement = document.querySelector('title');
+      if (!titleElement) {
+        titleElement = document.createElement('title');
+        titleElement.textContent = path.basename(task.targetFilePath_);
+        task.addLog('DOMUtil.tidyHeadElementSync',
+                    'Title element was missing thus a new one was added.');
+      }
+
+      // The title element should be the first.
+      let headElement = document.querySelector('head');
+      headElement.insertBefore(titleElement, headElement.firstChild);
+
+      // If a script element in body does not have JS code, move to the head
+      // section.

[Raymond Toy, 2017/05/10 21:28:44]

Do we really want to do this? Can't think of anything that would break with
this, though.

[hongchan, 2017/05/11 21:13:01]

Yes, we do want to have this.

All the scripts in the head is synchronous/blocking whereas the script in the
body is not. We do want to have all the dependencies loaded first. We actually
move all the script element to the head and make the body empty, but I didn't do
that. 

+      let scriptElementsInBody = document.body.querySelectorAll('script');
+      scriptElementsInBody.forEach((scriptElement) => {
+        if (!scriptElement.textContent)
+          headElement.appendChild(scriptElement);
+      });
+    } catch (error) {
+      task.addLog('DOMUtil.tidyHeadElementSync', error.toString());
+    }
+  },
+
+  /**
+   * Sanitize and extract |script| element with JS test code.
+   * @param {Document} document A |document| object.
+   * @param {Task} task An associated Task object.
+   * @return {ScriptElement}
+   */
+  getElementWithTestCodeSync: (document, task) => {
+    let numberOfElementsWithCode = 0;
+    let elementWithTestCode;
+    let scriptElements = document.querySelectorAll('script');
+
+    scriptElements.forEach(function (element) {
+      // We don't want type attribute.
+      element.removeAttribute('type');
+
+      if (element.textContent.length > 0) {
+        ++numberOfElementsWithCode;
+        elementWithTestCode = element;
+        element.id = 'layout-test-code';

[Raymond Toy, 2017/05/10 21:28:44]

Is this safe?  What if another element has the same id?

[hongchan, 2017/05/11 21:13:01]

If there are two or more script elements with the code, this will throw.

+        // If the element was belong to something else than body, move it.

[Raymond Toy, 2017/05/10 21:28:44]

"was belong" -> "belongs", I think.
"else than" -> "else other than"

Move it where?

[hongchan, 2017/05/11 21:13:02]

Done.

+        if (element.parentElement !== document.body)
+          document.body.appendChild(element);
+      }
+    });
+
+    if (numberOfElementsWithCode !== 1) {
+      task.addLog('DOMUtil.getElementWithTestCodeSync',

[Raymond Toy, 2017/05/10 21:28:43]

Is it really bad to have more than one? What if we do?

[hongchan, 2017/05/11 21:13:01]

Yes, it's bad. If there are two script tags with actual code, that means
refactoring is necessary. I believe only the high-level code should be used in
the html file within one element. Other codes need to be in the separate JS
files.

+                  numberOfElementsWithCode + ' <script> element(s) with JS ' +
+                  'code were found.');
+      elementWithTestCode = null;
+    }
+
+    return elementWithTestCode;
+  }
+
+};
+
+
+/**
+ * @class TidyTask
+ * @description Per-file processing task. This object should be constructed
+ *              directly. The task runner creates this when it is necessary.
+ */
+class TidyTask {
+
+  /**
+   * @param {String} targetFilePath A path to file to be processed.
+   * @param {Boolean} isDryRun Print out the result to |stdout| when true. By
+   *                           default, this is true.
+   */
+  constructor (targetFilePath, isDryRun) {
+    this.targetFilePath_ = targetFilePath;
+    this.isDryRun_ = isDryRun;
+
+    this.fileType_ = path.extname(this.targetFilePath_);
+    this.pageString_ = Util.loadFileToStringSync(this.targetFilePath_);
+    this.jsdom_ = null;
+    this.logs_ = {};
+  }
+
+  /**
+   * Run processing sequence. Don't call this directly.
+   * @param {Function} taskDone Task runner callback function.
+   */
+  run (taskDone) {
+    switch (this.fileType_) {
+      case '.html':
+        this.processHTML_(taskDone);
+        break;
+      case '.js':
+        this.processJS_(taskDone);
+        break;
+      default:
+        Util.logAndExit('TidyTask.constructor', 'Invalid file type: '
+            + this.fileType_);
+        break;
+    }
+  }
+
+  processHTML_ (taskDone) {
+    // Parse page string into JSDOM.element object.
+    this.jsdom_ = DOMUtil.getJSDOMFromStringSync(this.pageString_);
+
+    // Clean up the head element section.
+    DOMUtil.tidyHeadElementSync(this.jsdom_.window.document, this);
+
+    let scriptElement =
+        DOMUtil.getElementWithTestCodeSync(this.jsdom_.window.document, this);
+
+    if (!scriptElement)
+      Util.logAndExit('TidyTask.processHTML_', 'Invalid <script> element.');
+
+    // Extract JS code string with additional braces for indentation hack.

[Raymond Toy, 2017/05/10 21:28:44]

Add note that the number of braces depends on the indent size and the expected
script tag level.

(It would be cool if this could be computed based on the level of the script
tag.

[hongchan, 2017/05/11 21:13:01]

Currently the main script is always two level lower from the <html> and one
level lower from the <body>. Are we expecting to have something else in the
future?

+    let codeString = '{\n{\n{' + scriptElement.textContent + '}\n}\n}';

[Raymond Toy, 2017/05/10 21:28:44]

When I did this by hand, I inserted "{{{\n" and appended "\n}}}". I think that's
better than separating each by \n, and it's nice to have the actual code start a
new line rather than being smashed up against "{".  (Not that that is really
visible anywhere, but might be useful for debugging.)

+
+    // Start with clang-foramt and text-based operation.

[Raymond Toy, 2017/05/10 21:28:43]

What does "Start" mean here?  What will you end with?

[hongchan, 2017/05/11 21:13:02]

Done.

+    Module.runClangFormat(codeString, OPTIONS.ClangFormat, this)
+        .then((formattedCodeString) => {
+            // Replace the original code with clang-formatted code.
+            scriptElement.textContent = formattedCodeString;
+
+            // Then tidy the text data from JSDOM. After this point, DOM
+            // manipulation is not possible anymore.
+            let pageString = this.jsdom_.serialize();
+            pageString = Module.runHTMLTidySync(
+                pageString, OPTIONS.HTMLTidy, this);
+            pageString = Module.runRegExpSwapSync(
+                pageString, OPTIONS.RegExpSwapCollection);
+
+            Module.detectLineOverflow(pageString, this);
+
+            this.finish_(pageString, taskDone);
+        });
+  }
+
+  processJS_ (taskDone) {
+    // The file is a JS code, so run clang-format directly.
+    Module.runClangFormat(this.pageString_, OPTIONS.ClangFormat, this)
+        .then((formattedCodeString) => {
+            formattedCodeString = Module.runRegExpSwapSync(
+                formattedCodeString, [OPTIONS.RegExpSwapCollection[0]]);
+
+            Module.detectLineOverflow(formattedCodeString, this);
+
+            this.finish_(formattedCodeString, taskDone);
+        });
+  }
+
+  finish_ (resultString, taskDone) {
+    // Print out the result to console when 'Dry Run', otherwise save
+    // directly to the target file path.
+    if (this.isDryRun_) {
+      console.log(Bar80.Start + '\n' + resultString + '\n' + Bar80.End);
+    } else {
+      Util.writeStringToFileSync(resultString, this.targetFilePath_);
+    }
+
+    this.printLog();
+    taskDone();
+  }
+
+  /**
+   * Adding log message.
+   * @param {String} location Caller information.
+   * @param {String} message Log message.
+   */
+  addLog (location, message) {
+    if (!this.logs_.hasOwnProperty(location))
+      this.logs_[location] = [];
+    this.logs_[location].push(message);
+  }
+
+  /**
+   * Print log messages at the end of task.
+   */
+  printLog () {
+    console.log('> Logs from: ' + this.targetFilePath_);
+    for (let location in this.logs_) {
+      console.log('  [] ' + location);
+      this.logs_[location].forEach((message) => {
+        if (Array.isArray(message)) {
+          message.forEach((subMessage) => {
+            if (subMessage.length > 0)
+              console.log('     - ' + subMessage);
+          });
+        } else {
+          console.log('     - ' + message);
+        }
+      });
+    }
+  }
+
+}
+
+
+/**
+ * @class  TidyTaskRunner
+ */
+class TidyTaskRunner {
+
+  /**
+   * @param {Array} files A list of file paths.
+   * @param {Boolean} isDryRun Dry run flag. When |true| the processed output
+   *                           will be printed out via stdout instead of actual
+   *                           target files.
+   * @return {TidyTaskRunner} A task runner object.
+   */
+  constructor (files, isDryRun) {
+    this.targetFiles_ = files;
+    this.isDryRun_ = isDryRun;
+
+    this.tasks_ = [];
+    this.currentTask_ = 0;
+    this.oncomplete = null;
+  }
+
+  startProcessing () {
+    this.targetFiles_.forEach((filePath) => {
+      this.tasks_.push(new TidyTask(filePath, this.isDryRun_));
+    });
+
+    console.log('[layout-test-tidy] Task runner started: '
+        + this.targetFiles_.length + ' file(s).');
+    this.runTask_();
+  }
+
+  runTask_ () {
+    console.log('[layout-test-tidy] Running task #' + (this.currentTask_ + 1)
+        + ': ' + this.targetFiles_[this.currentTask_]
+        + (this.isDryRun_ ? ' (DRYRUN)' : ''));
+    this.tasks_[this.currentTask_].run(this.done_.bind(this));
+  }
+
+  done_ () {
+    console.log('[layout-test-tidy] Task #' + (this.currentTask_ + 1)
+        + ' completed.\n');
+    this.currentTask_++;
+    if (this.currentTask_ < this.tasks_.length) {
+      this.runTask_();
+    } else {
+      console.log('[layout-test-tidy] Task runner completed: '
+          + this.targetFiles_.length + ' file(s) processed.');
+      if (this.oncomplete)
+        this.oncomplete();
+    }
+  }
+
+}
+
+
+// Entry point.
+function main() {
+  let args = process.argv.slice(2);
+  if (!args || args.length == 0 || args.length > 2)
+    Util.logAndExit('main', 'Invalid arguments. (' + args + ')');
+
+  let targetPath = args[0];
+  let isDryRun = args[1] === '--dryrun';
+
+  let files = [];
+
+  if (targetPath) {
+    try {
+      let stat = fs.lstatSync(targetPath);
+      if (stat.isFile()) {
+        files.push(targetPath);
+      } else if (stat.isDirectory()) {
+        files = glob.sync(targetPath + '/**/*.{html,js}');
+      }
+    } catch (error) {
+      let errorMessage = 'Invalid file path. (' + targetPath + ')\n'
+          + '  > ' + error.toString();
+      Util.logAndExit('main', errorMessage);
+    }
+  }
+
+  // Files to be skipped.
+  let filesToBeSkipped =
+      Util.loadFileToStringSync('skip-tidy').toString().split('\n');
+  filesToBeSkipped.forEach((fileSkipped) => {
+    let index = files.indexOf(fileSkipped);
+    if (index > -1) {
+      files.splice(index, 1);
+    }
+  });
+
+  if (files.length > 0) {
+    let taskRunner = new TidyTaskRunner(files, isDryRun);
+    taskRunner.startProcessing();
+  } else {
+    Util.logAndExit('main', 'No files to process.');
+  }
+}
+
+main();