Skip to content

Commit 287c127

Browse files
alangpiercealangpierce
authored
feat: rework handling of untracked files (#123)
Fixes #98 Fixes #120 Untracked files no longer block bulk-decaffeinate from running. But they do show a warning since it might indicate a mistake. Similarly, when discovering files, we skip any untracked files that would otherwise be converted, and we show a warning in that case (hopefully untracked CoffeeScript files are rare anyway). If an untracked files are explicitly specified for conversion, we now fail with a friendlier error than before.
1 parent 36b12e8 commit 287c127

10 files changed

Lines changed: 78 additions & 50 deletions

‎src/config/getFilesToProcess.js‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { resolve } from 'path';
33

44
import getFilesFromPathFile from './getFilesFromPathFile';
55
import getFilesUnderPath from '../util/getFilesUnderPath';
6+
import getTrackedFiles from '../util/getTrackedFiles';
67
import { shouldConvertFile, isExtensionless, jsPathFor } from '../util/FilePaths';
78
import CLIError from '../util/CLIError';
89

@@ -16,7 +17,9 @@ export default async function getFilesToProcess(config) {
1617
async function resolveFilesToProcess(config) {
1718
let {filesToProcess, pathFile, searchDirectory} = config;
1819
if (!filesToProcess && !pathFile && !searchDirectory) {
19-
return await getFilesUnderPath('.', shouldConvertFile);
20+
let trackedFiles = await getTrackedFiles();
21+
return await getFilesUnderPath('.', async (path) =>
22+
await shouldConvertFile(path, trackedFiles));
2023
}
2124
let files = [];
2225
if (filesToProcess) {
@@ -26,7 +29,9 @@ async function resolveFilesToProcess(config) {
2629
files.push(...await getFilesFromPathFile(pathFile));
2730
}
2831
if (searchDirectory) {
29-
files.push(...await getFilesUnderPath(searchDirectory, shouldConvertFile));
32+
let trackedFiles = await getTrackedFiles();
33+
files.push(...await getFilesUnderPath(searchDirectory, async (path) =>
34+
await shouldConvertFile(path, trackedFiles)));
3035
}
3136
files = files.map(path => resolve(path));
3237
files = Array.from(new Set(files)).sort();
@@ -41,7 +46,11 @@ function resolveFileFilter(filesToProcess, config) {
4146
}
4247

4348
async function validateFilesToProcess(filesToProcess, config) {
49+
let trackedFiles = await getTrackedFiles();
4450
for (let path of filesToProcess) {
51+
if (!trackedFiles.has(path)) {
52+
throw new CLIError(`The file ${path} is not tracked in the git repo.`);
53+
}
4554
if (isExtensionless(path)) {
4655
continue;
4756
}

‎src/convert.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import CLIError from './util/CLIError';
1212
import execLive from './util/execLive';
1313
import { backupPathFor, decaffeinateOutPathFor, jsPathFor } from './util/FilePaths';
1414
import getFilesUnderPath from './util/getFilesUnderPath';
15-
import isWorktreeEmpty from './util/isWorktreeEmpty';
1615
import makeCommit from './util/makeCommit';
1716
import pluralize from './util/pluralize';
1817

@@ -179,10 +178,17 @@ Re-run with the "check" command for more details.`);
179178
}
180179

181180
async function assertGitWorktreeClean() {
182-
if (!await isWorktreeEmpty()) {
181+
let status = await git().status();
182+
if (status.files.length > status.not_added.length) {
183183
throw new CLIError(`\
184184
You have modifications to your git worktree.
185185
Please revert or commit them before running convert.`);
186+
} else if (status.not_added.length > 0) {
187+
console.log(`\
188+
Warning: the following untracked files are present in your repository:
189+
${status.not_added.join('\n')}
190+
Proceeding anyway.
191+
`);
186192
}
187193
}
188194

‎src/util/FilePaths.js‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,24 @@ function basePathFor(path) {
1616
return join(dirname(path), basename(path, extension));
1717
}
1818

19-
export async function shouldConvertFile(path) {
20-
if (COFFEE_EXTENSIONS.some(ext =>
21-
path.endsWith(ext) && !path.endsWith(`.original${ext}`))) {
22-
return true;
19+
export async function shouldConvertFile(path, trackedFiles) {
20+
if (!hasCoffeeExtension(path) && !await isExecutableScript(path)) {
21+
return false;
2322
}
23+
if (!trackedFiles.has(path)) {
24+
console.log(
25+
`Warning: Skipping ${path} because the file is not tracked in the git repo.`);
26+
return false;
27+
}
28+
return true;
29+
}
30+
31+
function hasCoffeeExtension(path) {
32+
return COFFEE_EXTENSIONS.some(ext =>
33+
path.endsWith(ext) && !path.endsWith(`.original${ext}`));
34+
}
35+
36+
async function isExecutableScript(path) {
2437
if (isExtensionless(path) && await executable(path)) {
2538
let contents = await readFile(path);
2639
let firstLine = contents.toString().split('\n')[0];

‎src/util/getFilesUnderPath.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { readdir, stat } from 'mz/fs';
2-
import { join } from 'path';
2+
import { join, resolve } from 'path';
33

44
/**
55
* Recursively discover any matching files in the current directory, ignoring
@@ -12,7 +12,7 @@ export default async function getFilesUnderPath(dirPath, asyncPathPredicate) {
1212
if (['node_modules', '.git'].includes(child)) {
1313
continue;
1414
}
15-
let childPath = join(dirPath, child);
15+
let childPath = resolve(join(dirPath, child));
1616
if ((await stat(childPath)).isDirectory()) {
1717
let subdirCoffeeFiles = await getFilesUnderPath(childPath, asyncPathPredicate);
1818
resultFiles.push(...subdirCoffeeFiles);

‎src/util/getTrackedFiles.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import git from 'simple-git/promise';
2+
import { resolve } from 'path';
3+
4+
export default async function getTrackedFiles() {
5+
let stdout = await git().raw(['ls-files']);
6+
return new Set(stdout.split('\n').map(s => s.trim()).map(s => resolve(s)));
7+
}

‎src/util/isWorktreeEmpty.js‎

Lines changed: 0 additions & 9 deletions
This file was deleted.

‎test/bulk-decaffeinate-test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
assertFileContents,
77
assertFileIncludes,
88
assertIncludes,
9-
initGitRepo,
109
runCli,
1110
runCliExpectSuccess,
1211
runWithTemplateDir,
@@ -24,7 +23,6 @@ describe('basic CLI', () => {
2423
describe('config', () => {
2524
it('allows explicitly-specified config files', async function() {
2625
await runWithTemplateDir('custom-config-location', async function() {
27-
await initGitRepo();
2826
await runCliExpectSuccess(
2927
'convert --config configDir1/config.js --config configDir2/otherConfig.js');
3028
await assertFileContents('./A.ts', `\

‎test/convert-test.js‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
import assert from 'assert';
33
import { exec } from 'mz/child_process';
44
import { readFile, writeFile } from 'mz/fs';
5+
import git from 'simple-git/promise';
56

67
import {
78
assertExists,
89
assertNotExists,
910
assertFileContents,
1011
assertIncludes,
11-
initGitRepo,
1212
runCli,
1313
runCliExpectSuccess,
1414
runCliExpectError,
@@ -18,7 +18,6 @@ import {
1818
describe('convert', () => {
1919
it('generates git commits converting the files', async function() {
2020
await runWithTemplateDir('simple-success', async function() {
21-
await initGitRepo();
2221
await runCliExpectSuccess('convert');
2322

2423
let logStdout = (await exec('git log --pretty="%an <%ae> %s"'))[0];
@@ -34,7 +33,6 @@ Sample User <sample@example.com> Initial commit
3433

3534
it('generates a nice commit message when converting just one file', async function() {
3635
await runWithTemplateDir('simple-success', async function() {
37-
await initGitRepo();
3836
await runCliExpectSuccess('convert --file ./A.coffee');
3937
let logStdout = (await exec('git log --pretty="%an <%ae> %s"'))[0];
4038
assert.equal(logStdout, `\
@@ -49,7 +47,6 @@ Sample User <sample@example.com> Initial commit
4947

5048
it('generates a nice commit message when converting three files', async function() {
5149
await runWithTemplateDir('file-list', async function () {
52-
await initGitRepo();
5350
await runCliExpectSuccess('convert --path-file ./files-to-decaffeinate.txt');
5451
let logStdout = (await exec('git log --pretty="%an <%ae> %s"'))[0];
5552
assert.equal(logStdout, `\
@@ -64,7 +61,6 @@ Sample User <sample@example.com> Initial commit
6461

6562
it('combines multiple path specifiers', async function() {
6663
await runWithTemplateDir('multiple-path-specifiers', async function () {
67-
await initGitRepo();
6864
await runCliExpectSuccess('convert');
6965
await assertExists('./A.js');
7066
await assertExists('./B.js');
@@ -79,7 +75,6 @@ Sample User <sample@example.com> Initial commit
7975

8076
it('converts literate coffeescript', async function() {
8177
await runWithTemplateDir('literate-coffeescript', async function () {
82-
await initGitRepo();
8378
await runCliExpectSuccess('convert');
8479
await assertFileContents('./A.js', `\
8580
/* eslint-disable
@@ -112,7 +107,6 @@ let c = 1;
112107

113108
it('runs jscodeshift', async function() {
114109
await runWithTemplateDir('jscodeshift-test', async function() {
115-
await initGitRepo();
116110
await runCliExpectSuccess('convert');
117111
await assertFileContents('./A.js', `\
118112
/* eslint-disable
@@ -128,7 +122,6 @@ let notChanged = 4;
128122

129123
it('runs built-in jscodeshift scripts', async function() {
130124
await runWithTemplateDir('builtin-jscodeshift-script', async function() {
131-
await initGitRepo();
132125
await runCliExpectSuccess('convert');
133126
await assertFileContents('./Func.js', `\
134127
/* eslint-disable
@@ -172,7 +165,6 @@ return;
172165

173166
it('prepends "eslint-env mocha" when specified', async function() {
174167
await runWithTemplateDir('mocha-env-test', async function () {
175-
await initGitRepo();
176168
await runCliExpectSuccess('convert');
177169
await assertFileContents('./A.js', `\
178170
// TODO: This file was created by bulk-decaffeinate.
@@ -191,7 +183,6 @@ console.log('This is test code');
191183

192184
it('prepends a custom prefix specified', async function() {
193185
await runWithTemplateDir('code-prefix-test', async function () {
194-
await initGitRepo();
195186
await runCliExpectSuccess('convert');
196187
await assertFileContents('./A.js', `\
197188
/** @babel */
@@ -204,7 +195,6 @@ console.log('This is a file');
204195

205196
it('respects decaffeinate args', async function() {
206197
await runWithTemplateDir('decaffeinate-args-test', async function () {
207-
await initGitRepo();
208198
await runCliExpectSuccess('convert');
209199
await assertFileContents('./A.js', `\
210200
/* eslint-disable
@@ -221,7 +211,6 @@ module.exports = c;
221211

222212
it('allows converting extensionless scripts', async function() {
223213
await runWithTemplateDir('extensionless-script', async function () {
224-
await initGitRepo();
225214
await runCliExpectSuccess('convert');
226215
await assertFileContents('./runThing', `\
227216
#!/usr/bin/env node
@@ -239,7 +228,6 @@ console.log('Ran the thing!');
239228
let untouchedContents2 = (await readFile('./executableScriptWithWrongShebang')).toString();
240229
let untouchedContents3 = (await readFile('./nonExecutableScript')).toString();
241230

242-
await initGitRepo();
243231
await runCliExpectSuccess('convert');
244232

245233
await assertFileContents('./executableScript', `\
@@ -264,7 +252,6 @@ console.log('This nested script is executable so it should be converted.');
264252

265253
it('allows converting a directory with no files', async function() {
266254
await runWithTemplateDir('empty-directory', async function () {
267-
await initGitRepo();
268255
let {stdout, stderr} = await runCli('convert');
269256
assert(stderr.length === 0, `Nonempty stderr. stderr:\n${stderr}\n\nstdout:\n${stdout}`);
270257
assertIncludes(stdout, 'There were no CoffeeScript files to convert.');
@@ -273,7 +260,6 @@ console.log('This nested script is executable so it should be converted.');
273260

274261
it('runs eslint, applying fixes and disabling existing issues', async function() {
275262
await runWithTemplateDir('eslint-fix-test', async function() {
276-
await initGitRepo();
277263
await runCliExpectSuccess('convert');
278264
await assertFileContents('./A.js', `\
279265
/* eslint-disable
@@ -291,15 +277,13 @@ console.log(x);
291277

292278
it('fails when .coffee and .js files both exist', async function() {
293279
await runWithTemplateDir('existing-js-file', async function() {
294-
await initGitRepo();
295280
let message = await runCliExpectError('convert');
296-
assertIncludes(message, 'The file A.js already exists.');
281+
assertIncludes(message, 'A.js already exists.');
297282
});
298283
});
299284

300-
it('fails when the git worktree has changes', async function() {
285+
it('fails when the git worktree has changes, staged or unstaged', async function() {
301286
await runWithTemplateDir('simple-success', async function() {
302-
await initGitRepo();
303287
await exec('echo "x = 2" >> A.coffee');
304288
let message = await runCliExpectError('convert');
305289
assertIncludes(message, 'You have modifications to your git worktree.');
@@ -309,9 +293,36 @@ console.log(x);
309293
});
310294
});
311295

296+
it('warns when the git worktree has untracked changes', async function() {
297+
await runWithTemplateDir('simple-success', async function() {
298+
await exec('echo "x = 2" >> new-file.coffee');
299+
await exec('echo "x = 2" >> other-new-file.coffee');
300+
let {stdout} = await runCliExpectSuccess('convert');
301+
assertIncludes(stdout, `\
302+
Warning: the following untracked files are present in your repository:
303+
new-file.coffee
304+
other-new-file.coffee
305+
Proceeding anyway.`);
306+
let status = await git().status();
307+
assert.deepEqual(status.not_added, [
308+
'A.original.coffee',
309+
'B.original.coffee',
310+
'new-file.coffee',
311+
'other-new-file.coffee',
312+
]);
313+
});
314+
});
315+
316+
it('errors when explicitly specifying an untracked file', async function() {
317+
await runWithTemplateDir('simple-success', async function() {
318+
await exec('echo "x = 2" >> new-file.coffee');
319+
let message = await runCliExpectError('convert -f ./new-file.coffee');
320+
assertIncludes(message, 'is not tracked in the git repo.');
321+
});
322+
});
323+
312324
it('generates backup files that are removed by clean', async function() {
313325
await runWithTemplateDir('backup-files', async function() {
314-
await initGitRepo();
315326
await runCli('convert');
316327
await assertExists('./A.original.coffee');
317328
await assertExists('./B.original.coffee.md');
@@ -325,7 +336,6 @@ console.log(x);
325336

326337
it('properly handles custom names', async function() {
327338
await runWithTemplateDir('custom-file-names', async function() {
328-
await initGitRepo();
329339
await runCli('convert');
330340
await assertExists('./A.js');
331341
await assertExists('./dir/B.ts');
@@ -335,15 +345,13 @@ console.log(x);
335345

336346
it('allows a custom file extension', async function() {
337347
await runWithTemplateDir('convert-to-typescript', async function() {
338-
await initGitRepo();
339348
await runCli('convert');
340349
await assertExists('./A.ts');
341350
});
342351
});
343352

344353
it('handles a missing eslint config', async function() {
345354
await runWithTemplateDir('simple-success', async function() {
346-
await initGitRepo();
347355
let cliResult;
348356
try {
349357
await exec('mv ../../../.eslintrc ../../../.eslintrc.backup');
@@ -358,7 +366,6 @@ console.log(x);
358366

359367
it('bypasses git commit hooks', async function() {
360368
await runWithTemplateDir('simple-success', async function() {
361-
await initGitRepo();
362369
await writeFile('.git/hooks/commit-msg', '#!/bin/sh\nexit 1');
363370
await exec('chmod +x .git/hooks/commit-msg');
364371
await runCliExpectSuccess('convert');
@@ -368,14 +375,12 @@ console.log(x);
368375

369376
it('allows invalid constructors when specified', async function() {
370377
await runWithTemplateDir('invalid-subclass-constructor', async function() {
371-
await initGitRepo();
372378
await runCliExpectSuccess('convert --allow-invalid-constructors');
373379
});
374380
});
375381

376382
it('does not allow invalid constructors when not specified', async function() {
377383
await runWithTemplateDir('invalid-subclass-constructor', async function() {
378-
await initGitRepo();
379384
let message = await runCliExpectError('convert');
380385
assertIncludes(message, 'Some files could not be converted with decaffeinate');
381386
});

‎test/fix-imports-test.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import { exec } from 'mz/child_process';
55
import {
66
assertFilesEqual,
77
assertIncludes,
8-
initGitRepo,
98
runCli,
109
runWithTemplateDir,
1110
} from './test-util';
@@ -16,7 +15,6 @@ describe('fix-imports', () => {
1615
await runWithTemplateDir(dirName, async function () {
1716
// We intentionally call the files ".js.expected" so that jscodeshift
1817
// doesn't discover and try to convert them.
19-
await initGitRepo();
2018
let {stdout, stderr} = await runCli('convert');
2119
assertIncludes(stdout, 'Fixing any imports across the whole codebase');
2220
assert.equal(stderr, '');

0 commit comments

Comments
 (0)