Skip to content

Commit

Permalink
fix: fix with --dry-run should not modify files (#169)
Browse files Browse the repository at this point in the history
Fixes: #118
  • Loading branch information
ofrobots committed Jun 8, 2018
1 parent 09ca073 commit 9a1bfdd
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/lint.ts
Expand Up @@ -24,10 +24,16 @@ import {Options} from './cli';
* Run tslint with the default configuration. Returns true on success.
* @param options gts options
* @param files files to run linter on
* @param fix automatically fix linter errors
* @param fix automatically fix linter errors. Ignored when options.dryRun is
* set.
*/
export function lint(
options: Options, files: string[] = [], fix = false): boolean {
if (options.dryRun && fix) {
options.logger.log('lint: skipping auto fix since --dry-run was passed');
fix = false;

This comment has been minimized.

Copy link
@vitkarpov

vitkarpov Feb 22, 2019

Contributor

@ofrobots hey there, I've got a question out of curiosity. Why this fix is being set? Isn't it in local scope anyway?

This comment has been minimized.

Copy link
@ofrobots

ofrobots Feb 22, 2019

Author Contributor

Hi @vitkarpov. This code is dealing with the case where we get conflicting values of options.dryRun and fix. One way to deal with this would have been to add another variable called shouldFix and set its value to fix && !options.dryRun. For brevity, we simply modify the (locally scoped) value of fix.

I hope this answers your question. Apologies if I misunderstood what you were trying to ask.

This comment has been minimized.

Copy link
@vitkarpov

vitkarpov Feb 23, 2019

Contributor

Yep, it perfectly does. I just realized that there's no return over here, and there's a bunch of code below (green diff of this commit confused me) so fix is being used there. Thanks for your explanation and sorry for a silly question :-)

}

if (files.length > 0) { // manually provided filenames.
const rcs = files.map(file => {
// Different config files may apply to each file.
Expand Down
55 changes: 55 additions & 0 deletions test/test-lint.ts
Expand Up @@ -15,6 +15,7 @@
*/

import test from 'ava';
import fs from 'fs';
import * as path from 'path';

import {Options} from '../src/cli';
Expand All @@ -35,6 +36,10 @@ const OPTIONS: Options = {
const BAD_CODE = `throw 'hello world';`;
const GOOD_CODE = `throw new Error('hello world');`;

// missing semicolon, array-type simple.
const FIXABLE_CODE = 'const x : Array<string> = []';
const FIXABLE_CODE_FIXED = 'const x : string[] = [];';

test.serial('createProgram should return an object', async t => {
await withFixtures({'tsconfig.json': '{}'}, async () => {
const program = lint.createProgram(OPTIONS);
Expand Down Expand Up @@ -66,6 +71,37 @@ test.serial('lint should return false on bad code', async t => {
});
});

test.serial('lint should auto fix fixable errors', async t => {
await withFixtures(
{
'tsconfig.json': JSON.stringify({files: ['a.ts']}),
'a.ts': FIXABLE_CODE
},
async (fixturesDir) => {
const okay = lint.lint(OPTIONS, [], true);
t.is(okay, true);
const contents =
fs.readFileSync(path.join(fixturesDir, 'a.ts'), 'utf8');
t.deepEqual(contents, FIXABLE_CODE_FIXED);
});
});

test.serial('lint should not auto fix on dry-run', async t => {
await withFixtures(
{
'tsconfig.json': JSON.stringify({files: ['a.ts']}),
'a.ts': FIXABLE_CODE
},
async (fixturesDir) => {
const optionsWithDryRun = Object.assign({}, OPTIONS, {dryRun: true});
const okay = lint.lint(optionsWithDryRun, [], true);
t.is(okay, false);
const contents =
fs.readFileSync(path.join(fixturesDir, 'a.ts'), 'utf8');
t.deepEqual(contents, FIXABLE_CODE);
});
});

test.serial('lint should lint files listed in tsconfig.files', async t => {
await withFixtures(
{
Expand Down Expand Up @@ -94,6 +130,25 @@ test.serial(
});
});

test.serial(
'lint should lint files listed in tsconfig.files when empty list is provided',
async t => {
await withFixtures(
{
'tsconfig.json': JSON.stringify({files: ['a.ts']}),
'a.ts': FIXABLE_CODE,
'b.ts': BAD_CODE
},
async (fixturesDir) => {
const okay = lint.lint(OPTIONS, [], true);
t.is(okay, true);
const contents =
fs.readFileSync(path.join(fixturesDir, 'a.ts'), 'utf8');
t.deepEqual(contents, FIXABLE_CODE_FIXED);
});
});


test.serial('lint should not lint files listed in exclude', async t => {
await withFixtures(
{
Expand Down

0 comments on commit 9a1bfdd

Please sign in to comment.