feat: add a modernize-js command to update existing JS code (#126)

This has a few pieces:

* We now need the file discovery process to be flexible enough to discover
  either CoffeeScript or JavaScript.
* In order to avoid duplicate comments, we now have a step to carefully remove
  any comments that look like they were previously auto-generated with
  bulk-decaffeinate. This is nice because it also means you can run modernize to
  get a fresh list of the lint failures.
* The command itself mostly just needs to call functions previously extracted
  from the convert command, and also specify `--modernize-js` when running
  decaffeinate.

.eslintignore

@@ -1 +1,2 @@
 test/tmp-projects/**/*.js
+test/examples/**/*.js

README.md

@@ -71,7 +71,10 @@ bulk-decaffeinate supports a number of commands:
   page with your source code encoded in the hash fragment of the URL. Because it
   is in the hash fragment and not a regular query param, your code is never sent
   to the server.
-* `convert` actually converts the files from CofeeScript to JavaScript.
+* `convert` actually converts the files from CofeeScript to JavaScript,
+  generating a commit for each intermediate step.
+* `modernize-js` runs only the JS-to-JS transformations on the specified
+  JavaScript files. Unlike `convert`, this command does not create a git commit.
 * `clean` deletes all files with ".original" in the name in the current
   directory or any of its subdirectories.
 * `land` packages multiple commits into a merge commit based on an remote branch

package.json

@@ -44,7 +44,7 @@
     "babel-preset-es2015": "^6.13.0",
     "babel-register": "^6.11.6",
     "babelrc-rollup": "^3.0.0",
-    "decaffeinate": "^2.54.2",
+    "decaffeinate": "^2.55.0",
     "eslint": "^3.18.0",
     "eslint-plugin-babel": "^4.0.0",
     "jscodeshift": "^0.3.30",

src/check.js

@@ -3,10 +3,11 @@ import { writeFile } from 'mz/fs';
 import getFilesToProcess from './config/getFilesToProcess';
 import makeDecaffeinateVerifyFn from './runner/makeDecaffeinateVerifyFn';
 import runWithProgressBar from './runner/runWithProgressBar';
+import { COFFEE_FILE_RECOGNIZER } from './util/FilePaths';
 import pluralize from './util/pluralize';
 
 export default async function check(config) {
-  let filesToProcess = await getFilesToProcess(config);
+  let filesToProcess = await getFilesToProcess(config, COFFEE_FILE_RECOGNIZER);
   let decaffeinateResults = await runWithProgressBar(
     `Doing a dry run of decaffeinate on ${pluralize(filesToProcess.length, 'file')}...`,
     filesToProcess, makeDecaffeinateVerifyFn(config),

src/cli.js

@@ -6,6 +6,7 @@ import clean from './clean';
 import resolveConfig from './config/resolveConfig';
 import convert from './convert';
 import land from './land';
+import modernizeJS from './modernizeJS';
 import CLIError from './util/CLIError';
 import viewErrors from './viewErrors';
 
@@ -73,6 +74,9 @@ async function runCommand(command) {
     } else if (command === 'convert') {
       let config = await resolveConfig(commander);
       await convert(config);
+    } else if (command === 'modernize-js') {
+      let config = await resolveConfig(commander);
+      await modernizeJS(config);
     } else if (command === 'view-errors') {
       await viewErrors();
     } else if (command === 'clean') {

src/config/getFilesToProcess.js

@@ -4,22 +4,27 @@ import { resolve } from 'path';
 import getFilesFromPathFile from './getFilesFromPathFile';
 import getFilesUnderPath from '../util/getFilesUnderPath';
 import getTrackedFiles from '../util/getTrackedFiles';
-import { shouldConvertFile, isExtensionless, jsPathFor } from '../util/FilePaths';
+import { shouldConvertFile, jsPathFor } from '../util/FilePaths';
 import CLIError from '../util/CLIError';
 
-export default async function getFilesToProcess(config) {
-  let filesToProcess = await resolveFilesToProcess(config);
+/**
+ * Get the files that we should process based on the config. "recognizer" should
+ * be an object describing the files to auto-recognize, e.g.
+ * COFFEE_FILE_RECOGNIZER.
+ */
+export default async function getFilesToProcess(config, recognizer) {
+  let filesToProcess = await resolveFilesToProcess(config, recognizer);
   filesToProcess = resolveFileFilter(filesToProcess, config);
   await validateFilesToProcess(filesToProcess, config);
   return filesToProcess;
 }
 
-async function resolveFilesToProcess(config) {
+async function resolveFilesToProcess(config, recognizer) {
   let {filesToProcess, pathFile, searchDirectory} = config;
   if (!filesToProcess && !pathFile && !searchDirectory) {
     let trackedFiles = await getTrackedFiles();
     return await getFilesUnderPath('.', async (path) =>
-      await shouldConvertFile(path, trackedFiles));
+      await shouldConvertFile(path, recognizer, trackedFiles));
   }
   let files = [];
   if (filesToProcess) {
@@ -31,7 +36,7 @@ async function resolveFilesToProcess(config) {
   if (searchDirectory) {
     let trackedFiles = await getTrackedFiles();
     files.push(...await getFilesUnderPath(searchDirectory, async (path) =>
-      await shouldConvertFile(path, trackedFiles)));
+      await shouldConvertFile(path, recognizer, trackedFiles)));
   }
   files = files.map(path => resolve(path));
   files = Array.from(new Set(files)).sort();
@@ -51,11 +56,8 @@ async function validateFilesToProcess(filesToProcess, config) {
     if (!trackedFiles.has(path)) {
       throw new CLIError(`The file ${path} is not tracked in the git repo.`);
     }
-    if (isExtensionless(path)) {
-      continue;
-    }
     let jsPath = jsPathFor(path, config);
-    if (await exists(jsPath)) {
+    if (jsPath !== path && await exists(jsPath)) {
       throw new CLIError(`The file ${jsPath} already exists.`);
     }
   }

src/convert.js

@@ -12,14 +12,19 @@ import makeCLIFn from './runner/makeCLIFn';
 import makeDecaffeinateVerifyFn from './runner/makeDecaffeinateVerifyFn';
 import runWithProgressBar from './runner/runWithProgressBar';
 import CLIError from './util/CLIError';
-import { backupPathFor, decaffeinateOutPathFor, jsPathFor } from './util/FilePaths';
+import {
+  backupPathFor,
+  COFFEE_FILE_RECOGNIZER,
+  decaffeinateOutPathFor,
+  jsPathFor,
+} from './util/FilePaths';
 import makeCommit from './util/makeCommit';
 import pluralize from './util/pluralize';
 
 export default async function convert(config) {
   await assertGitWorktreeClean();
 
-  let coffeeFiles = await getFilesToProcess(config);
+  let coffeeFiles = await getFilesToProcess(config, COFFEE_FILE_RECOGNIZER);
   if (coffeeFiles.length === 0) {
     console.log('There were no CoffeeScript files to convert.');
     return;
@@ -113,7 +118,7 @@ Re-run with the "check" command for more details.`);
   if (config.fixImportsConfig) {
     thirdCommitModifiedFiles = await runFixImports(jsFiles, config);
   }
-  await runEslintFix(jsFiles, config);
+  await runEslintFix(jsFiles, config, {isUpdate: false});
   if (config.codePrefix) {
     await prependCodePrefix(jsFiles, config.codePrefix);
   }

src/modernize/removeAutogeneratedHeader.js

@@ -0,0 +1,56 @@
+import { readFile, writeFile } from 'fs-promise';
+
+import runWithProgressBar from '../runner/runWithProgressBar';
+import { HEADER_COMMENT_LINES } from './runEslintFix';
+
+export default async function removeAutogeneratedHeader(jsFiles) {
+  await runWithProgressBar(
+    'Removing any existing autogenerated headers...', jsFiles, removeHeadersFromFile);
+}
+
+async function removeHeadersFromFile(path) {
+  let contents = await readFile(path);
+  let newContents = removeHeadersFromCode(contents);
+  if (newContents !== contents) {
+    await writeFile(path, newContents);
+  }
+}
+
+export function removeHeadersFromCode(code) {
+  let lines = code.toString().split('\n');
+
+  let resultLines = [];
+  for (let i = 0; i < lines.length; i++) {
+    let line = lines[i];
+    // Remove lines exactly matching a line we auto-generate.
+    if (Object.values(HEADER_COMMENT_LINES).includes(line)) {
+      continue;
+    }
+
+    // Remove regions of lines exactly matching our eslint-disable format.
+    if (line === '/* eslint-disable') {
+      let j = i + 1;
+      let foundMatch = false;
+      while (j < lines.length) {
+        if (lines[j] === '*/') {
+          foundMatch = true;
+          break;
+        }
+        if (!lines[j].startsWith('    ') || !lines[j].endsWith(',')) {
+          break;
+        }
+        j++;
+      }
+      if (foundMatch) {
+        // Skip forward to j, the "*/" line, so the next considered line will be
+        // the one after.
+        i = j;
+        continue;
+      }
+    }
+
+    // Everything else gets added to the file.
+    resultLines.push(line);
+  }
+  return resultLines.join('\n');
+}

src/modernize/runEslintFix.js

@@ -4,9 +4,9 @@ import runWithProgressBar from '../runner/runWithProgressBar';
 import CLIError from '../util/CLIError';
 import prependToFile from '../util/prependToFile';
 
-export default async function runEslintFix(jsFiles, config) {
+export default async function runEslintFix(jsFiles, config, {isUpdate}) {
   let eslintResults = await runWithProgressBar(
-    'Running eslint --fix on all files...', jsFiles, makeEslintFixFn(config));
+    'Running eslint --fix on all files...', jsFiles, makeEslintFixFn(config, {isUpdate}));
   for (let result of eslintResults) {
     for (let message of result.messages) {
       console.log(message);
@@ -16,11 +16,12 @@ export default async function runEslintFix(jsFiles, config) {
 
 export const HEADER_COMMENT_LINES = {
   todo: '// TODO: This file was created by bulk-decaffeinate.',
+  todoUpdated: '// TODO: This file was updated by bulk-decaffeinate.',
   fixIssues: '// Fix any style issues and re-enable lint.',
   sanityCheck: '// Sanity-check the conversion and remove this comment.',
 };
 
-function makeEslintFixFn(config) {
+function makeEslintFixFn(config, {isUpdate}) {
   return async function runEslint(path) {
     let messages = [];
 
@@ -47,10 +48,24 @@ function makeEslintFixFn(config) {
       ruleIds = Array.from(new Set(ruleIds)).sort();
     }
 
-    await prependToFile(`${path}`, `\
-${HEADER_COMMENT_LINES.todo}
-${ruleIds.length > 0 ? HEADER_COMMENT_LINES.fixIssues : HEADER_COMMENT_LINES.sanityCheck}
-`);
+    if (isUpdate) {
+      // When we're just updating a JS file, a TODO is useful if there's real
+      // stuff to fix.
+      if (ruleIds.length > 0) {
+        await prependToFile(
+          `${path}`, `${HEADER_COMMENT_LINES.todoUpdated}\n${HEADER_COMMENT_LINES.fixIssues}\n`);
+      }
+    } else {
+      // If we generated the whole file from CoffeeScript, always leave a
+      // suggestion to clean up the file.
+      if (ruleIds.length > 0) {
+        await prependToFile(
+          `${path}`, `${HEADER_COMMENT_LINES.todo}\n${HEADER_COMMENT_LINES.fixIssues}\n`);
+      } else {
+        await prependToFile(
+          `${path}`, `${HEADER_COMMENT_LINES.todo}\n${HEADER_COMMENT_LINES.sanityCheck}\n`);
+      }
+    }
     if (ruleIds.length > 0) {
       await prependToFile(`${path}`, `\
 /* eslint-disable

src/modernizeJS.js

@@ -0,0 +1,36 @@
+import getFilesToProcess from './config/getFilesToProcess';
+import removeAutogeneratedHeader from './modernize/removeAutogeneratedHeader';
+import runEslintFix from './modernize/runEslintFix';
+import runFixImports from './modernize/runFixImports';
+import runJscodeshiftScripts from './modernize/runJscodeshiftScripts';
+import makeCLIFn from './runner/makeCLIFn';
+import runWithProgressBar from './runner/runWithProgressBar';
+import { JS_FILE_RECOGNIZER } from './util/FilePaths';
+import pluralize from './util/pluralize';
+
+export default async function modernizeJS(config) {
+  let {decaffeinateArgs = [], decaffeinatePath} = config;
+
+  let jsFiles = await getFilesToProcess(config, JS_FILE_RECOGNIZER);
+  if (jsFiles.length === 0) {
+    console.log('There were no JavaScript files to convert.');
+    return;
+  }
+
+  await removeAutogeneratedHeader(jsFiles);
+  await runWithProgressBar(
+    'Running decaffeinate --modernize-js on all files...',
+    jsFiles,
+    makeCLIFn(path => `${decaffeinatePath} --modernize-js ${decaffeinateArgs.join(' ')} ${path}`)
+  );
+  if (config.jscodeshiftScripts) {
+    await runJscodeshiftScripts(jsFiles, config);
+  }
+  if (config.fixImportsConfig) {
+    await runFixImports(jsFiles, config);
+  }
+  await runEslintFix(jsFiles, config, {isUpdate: true});
+
+  console.log(`Successfully modernized ${pluralize(jsFiles.length, 'file')}.`);
+  console.log('You should now fix lint issues in any affected files.');
+}

src/util/FilePaths.js

@@ -2,7 +2,15 @@ import executable from 'executable';
 import { readFile } from 'mz/fs';
 import { basename, dirname, extname, join } from 'path';
 
-const COFFEE_EXTENSIONS = ['.coffee', '.litcoffee', '.coffee.md'];
+export const COFFEE_FILE_RECOGNIZER = {
+  extensions: ['.coffee', '.litcoffee', '.coffee.md'],
+  shebangSuffix: 'coffee',
+};
+
+export const JS_FILE_RECOGNIZER = {
+  extensions: ['.js'],
+  shebangSuffix: 'node',
+};
 
 function extensionFor(path) {
   if (path.endsWith('.coffee.md')) {
@@ -16,8 +24,9 @@ function basePathFor(path) {
   return join(dirname(path), basename(path, extension));
 }
 
-export async function shouldConvertFile(path, trackedFiles) {
-  if (!hasCoffeeExtension(path) && !await isExecutableScript(path)) {
+export async function shouldConvertFile(path, recognizer, trackedFiles) {
+  if (!hasRecognizedExtension(path, recognizer) &&
+      !await isExecutableScript(path, recognizer)) {
     return false;
   }
   if (!trackedFiles.has(path)) {
@@ -28,16 +37,16 @@ export async function shouldConvertFile(path, trackedFiles) {
   return true;
 }
 
-function hasCoffeeExtension(path) {
-  return COFFEE_EXTENSIONS.some(ext =>
+function hasRecognizedExtension(path, recognizer) {
+  return recognizer.extensions.some(ext =>
     path.endsWith(ext) && !path.endsWith(`.original${ext}`));
 }
 
-async function isExecutableScript(path) {
+async function isExecutableScript(path, recognizer) {
   if (isExtensionless(path) && await executable(path)) {
     let contents = await readFile(path);
     let firstLine = contents.toString().split('\n')[0];
-    if (firstLine.startsWith('#!') && firstLine.includes('coffee')) {
+    if (firstLine.startsWith('#!') && firstLine.includes(recognizer.shebangSuffix)) {
       return true;
     }
   }

test/examples/modernize-jscodeshift-test/A.js

@@ -0,0 +1,2 @@
+var nameBefore = 1;
+var notChanged = 2;

test/examples/modernize-jscodeshift-test/bulk-decaffeinate.config.js

@@ -0,0 +1,5 @@
+module.exports = {
+  jscodeshiftScripts: [
+    './change-name.js',
+  ],
+};

test/examples/modernize-jscodeshift-test/change-name.js

@@ -0,0 +1,15 @@
+export default function transformer(file, api) {
+  const j = api.jscodeshift;
+  return j(file.source)
+    .find(j.Identifier)
+    .replaceWith(
+      p => {
+        if (p.node.name === 'nameBefore') {
+          return j.identifier('nameAfter');
+        } else {
+          return p.node;
+        }
+      }
+    )
+    .toSource();
+}

test/examples/modernize-no-lint-failure/A.js

@@ -0,0 +1,2 @@
+var path = require('path');
+path.resolve();

test/examples/simple-modernize/A.js

@@ -0,0 +1 @@
+var a = 1;

test/mocha.opts

@@ -1,2 +1,3 @@
+--require babel-polyfill
 --compilers js:babel-register
 --timeout 60000