Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: warn when clang-format fails #62

Merged
merged 6 commits into from
Oct 19, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 3 additions & 1 deletion src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) &&
Expand Down
13 changes: 11 additions & 2 deletions src/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?
Expand All @@ -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;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/kitchen/src/server.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
const isThisTypeScript = true
const isThisTypeScript = true
40 changes: 30 additions & 10 deletions test/test-kitchen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,31 @@ 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);

const execp =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deserves a comment. Also, we should probably look for an ecosystem module for 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;
Expand All @@ -30,43 +47,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`);
Expand All @@ -78,7 +98,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`);
});
Expand Down