feat: print clang-format output (#186)

google · Jul 17, 2018 · f7c75ee · f7c75ee
1 parent e107ab3
commit f7c75ee
Show file tree

Hide file tree

Showing 4 changed files with 239 additions and 6 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -42,6 +42,8 @@
   "dependencies": {
     "chalk": "^2.4.1",
     "clang-format": "1.2.3",
+    "diff": "^3.5.0",
+    "entities": "^1.1.1",
     "inquirer": "^6.0.0",
     "meow": "^5.0.0",
     "pify": "^3.0.0",
@@ -51,6 +53,8 @@
     "write-file-atomic": "^2.3.0"
   },
   "devDependencies": {
+    "@types/diff": "^3.5.1",
+    "@types/entities": "^1.1.0",
     "@types/glob": "^5.0.35",
     "@types/inquirer": "^0.0.42",
     "@types/make-dir": "^1.0.3",

diff --git a/src/format.ts b/src/format.ts
@@ -13,10 +13,25 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+import chalk from 'chalk';
+import * as jsdiff from 'diff';
+import * as entities from 'entities';
 import * as fs from 'fs';
 import * as path from 'path';
+
 import {Options} from './cli';
 import {createProgram} from './lint';
+import {readFilep} from './util';
+
+/**
+ * Object that contains the position of formatting issue within the file, the
+ * length of it, and the string to replace it with.
+ */
+export interface Replacement {
+  offset: number;
+  length: number;
+  fix: string;
+}
 
 // Exported for testing purposes.
 export const clangFormat = require('clang-format');
@@ -59,7 +74,7 @@ export async function format(
   if (fix) {
     return fixFormat(srcFiles, baseClangFormatArgs);
   } else {
-    const result = await checkFormat(srcFiles, baseClangFormatArgs);
+    const result = await checkFormat(options, srcFiles, baseClangFormatArgs);
     if (!result) {
       options.logger.log(
           'clang-format reported errors... run `gts fix` to address.');
@@ -92,7 +107,8 @@ function fixFormat(srcFiles: string[], baseArgs: string[]): Promise<boolean> {
  *
  * @param srcFiles list of source files
  */
-function checkFormat(srcFiles: string[], baseArgs: string[]): Promise<boolean> {
+function checkFormat(options: Options, srcFiles: string[], baseArgs: string[]):
+    Promise<boolean> {
   return new Promise<boolean>((resolve, reject) => {
     let output = '';
     const args = baseArgs.concat(['-output-replacements-xml'], srcFiles);
@@ -110,8 +126,138 @@ function checkFormat(srcFiles: string[], baseArgs: string[]): Promise<boolean> {
     out.on('data', (data: Buffer) => {
       output += data;
     });
-    out.on('end', () => {
+
+    out.on('end', async () => {
+      const files: string[] =
+          output.split('<?xml version=\'1.0\'?>\n').slice(1);
+      for (let i = 0; i < files.length; i++) {
+        if (files[i].indexOf('<replacement ') === -1) {
+          continue;
+        }
+        const replacements = getReplacements(files[i]);
+        if (replacements.length > 0) {
+          const diff = await getDiffObj(srcFiles[i], replacements);
+          printDiffs(diff, options);
+        }
+      }
       resolve(output.indexOf('<replacement ') === -1 ? true : false);
     });
   });
 }
+
+/**
+ * Parses through xml string for the replacement string, offset, length and
+ * returns all of the necessary replacements for the file
+ *
+ * @param output xml string from clangFormat
+ */
+export function getReplacements(fileXML: string): Replacement[] {
+  const replacements: Replacement[] = [];
+
+  let xmlLines = fileXML.trim().split('\n');
+  // first and last elements are outer 'replacements' tags
+  xmlLines = xmlLines.slice(1, xmlLines.length - 1);
+
+  for (let i = 0; i < xmlLines.length; i++) {
+    // Uses regex to capture the xml attributes and element
+    // XML format:
+    // <replacement offset='OFFSET' length='LENGTH'>FIX</replacement>
+    const offset: string[]|null = (/offset=\'(\d+)\'/g).exec(xmlLines[i]);
+    const length: string[]|null = (/length=\'(\d+)\'/g).exec(xmlLines[i]);
+    const fix: string[]|null =
+        (/length=\'\d+\'>(.*)<\/replacement>/g).exec(xmlLines[i]);
+
+    if (length === null || offset === null || fix === null) {
+      throw new Error('Unable to get replacement');
+    }
+    replacements[i] = {
+      offset: Number(offset[1]),
+      length: Number(length[1]),
+      fix: entities.decodeXML(fix[1])
+    };
+  }
+  return replacements;
+}
+
+/**
+ * Gets an object containing the differences between the original file and after
+ * changes have been made
+ *
+ * @param file
+ * @param replacements array of all the formatting issues within in the file
+ */
+async function getDiffObj(
+    file: string, replacements: Replacement[]): Promise<jsdiff.IUniDiff> {
+  const text = await readFilep(file, 'utf8');
+  const fixed = performFixes(text, replacements);
+  const diff =
+      jsdiff.structuredPatch(file, '', text, fixed, '', '', {context: 3});
+  jsdiff.applyPatch('diff', diff);
+  return diff;
+}
+
+/**
+ * Performs formatting fixes to the original string
+ *
+ * @param data original string
+ * @param errOffset start index of the formatting issue
+ * @param errLength length of formatting issue
+ * @param replacements string that resolves formatting issue
+ */
+function performFixes(data: string, replacements: Replacement[]) {
+  const replaced: string[] = [];
+  replaced.push(substring(data, 0, replacements[0].offset));
+  for (let i = 0; i < replacements.length - 1; i++) {
+    replaced.push(replacements[i].fix);
+    replaced.push(substring(
+        data, replacements[i].offset + replacements[i].length,
+        replacements[i + 1].offset));
+  }
+  replaced.push(replacements[replacements.length - 1].fix);
+  replaced.push(substring(
+      data,
+      replacements[replacements.length - 1].offset +
+          replacements[replacements.length - 1].length,
+      Buffer.byteLength(data, 'utf8')));
+  return replaced.join('');
+}
+
+/**
+ * Prints the lines with formatting issues
+ *
+ * @param diffs contains all information about the formatting changes
+ * @param options
+ */
+function printDiffs(diffs: jsdiff.IUniDiff, options: Options) {
+  options.logger.log(chalk.inverse.bold(diffs.oldFileName));
+  diffs.hunks.forEach((diff: JsDiff.IHunk) => {
+    const log = `  Lines: ${diff.oldStart}-${diff.oldStart + diff.oldLines}`;
+    options.logger.log(chalk.bold(log));
+
+    diff.lines.forEach((line: string) => {
+      if (line[0] === '-') {
+        options.logger.log(`   ${chalk.red(line)}`);
+      } else if (line[0] === '+') {
+        options.logger.log(`   ${chalk.green(line)}`);
+      } else {
+        options.logger.log(`   ${chalk.gray(line)}`);
+      }
+    });
+    options.logger.log('\n');
+  });
+}
+
+/**
+ * Substring using bytes
+ *
+ * @param str original string
+ * @param indexStart position where to start extraction
+ * @param indexEnd position (up to, but not including) where to end extraction
+ * @param encoding
+ */
+function substring(
+    str: string, indexStart: number, indexEnd: number, encoding = 'utf8') {
+  return Buffer.from(str, encoding)
+      .slice(indexStart, indexEnd)
+      .toString(encoding);
+}
diff --git a/test/test-format.ts b/test/test-format.ts
@@ -25,8 +25,11 @@ import {nop} from '../src/util';
 import {withFixtures} from './fixtures';
 
 // clang-format won't pass this code because of trailing spaces.
-const BAD_CODE = 'export const foo = [ 2 ];';
-const GOOD_CODE = 'export const foo = [2];';
+const BAD_CODE = 'export const foo = [ \"2\" ];';
+const GOOD_CODE = 'export const foo = [\'2\'];';
+
+const CLANG_FORMAT_MESSAGE =
+    'clang-format reported errors... run `gts fix` to address.';
 
 const OPTIONS: Options = {
   gtsRootDir: path.resolve(__dirname, '../..'),
@@ -94,7 +97,7 @@ test.serial('format should format files listed in tsconfig.files', t => {
 });
 
 test.serial(
-    'format should format *.ts files when no files or inlcude has been specified',
+    'format should format *.ts files when no files or include has been specified',
     async t => {
       return withFixtures(
           {
@@ -210,3 +213,66 @@ test.serial('format should return error from failed spawn', async t => {
         format.clangFormat.spawnClangFormat = original;
       });
 });
+
+test.serial(
+    'format should print suggestions for fixes for ill-formatted file', t => {
+      return withFixtures(
+          {
+            'tsconfig.json': JSON.stringify({files: ['a.ts']}),
+            'a.ts': BAD_CODE
+          },
+          async () => {
+            let output = '';
+            const newLogger = Object.assign({}, OPTIONS.logger, {
+              log: (n: string) => {
+                output += n;
+              }
+            });
+            const options = Object.assign({}, OPTIONS, {logger: newLogger});
+
+            await format.format(options, [], false);
+            t.true(output.search(CLANG_FORMAT_MESSAGE) !== -1);
+            t.true(output.indexOf('+export const foo = [\'2\'];') !== -1);
+            t.true(output.indexOf('-export const foo = [ \"2\" ];') !== -1);
+          });
+    });
+
+test.serial('formatting output should display unicode characters correctly', t => {
+  return withFixtures(
+      {
+
+        'tsconfig.json': JSON.stringify({files: ['a.ts']}),
+        'a.ts':
+            '//🦄 This is a comment 🌷🏳️‍🌈	— /\nconst variable =    \'5\''
+      },
+      async () => {
+        let output = '';
+        const newLogger = Object.assign({}, OPTIONS.logger, {
+          log: (n: string) => {
+            output += n;
+          }
+        });
+        const options = Object.assign({}, OPTIONS, {logger: newLogger});
+
+        await format.format(options, [], false);
+        t.true(output.search(CLANG_FORMAT_MESSAGE) !== -1);
+        t.true(
+            output.indexOf(
+                '//🦄 This is a comment 🌷🏳️‍🌈	—') !== -1);
+        t.true(output.indexOf('const variable = \'5\'') !== -1);
+      });
+});
+
+test.serial(
+    'should throw error if xml are missing offset, length, or fix values',
+    t => {
+      return withFixtures({}, async () => {
+        const missingLength =
+            '<?xml version=\'1.0\'?>\n<replacements xml:space=\'' +
+            'preserve\' incomplete_format=\'false\'>\n<replacement ' +
+            'offset=\'8\' length=\'\'>FIX</replacement></replacements>';
+        t.throws(() => {
+          format.getReplacements(missingLength);
+        });
+      });
+    });