feat: warn when clang-format fails (#62)

Fixes #60

Author: kjin

package-lock.json

@@ -26,7 +26,7 @@
     "format-check": "./bin/format-check.sh",
     "format": "clang-format -i -style='{Language: JavaScript, BasedOnStyle: Google, ColumnLimit: 80}' src/*.ts test/*.ts",
     "lint": "tslint -c tslint.json  --project . --type-check -t codeFrame",
-    "lint-fix": "tslint -c tslint.json --project .  --type-check -t codeFrame --fix",
+    "lint-fix": "tslint -c tslint.json --project . --type-check -t codeFrame --fix",
     "prepare": "npm run compile",
     "test": "npm run compile && nyc ava build/test/test*.js",
     "posttest": "npm run lint && npm run format-check"
@@ -38,7 +38,7 @@
   "author": "Google Inc.",
   "license": "Apache-2.0",
   "dependencies": {
-    "chalk": "^2.0.1",
+    "chalk": "^2.2.0",
     "clang-format": "^1.0.53",
     "inquirer": "^3.2.1",
     "meow": "^3.7.0",
@@ -49,7 +49,6 @@
     "write-file-atomic": "^2.1.0"
   },
   "devDependencies": {
-    "@types/chalk": "^0.4.31",
     "@types/glob": "^5.0.30",
     "@types/inquirer": "0.0.35",
     "@types/make-dir": "^1.0.1",

package.json

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import * as chalk from 'chalk';
+import chalk from 'chalk';
 
 import {Options} from './cli';
 import {getTSConfig, rimrafp} from './util';

src/clean.ts

@@ -86,7 +86,9 @@ async function run(verb: string, files: string[]): Promise<boolean> {
   const format: VerbFilesFunction = require('./format').format;
   switch (verb) {
     case 'check':
-      return (await lint(options, files) && await format(options, files));
+      const passLint = await lint(options, files);
+      const passFormat = await format(options, files);
+      return passLint && passFormat;
     case 'fix':
       return (
           await lint(options, files, true) &&

src/cli.ts

@@ -27,7 +27,7 @@ const baseArgs =
  * @param fix whether to automatically fix the format
  * @param files files to format
  */
-export function format(
+export async function format(
     options: Options, files: string[] = [], fix = false): Promise<boolean> {
   const program = createProgram(options);
   const srcFiles = files.length > 0 ?
@@ -36,7 +36,16 @@ export function format(
           .map(sourceFile => sourceFile.fileName)
           .filter(f => !f.endsWith('.d.ts'));
 
-  return fix ? fixFormat(srcFiles) : checkFormat(srcFiles);
+  if (fix) {
+    return fixFormat(srcFiles);
+  } else {
+    const result = await checkFormat(srcFiles);
+    if (!result) {
+      options.logger.log(
+          'clang-format reported errors... run `gts fix` to address.');
+    }
+    return result;
+  }
 }
 
 /**

src/format.ts

@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import * as chalk from 'chalk';
+import chalk from 'chalk';
 import * as cp from 'child_process';
 import * as inquirer from 'inquirer';
 import * as path from 'path';

src/init.ts

@@ -1 +1 @@
-const isThisTypeScript = true
+  const isThisTypeScript = true

test/fixtures/kitchen/src/server.ts

@@ -1,5 +1,5 @@
 import test from 'ava';
-import * as chalk from 'chalk';
+import chalk from 'chalk';
 import * as cp from 'child_process';
 import * as fs from 'fs';
 import * as ncp from 'ncp';
@@ -8,14 +8,36 @@ import * as pify from 'pify';
 import * as rimraf from 'rimraf';
 import * as tmp from 'tmp';
 
+interface ExecResult {
+  exitCode: number;
+  stdout: string;
+  stderr: string;
+}
+
 const pkg = require('../../package.json');
 
 const rimrafp = pify(rimraf);
 const mkdirp = pify(fs.mkdir);
-const execp = pify(cp.exec);
+const simpleExecp = pify(cp.exec);
 const renamep = pify(fs.rename);
 const ncpp = pify(ncp.ncp);
 
+// cp.exec doesn't fit the (err ^ result) pattern because a process can write
+// to stdout/stderr and still exit with error code != 0.
+// In most cases simply promisifying cp.exec is adequate, but it's not if we
+// need to see console output for a process that exited with a non-zero exit
+// code, so we define a more exhaustive promsified cp.exec here.
+const execp =
+    (command: string, execOptions?: cp.ExecOptions): Promise<ExecResult> => {
+      return new Promise((resolve) => {
+        cp.exec(
+            command, execOptions || {},
+            (err: Error&{code: number}, stdout, stderr) => {
+              resolve({exitCode: err ? err.code : 0, stdout, stderr});
+            });
+      });
+    };
+
 const keep = !!process.env.GTS_KEEP_TEMPDIRS;
 const stagingDir = tmp.dirSync({keep: keep, unsafeCleanup: true});
 const stagingPath = stagingDir.name;
@@ -30,43 +52,46 @@ console.log(`${chalk.blue(`${__filename} staging area: ${stagingPath}`)}`);
  * to test on a fresh application.
  */
 test.before(async () => {
-  await execp('npm pack');
+  await simpleExecp('npm pack');
   const tarball = `${pkg.name}-${pkg.version}.tgz`;
   await renamep(tarball, `${stagingPath}/gts.tgz`);
   await ncpp('test/fixtures', `${stagingPath}/`);
-  await execp('npm install', execOpts);
+  await simpleExecp('npm install', execOpts);
 });
 
 test.serial('init', async t => {
-  await execp('./node_modules/.bin/gts init -y', execOpts);
+  await simpleExecp('./node_modules/.bin/gts init -y', execOpts);
   fs.accessSync(`${stagingPath}/kitchen/tsconfig.json`);
   t.pass();
 });
 
 test.serial('check before fix', async t => {
-  await t.throws(execp('npm run check', execOpts));
+  const {exitCode, stdout} = await execp('npm run check', execOpts);
+  t.deepEqual(exitCode, 1);
+  t.notDeepEqual(stdout.indexOf('clang-format reported errors'), -1);
   t.pass();
 });
 
 test.serial('fix', async t => {
   const preFix = fs.readFileSync(`${stagingPath}/kitchen/src/server.ts`, 'utf8')
                      .split('\n');
-  await execp('npm run fix', execOpts);
+  await simpleExecp('npm run fix', execOpts);
   const postFix =
       fs.readFileSync(`${stagingPath}/kitchen/src/server.ts`, 'utf8')
           .split('\n');
   t.deepEqual(
-      preFix[0] + ';', postFix[0]);  // fix should have added a semi-colon
+      preFix[0].trim() + ';',
+      postFix[0]);  // fix should have added a semi-colon
   t.pass();
 });
 
 test.serial('check after fix', async t => {
-  await execp('npm run check', execOpts);
+  await simpleExecp('npm run check', execOpts);
   t.pass();
 });
 
 test.serial('build', async t => {
-  await execp('npm run compile', execOpts);
+  await simpleExecp('npm run compile', execOpts);
   fs.accessSync(`${stagingPath}/kitchen/build/src/server.js`);
   fs.accessSync(`${stagingPath}/kitchen/build/src/server.js.map`);
   fs.accessSync(`${stagingPath}/kitchen/build/src/server.d.ts`);
@@ -78,7 +103,7 @@ test.serial('build', async t => {
  * output dir
  */
 test.serial('clean', async t => {
-  await execp('npm run clean', execOpts);
+  await simpleExecp('npm run clean', execOpts);
   t.throws(() => {
     fs.accessSync(`${stagingPath}/kitchen/build`);
   });