diff --git a/.eslintignore b/.eslintignore index ce9078b..d162442 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1 +1,2 @@ test/tmp-projects/**/*.js +test/examples/**/*.js diff --git a/README.md b/README.md index 605e448..a6724be 100644 --- a/README.md +++ b/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 diff --git a/package.json b/package.json index f435ef8..b1ecf5e 100644 --- a/package.json +++ b/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", diff --git a/src/check.js b/src/check.js index c9e4e54..7ffe032 100644 --- a/src/check.js +++ b/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), diff --git a/src/cli.js b/src/cli.js index 4cd1a6b..bd295e7 100644 --- a/src/cli.js +++ b/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') { diff --git a/src/config/getFilesToProcess.js b/src/config/getFilesToProcess.js index ef9c0af..74e647d 100644 --- a/src/config/getFilesToProcess.js +++ b/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.`); } } diff --git a/src/convert.js b/src/convert.js index 838d9c9..bad78ae 100644 --- a/src/convert.js +++ b/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); } diff --git a/src/modernize/removeAutogeneratedHeader.js b/src/modernize/removeAutogeneratedHeader.js new file mode 100644 index 0000000..e718bef --- /dev/null +++ b/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'); +} diff --git a/src/modernize/runEslintFix.js b/src/modernize/runEslintFix.js index 725fcde..13c9b6e 100644 --- a/src/modernize/runEslintFix.js +++ b/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 diff --git a/src/modernizeJS.js b/src/modernizeJS.js new file mode 100644 index 0000000..2e55073 --- /dev/null +++ b/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.'); +} diff --git a/src/util/FilePaths.js b/src/util/FilePaths.js index 8d487e6..d262b12 100644 --- a/src/util/FilePaths.js +++ b/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; } } diff --git a/test/examples/modernize-jscodeshift-test/A.js b/test/examples/modernize-jscodeshift-test/A.js new file mode 100644 index 0000000..d053c1a --- /dev/null +++ b/test/examples/modernize-jscodeshift-test/A.js @@ -0,0 +1,2 @@ +var nameBefore = 1; +var notChanged = 2; diff --git a/test/examples/modernize-jscodeshift-test/bulk-decaffeinate.config.js b/test/examples/modernize-jscodeshift-test/bulk-decaffeinate.config.js new file mode 100644 index 0000000..fc07576 --- /dev/null +++ b/test/examples/modernize-jscodeshift-test/bulk-decaffeinate.config.js @@ -0,0 +1,5 @@ +module.exports = { + jscodeshiftScripts: [ + './change-name.js', + ], +}; diff --git a/test/examples/modernize-jscodeshift-test/change-name.js b/test/examples/modernize-jscodeshift-test/change-name.js new file mode 100644 index 0000000..27e7245 --- /dev/null +++ b/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(); +} diff --git a/test/examples/modernize-no-lint-failure/A.js b/test/examples/modernize-no-lint-failure/A.js new file mode 100644 index 0000000..2c26c3d --- /dev/null +++ b/test/examples/modernize-no-lint-failure/A.js @@ -0,0 +1,2 @@ +var path = require('path'); +path.resolve(); diff --git a/test/examples/simple-modernize/A.js b/test/examples/simple-modernize/A.js new file mode 100644 index 0000000..f688910 --- /dev/null +++ b/test/examples/simple-modernize/A.js @@ -0,0 +1 @@ +var a = 1; diff --git a/test/mocha.opts b/test/mocha.opts index 14b80bc..8657a52 100644 --- a/test/mocha.opts +++ b/test/mocha.opts @@ -1,2 +1,3 @@ +--require babel-polyfill --compilers js:babel-register --timeout 60000 diff --git a/test/modernize-js-test.js b/test/modernize-js-test.js new file mode 100644 index 0000000..7a2f325 --- /dev/null +++ b/test/modernize-js-test.js @@ -0,0 +1,64 @@ +/* eslint-env mocha */ + +import { + assertFileContents, + runCliExpectSuccess, + runWithTemplateDir, +} from './test-util'; + +describe('modernize-js', () => { + it('discovers and converts JS files', async function() { + await runWithTemplateDir('simple-modernize', async function() { + await runCliExpectSuccess('modernize-js'); + await assertFileContents('./A.js', `\ +/* eslint-disable + no-unused-vars, +*/ +// TODO: This file was updated by bulk-decaffeinate. +// Fix any style issues and re-enable lint. +let a = 1; +`); + }); + }); + + it('does not leave repeated messages when run multiple times', async function() { + await runWithTemplateDir('simple-modernize', async function() { + await runCliExpectSuccess('modernize-js'); + await runCliExpectSuccess('modernize-js'); + await assertFileContents('./A.js', `\ +/* eslint-disable + no-unused-vars, +*/ +// TODO: This file was updated by bulk-decaffeinate. +// Fix any style issues and re-enable lint. +let a = 1; +`); + }); + }); + + it('does not leave a code prefix when there is no lint failure', async function() { + await runWithTemplateDir('modernize-no-lint-failure', async function() { + await runCliExpectSuccess('modernize-js'); + await assertFileContents('./A.js', `\ +import path from 'path'; +path.resolve(); +`); + }); + }); + + it('runs jscodeshift scripts', async function() { + await runWithTemplateDir('modernize-jscodeshift-test', async function() { + await runCliExpectSuccess('modernize-js'); + await assertFileContents('./A.js', `\ +/* eslint-disable + no-unused-vars, +*/ +// TODO: This file was updated by bulk-decaffeinate. +// Fix any style issues and re-enable lint. +let nameAfter = 1; +let notChanged = 2; +`); + }); + }); + +}); diff --git a/test/modernize/removeAutogeneratedHeader-test.js b/test/modernize/removeAutogeneratedHeader-test.js new file mode 100644 index 0000000..e04478f --- /dev/null +++ b/test/modernize/removeAutogeneratedHeader-test.js @@ -0,0 +1,20 @@ +/* eslint-env mocha */ +import assert from 'assert'; + +import { removeHeadersFromCode } from '../../src/modernize/removeAutogeneratedHeader'; + +describe('removeHeadersFromCode', () => { + it('works on a simple case', () => { + let code = `\ +/* eslint-disable + no-unused-vars, +*/ +// TODO: This file was created by bulk-decaffeinate. +// Fix any style issues and re-enable lint. +let a = 1; +`; + assert.equal(removeHeadersFromCode(code), `\ +let a = 1; +`); + }); +}); diff --git a/test/test-util.js b/test/test-util.js index 2a4ff40..ef29d02 100644 --- a/test/test-util.js +++ b/test/test-util.js @@ -1,6 +1,4 @@ /* eslint-env mocha */ -import 'babel-polyfill'; - import assert from 'assert'; import { exec } from 'mz/child_process'; import { exists, readFile } from 'mz/fs'; @@ -19,7 +17,7 @@ export async function runCli(args) { export async function runCliExpectSuccess(command) { let {stdout, stderr} = await runCli(command); assert(stderr.length === 0, `Nonempty stderr. stderr:\n${stderr}\n\nstdout:\n${stdout}`); - assertIncludes(stdout, 'Successfully ran decaffeinate'); + assertIncludes(stdout, 'Successfully'); return {stdout, stderr}; }