diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 229fc486b..2ccbcfacb 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -261,6 +261,11 @@ useEffect( This version updates the TypeScript parser to correctly handle JSX text with double slashes (`//`). In previous versions, this would cause Prettier to crash. +#### CLI: Add `--only-changed` flag ([#5910] by [@g-harel]) + +Flag used with `--write` to avoid re-checking files that were not changed since they were last written (with the same formatting configuration). + +[#5910]: https://github.com/prettier/prettier/pull/5910 [#6186]: https://github.com/prettier/prettier/pull/6186 [#6206]: https://github.com/prettier/prettier/pull/6206 [#6209]: https://github.com/prettier/prettier/pull/6209 @@ -272,3 +277,4 @@ This version updates the TypeScript parser to correctly handle JSX text with dou [@duailibe]: https://github.com/duailibe [@gavinjoyce]: https://github.com/gavinjoyce [@sosukesuzuki]: https://github.com/sosukesuzuki +[@g-harel]: https://github.com/g-harel diff --git a/docs/cli.md b/docs/cli.md index 5dab730fc..d5a94ec7f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -148,6 +148,8 @@ Prettier CLI will ignore files located in `node_modules` directory. To opt-out f This rewrites all processed files in place. This is comparable to the `eslint --fix` workflow. +To avoid re-checking unchanged files, use the `--only-changed` flag. + ## `--loglevel` Change the level of logging for the CLI. Valid options are: diff --git a/package.json b/package.json index 509f8b9e4..11d2bbfbd 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "editorconfig-to-prettier": "0.1.1", "escape-string-regexp": "1.0.5", "esutils": "2.0.2", + "find-cache-dir": "1.0.0", "find-parent-dir": "0.3.0", "find-project-root": "1.1.1", "flow-parser": "0.84.0", @@ -71,6 +72,7 @@ "unicode-regex": "2.0.0", "unified": "6.1.6", "vnopts": "1.0.2", + "write-file-atomic": "2.3.0", "yaml": "1.0.2", "yaml-unist-parser": "1.0.0" }, diff --git a/src/cli/changed-cache.js b/src/cli/changed-cache.js new file mode 100644 index 000000000..abb377e79 --- /dev/null +++ b/src/cli/changed-cache.js @@ -0,0 +1,79 @@ +"use strict"; + +const crypto = require("crypto"); + +// Generates a hash of the input string. +function hash(data) { + return crypto + .createHash("sha1") + .update(data) + .digest("base64"); +} + +// Generates the cache key using the file path, options and the support info hash. +function calcKey(path, options, supportInfoHash) { + return hash(path + JSON.stringify(options) + supportInfoHash); +} + +class ChangedCache { + // Initializes the in-memory cache data from the configured location. + // Also calculates the static support info hash used to compute file keys. + // A missing cache file is not treated as an error because it is expected on first run. + constructor(options) { + this.location = options.location; + this.readFile = options.readFile; + this.writeFile = options.writeFile; + this.context = options.context; + this.supportInfoHash = hash(JSON.stringify(options.supportInfo)); + + this.cache = {}; + + let contents; + try { + contents = this.readFile(this.location, "utf8"); + } catch (err) { + if (err.code !== "ENOENT") { + this.context.logger.error(`Could not read cache file: ${err}`); + } + return; + } + + try { + this.cache = JSON.parse(contents); + } catch (err) { + this.context.logger.error(`Could not parse cache contents: ${err}`); + } + } + + // Writes the in-memory cache data to the configured file. + // Previous file contents are overwritten. + close() { + let contents; + try { + contents = JSON.stringify(this.cache); + } catch (err) { + this.context.logger.error(`Could not serialize cache: ${err}`); + return; + } + + try { + this.writeFile(this.location, contents, "utf8"); + } catch (err) { + this.context.logger.error(`Could not write cache to file: ${err}`); + } + } + + // Checks if the expected contents of the file path match the in-memory data. + notChanged(path, options, content) { + return ( + this.cache[calcKey(path, options, this.supportInfoHash)] === hash(content) + ); + } + + // Updates the expected contents of the file path in the in-memory data. + update(path, options, content) { + this.cache[calcKey(path, options, this.supportInfoHash)] = hash(content); + } +} + +module.exports = ChangedCache; diff --git a/src/cli/constant.js b/src/cli/constant.js index 05f7047fe..be4175a97 100644 --- a/src/cli/constant.js +++ b/src/cli/constant.js @@ -187,6 +187,10 @@ const options = { default: "log", choices: ["silent", "error", "warn", "log", "debug"] }, + "only-changed": { + type: "boolean", + description: "Only format files changed since last '--write'." + }, stdin: { type: "boolean", description: "Force reading input from stdin." diff --git a/src/cli/index.js b/src/cli/index.js index f7a6b7832..9ffd63e45 100644 --- a/src/cli/index.js +++ b/src/cli/index.js @@ -22,6 +22,11 @@ function run(args) { process.exit(1); } + if (context.argv["only-changed"] && !context.argv["write"]) { + context.logger.error("Cannot use --only-changed without --write."); + process.exit(1); + } + if (context.argv["find-config-path"] && context.filePatterns.length) { context.logger.error("Cannot use --find-config-path with multiple files"); process.exit(1); diff --git a/src/cli/util.js b/src/cli/util.js index 592575c00..86d7f6cb8 100644 --- a/src/cli/util.js +++ b/src/cli/util.js @@ -8,6 +8,7 @@ const globby = require("globby"); const chalk = require("chalk"); const readline = require("readline"); const stringify = require("json-stable-stringify"); +const findCacheDir = require("find-cache-dir"); const minimist = require("./minimist"); const prettier = require("../../index"); @@ -20,6 +21,7 @@ const optionsNormalizer = require("../main/options-normalizer"); const thirdParty = require("../common/third-party"); const arrayify = require("../utils/arrayify"); const isTTY = require("../utils/is-tty"); +const ChangedCache = require("./changed-cache"); const OPTION_USAGE_THRESHOLD = 25; const CHOICE_USAGE_MARGIN = 3; @@ -441,6 +443,18 @@ function formatFiles(context) { context.logger.log("Checking formatting..."); } + let changedCache = null; + if (context.argv["only-changed"]) { + const cacheDir = findCacheDir({ name: "prettier", create: true }); + changedCache = new ChangedCache({ + location: path.join(cacheDir, "changed"), + readFile: fs.readFileSync, + writeFile: thirdParty.writeFileAtomic, + context: context, + supportInfo: prettier.getSupportInfo() + }); + } + eachFilename(context, context.filePatterns, filename => { const fileIgnored = ignorer.filter([filename]).length === 0; if ( @@ -457,9 +471,15 @@ function formatFiles(context) { filepath: filename }); + let removeFilename = () => {}; if (isTTY()) { // Don't use `console.log` here since we need to replace this line. context.logger.log(filename, { newline: false }); + removeFilename = () => { + readline.clearLine(process.stdout, 0); + readline.cursorTo(process.stdout, 0, null); + removeFilename = () => {}; + }; } let input; @@ -477,6 +497,19 @@ function formatFiles(context) { return; } + if (changedCache) { + if (changedCache.notChanged(filename, options, input)) { + // Remove previously printed filename to log it with "unchanged". + removeFilename(); + + if (!context.argv["check"] && !context.argv["list-different"]) { + context.logger.log(chalk.grey(`${filename} unchanged`)); + } + + return; + } + } + if (fileIgnored) { writeOutput(context, { formatted: input }, options); return; @@ -501,11 +534,8 @@ function formatFiles(context) { const isDifferent = output !== input; - if (isTTY()) { - // Remove previously printed filename to log it with duration. - readline.clearLine(process.stdout, 0); - readline.cursorTo(process.stdout, 0, null); - } + // Remove previously printed filename to log it with duration. + removeFilename(); if (context.argv["write"]) { // Don't write the file if it won't change in order not to invalidate @@ -524,8 +554,15 @@ function formatFiles(context) { // Don't exit the process if one file failed process.exitCode = 2; } - } else if (!context.argv["check"] && !context.argv["list-different"]) { - context.logger.log(`${chalk.grey(filename)} ${Date.now() - start}ms`); + } else { + if (!context.argv["check"] && !context.argv["list-different"]) { + context.logger.log(`${chalk.grey(filename)} ${Date.now() - start}ms`); + } + } + + // Cache is updated to record pretty content. + if (changedCache) { + changedCache.update(filename, options, output); } } else if (context.argv["debug-check"]) { if (result.filepath) { @@ -546,6 +583,10 @@ function formatFiles(context) { } }); + if (changedCache) { + changedCache.close(); + } + // Print check summary based on expected exit code if (context.argv["check"]) { context.logger.log( diff --git a/src/common/third-party.js b/src/common/third-party.js index 8089f5fde..570dbbc2d 100644 --- a/src/common/third-party.js +++ b/src/common/third-party.js @@ -4,5 +4,6 @@ module.exports = { cosmiconfig: require("cosmiconfig"), findParentDir: require("find-parent-dir").sync, getStream: require("get-stream"), - isCI: () => require("is-ci") + isCI: () => require("is-ci"), + writeFileAtomic: require("write-file-atomic").sync }; diff --git a/tests_integration/__tests__/__snapshots__/early-exit.js.snap b/tests_integration/__tests__/__snapshots__/early-exit.js.snap index 458a18be6..785f2e912 100644 --- a/tests_integration/__tests__/__snapshots__/early-exit.js.snap +++ b/tests_integration/__tests__/__snapshots__/early-exit.js.snap @@ -141,6 +141,7 @@ Other options: --loglevel What level of logs to report. Defaults to log. + --only-changed Only format files changed since last '--write'. --require-pragma Require either '@prettier' or '@format' to be present in the file's first docblock comment in order for it to be formatted. Defaults to false. @@ -294,6 +295,7 @@ Other options: --loglevel What level of logs to report. Defaults to log. + --only-changed Only format files changed since last '--write'. --require-pragma Require either '@prettier' or '@format' to be present in the file's first docblock comment in order for it to be formatted. Defaults to false. @@ -344,6 +346,15 @@ exports[`throw error with --help not-found (stdout) 1`] = `""`; exports[`throw error with --help not-found (write) 1`] = `Array []`; +exports[`throw error with --only-changed without --write (stderr) 1`] = ` +"[error] Cannot use --only-changed without --write. +" +`; + +exports[`throw error with --only-changed without --write (stdout) 1`] = `""`; + +exports[`throw error with --only-changed without --write (write) 1`] = `Array []`; + exports[`throw error with --write + --debug-check (stderr) 1`] = ` "[error] Cannot use --write and --debug-check together. " diff --git a/tests_integration/__tests__/__snapshots__/help-options.js.snap b/tests_integration/__tests__/__snapshots__/help-options.js.snap index 4f43f9fa9..29aaeaeae 100644 --- a/tests_integration/__tests__/__snapshots__/help-options.js.snap +++ b/tests_integration/__tests__/__snapshots__/help-options.js.snap @@ -329,6 +329,17 @@ exports[`show detailed usage with --help no-semi (stdout) 1`] = ` exports[`show detailed usage with --help no-semi (write) 1`] = `Array []`; +exports[`show detailed usage with --help only-changed (stderr) 1`] = `""`; + +exports[`show detailed usage with --help only-changed (stdout) 1`] = ` +"--only-changed + + Only format files changed since last '--write'. +" +`; + +exports[`show detailed usage with --help only-changed (write) 1`] = `Array []`; + exports[`show detailed usage with --help parser (stderr) 1`] = `""`; exports[`show detailed usage with --help parser (stdout) 1`] = ` diff --git a/tests_integration/__tests__/__snapshots__/only-changed.js.snap b/tests_integration/__tests__/__snapshots__/only-changed.js.snap new file mode 100644 index 000000000..c49c2d715 --- /dev/null +++ b/tests_integration/__tests__/__snapshots__/only-changed.js.snap @@ -0,0 +1,43 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`create cache with --write --only-changed + formatted file (stderr) 1`] = `""`; + +exports[`create cache with --write --only-changed + formatted file (stdout) 1`] = ` +"formatted.js 0ms +" +`; + +exports[`create cache with --write --only-changed + unformatted file (stderr) 1`] = `""`; + +exports[`create cache with --write --only-changed + unformatted file (stdout) 1`] = ` +"unformatted.js 0ms +" +`; + +exports[`detect config change with --write --only-changed + unformatted file (stderr) 1`] = `""`; + +exports[`detect config change with --write --only-changed + unformatted file (stderr) 2`] = `""`; + +exports[`detect config change with --write --only-changed + unformatted file (stdout) 1`] = ` +"unformatted.js 0ms +" +`; + +exports[`detect config change with --write --only-changed + unformatted file (stdout) 2`] = ` +"unformatted.js 0ms +" +`; + +exports[`detect unchanged with --write --only-changed + formatted file (stderr) 1`] = `""`; + +exports[`detect unchanged with --write --only-changed + formatted file (stderr) 2`] = `""`; + +exports[`detect unchanged with --write --only-changed + formatted file (stdout) 1`] = ` +"formatted.js 0ms +" +`; + +exports[`detect unchanged with --write --only-changed + formatted file (stdout) 2`] = ` +"formatted.js unchanged +" +`; diff --git a/tests_integration/__tests__/early-exit.js b/tests_integration/__tests__/early-exit.js index 2148eaa74..e0d291b8b 100644 --- a/tests_integration/__tests__/early-exit.js +++ b/tests_integration/__tests__/early-exit.js @@ -68,6 +68,12 @@ describe("throw error with --write + --debug-check", () => { }); }); +describe("throw error with --only-changed without --write", () => { + runPrettier("cli", ["--only-changed"]).test({ + status: 1 + }); +}); + describe("throw error with --find-config-path + multiple files", () => { runPrettier("cli", ["--find-config-path", "abc.js", "def.js"]).test({ status: 1 diff --git a/tests_integration/__tests__/only-changed.js b/tests_integration/__tests__/only-changed.js new file mode 100644 index 000000000..e6205b4d4 --- /dev/null +++ b/tests_integration/__tests__/only-changed.js @@ -0,0 +1,162 @@ +"use strict"; + +const path = require("path"); +const findCacheDir = require("find-cache-dir"); + +const runPrettier = require("../runPrettier"); +const ChangedCache = require("../../src/cli/changed-cache"); + +// Cache name must be kept consistent with value in the implementation. +const cacheName = "changed"; +const cachePath = path.join(findCacheDir({ name: "prettier" }), cacheName); + +describe("create cache with --write --only-changed + unformatted file", () => { + runPrettier("cli/only-changed", [ + "--write", + "--only-changed", + "unformatted.js" + ]).test({ + write: [{ filename: "unformatted.js" }, { filename: cachePath }], + status: 0 + }); +}); + +describe("create cache with --write --only-changed + formatted file", () => { + runPrettier("cli/only-changed", [ + "--write", + "--only-changed", + "formatted.js" + ]).test({ + write: [{ filename: cachePath }], + status: 0 + }); +}); + +describe("detect unchanged with --write --only-changed + formatted file", () => { + const res = runPrettier("cli/only-changed", [ + "--write", + "--only-changed", + "formatted.js" + ]).test({ + write: [{ filename: cachePath }], + status: 0 + }); + + const cacheContents = res.write[0].content; + + runPrettier( + "cli/only-changed", + ["--write", "--only-changed", "formatted.js"], + { + virtualFiles: { + [cachePath]: cacheContents + } + } + ).test({ + write: [{ filename: cachePath, content: cacheContents }], + status: 0 + }); +}); + +describe("detect config change with --write --only-changed + unformatted file", () => { + const resBefore = runPrettier("cli/only-changed", [ + "--write", + "--only-changed", + "unformatted.js" + ]).test({ + write: [{ filename: "unformatted.js" }, { filename: cachePath }], + status: 0 + }); + + const cacheContentsBefore = resBefore.write[1].content; + + const resAfter = runPrettier( + "cli/only-changed", + ["--write", "--only-changed", "--use-tabs", "unformatted.js"], + { + virtualFiles: { + [cachePath]: cacheContentsBefore + } + } + ).test({ + write: [{ filename: "unformatted.js" }, { filename: cachePath }], + status: 0 + }); + + const cacheContentsAfter = resAfter.write[1].content; + + expect(cacheContentsAfter).not.toBe(cacheContentsBefore); +}); + +describe("ChangedCache", () => { + it("should log errors when opening the cache file", () => { + const errLogger = jest.fn(); + const msg = "open-cache-file-error"; + + new ChangedCache({ + location: cachePath, + readFile: () => { + throw new Error(msg); + }, + writeFile: () => {}, + context: { logger: { error: errLogger } }, + supportInfo: {} + }); + + expect(errLogger).toHaveBeenCalledWith(expect.stringContaining(msg)); + }); + + it("should log errors when parsing the cache file", () => { + const errLogger = jest.fn(); + + new ChangedCache({ + location: cachePath, + readFile: () => "invalid json", + writeFile: () => {}, + context: { logger: { error: errLogger } }, + supportInfo: {} + }); + + expect(errLogger).toHaveBeenCalledWith( + expect.stringContaining("cache content") + ); + }); + + it("should log errors when closing the cache file", () => { + const errLogger = jest.fn(); + const msg = "close-cache-file-error"; + + const changedCache = new ChangedCache({ + location: cachePath, + readFile: () => "{}", + writeFile: () => { + throw new Error(msg); + }, + context: { logger: { error: errLogger } }, + supportInfo: {} + }); + changedCache.close(); + + expect(errLogger).toHaveBeenCalledWith(expect.stringContaining(msg)); + }); + + it("should log errors when serializing the cache contents", () => { + const errLogger = jest.fn(); + + const changedCache = new ChangedCache({ + location: cachePath, + readFile: () => "{}", + writeFile: () => {}, + context: { logger: { error: errLogger } }, + supportInfo: {} + }); + const mirror = {}; + mirror.self = mirror; + changedCache.cache = mirror; + changedCache.close(); + + expect(errLogger).toHaveBeenCalledWith( + expect.stringContaining("serialize cache") + ); + }); +}); diff --git a/tests_integration/cli/only-changed/formatted.js b/tests_integration/cli/only-changed/formatted.js new file mode 100644 index 000000000..b506100a9 --- /dev/null +++ b/tests_integration/cli/only-changed/formatted.js @@ -0,0 +1 @@ +var x = 1; diff --git a/tests_integration/cli/only-changed/unformatted.js b/tests_integration/cli/only-changed/unformatted.js new file mode 100644 index 000000000..646611c06 --- /dev/null +++ b/tests_integration/cli/only-changed/unformatted.js @@ -0,0 +1 @@ +var x = 1; \ No newline at end of file diff --git a/tests_integration/runPrettier.js b/tests_integration/runPrettier.js index 27226065a..c1d62bc1e 100644 --- a/tests_integration/runPrettier.js +++ b/tests_integration/runPrettier.js @@ -64,6 +64,18 @@ function runPrettier(dir, args, options) { return origStatSync(filename); }); + // Mock contents of "virtualFiles" when option is defined. + const origReadFileSync = fs.readFileSync; + jest.spyOn(fs, "readFileSync").mockImplementation((filename, opts) => { + if ( + typeof options.virtualFiles === "object" && + typeof options.virtualFiles[filename] === "string" + ) { + return options.virtualFiles[filename]; + } + return origReadFileSync(filename, opts); + }); + const originalCwd = process.cwd(); const originalArgv = process.argv; const originalExitCode = process.exitCode; @@ -99,6 +111,11 @@ function runPrettier(dir, args, options) { jest .spyOn(require(thirdParty), "findParentDir") .mockImplementation(() => process.cwd()); + jest + .spyOn(require(thirdParty), "writeFileAtomic") + .mockImplementation((filename, content) => { + write.push({ filename, content }); + }); try { require(prettierCli); @@ -135,6 +152,9 @@ function runPrettier(dir, args, options) { if (name in testOptions) { if (name === "status" && testOptions[name] === "non-zero") { expect(value).not.toEqual(0); + } else if (name === "write") { + // Allows assertions on a subset of the "write" result. (ex. only file name) + expect(value).toMatchObject(testOptions[name]); } else { expect(value).toEqual(testOptions[name]); } diff --git a/yarn.lock b/yarn.lock index 3f811d0b2..e1d0c7913 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2691,6 +2691,15 @@ fill-range@^4.0.0: repeat-string "^1.6.1" to-regex-range "^2.1.0" +find-cache-dir@1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/find-cache-dir/-/find-cache-dir-1.0.0.tgz#9288e3e9e3cc3748717d39eade17cf71fc30ee6f" + integrity sha1-kojj6ePMN0hxfTnq3hfPcfww7m8= + dependencies: + commondir "^1.0.1" + make-dir "^1.0.0" + pkg-dir "^2.0.0" + find-cache-dir@^2.0.0: version "2.1.0" resolved "https://registry.yarnpkg.com/find-cache-dir/-/find-cache-dir-2.1.0.tgz#8d0f94cd13fe43c6c7c261a0d86115ca918c05f7" @@ -6747,9 +6756,10 @@ wrappy@1: version "1.0.2" resolved "https://registry.yarnpkg.com/wrappy/-/wrappy-1.0.2.tgz#b5243d8f3ec1aa35f1364605bc0d1036e30ab69f" -write-file-atomic@^2.1.0: +write-file-atomic@2.3.0, write-file-atomic@^2.1.0: version "2.3.0" resolved "https://registry.yarnpkg.com/write-file-atomic/-/write-file-atomic-2.3.0.tgz#1ff61575c2e2a4e8e510d6fa4e243cce183999ab" + integrity sha512-xuPeK4OdjWqtfi59ylvVL0Yn35SF3zgcAcv7rBPFHVaEapaDr4GdGgm3j7ckTwH9wHL7fGmgfAnb0+THrHb8tA== dependencies: graceful-fs "^4.1.11" imurmurhash "^0.1.4"