From 63a9a2e17283890ef9e54fd36a743caf679e60c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 21:43:07 +0200 Subject: [PATCH 1/7] Ignore node_modules by default --- bin/prettier.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/bin/prettier.js b/bin/prettier.js index 36ca4907c967..1ba6baf3f58b 100755 --- a/bin/prettier.js +++ b/bin/prettier.js @@ -3,6 +3,7 @@ "use strict"; const fs = require("fs"); +const path = require("path"); const getStdin = require("get-stdin"); const glob = require("glob"); const chalk = require("chalk"); @@ -30,6 +31,7 @@ const argv = minimist(process.argv.slice(2), { "version", "debug-print-doc", "debug-check", + "with-node-modules", // Deprecated in 0.0.10 "flow-parser" ], @@ -64,6 +66,10 @@ if (argv["version"]) { const filepatterns = argv["_"]; const write = argv["write"]; const stdin = argv["stdin"] || (!filepatterns.length && !process.stdin.isTTY); +const ignoreNodeModules = argv["with-node-modules"] === false; +const globOptions = { + ignore: ignoreNodeModules && "**/node_modules/**" +}; if (write && argv["debug-check"]) { console.error("Cannot use --write and --debug-check together."); @@ -341,11 +347,14 @@ if (stdin) { function eachFilename(patterns, callback) { patterns.forEach(pattern => { if (!glob.hasMagic(pattern)) { + if (shouldIgnorePattern(pattern)) { + return; + } callback(pattern); return; } - glob(pattern, (err, filenames) => { + glob(pattern, globOptions, (err, filenames) => { if (err) { console.error("Unable to expand glob pattern: " + pattern + "\n" + err); // Don't exit the process if one pattern failed @@ -359,3 +368,7 @@ function eachFilename(patterns, callback) { }); }); } + +function shouldIgnorePattern(pattern) { + return ignoreNodeModules && path.resolve(pattern).includes("/node_modules/"); +} From 0a8f70a119d17a7ae7799fecb266d62f6394368e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 21:43:42 +0200 Subject: [PATCH 2/7] Add integration test suite for --with-node-modules CLI arg --- package.json | 5 +- tests_integration/.eslintrc.js | 7 +++ .../__tests__/with-node-modules.js | 57 +++++++++++++++++++ .../not_node_modules/file.js | 1 + .../cli/with-node-modules/regular-module.js | 1 + tests_integration/runPrettier.js | 30 ++++++++++ yarn.lock | 35 +++++++++++- 7 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 tests_integration/.eslintrc.js create mode 100644 tests_integration/__tests__/with-node-modules.js create mode 100644 tests_integration/cli/with-node-modules/not_node_modules/file.js create mode 100644 tests_integration/cli/with-node-modules/regular-module.js create mode 100644 tests_integration/runPrettier.js diff --git a/package.json b/package.json index b440fc7a87cd..093e96c8120d 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "minimist": "1.2.0" }, "devDependencies": { + "cross-spawn": "^5.1.0", "diff": "3.2.0", "eslint": "^3.19.0", "eslint-plugin-prettier": "^2.1.1", @@ -40,6 +41,7 @@ }, "scripts": { "test": "jest", + "test-integration": "jest tests_integration", "lint": "eslint .", "build:docs": "rollup -c docs/rollup.config.js" }, @@ -50,11 +52,10 @@ "snapshotSerializers": [ "/tests_config/raw-serializer.js" ], - "testRegex": "jsfmt\\.spec\\.js$", + "testRegex": "jsfmt\\.spec\\.js$|__tests__/(.*).js", "testPathIgnorePatterns": [ "tests/new_react", "tests/more_react", - "see https://github.com/eslint/typescript-eslint-parser/issues/269", "tests/typescript/conformance/types/abstractKeyword/jsfmt.spec.js" ] diff --git a/tests_integration/.eslintrc.js b/tests_integration/.eslintrc.js new file mode 100644 index 000000000000..5c29e44090ef --- /dev/null +++ b/tests_integration/.eslintrc.js @@ -0,0 +1,7 @@ +"use strict"; + +module.exports = { + env: { + jest: true + } +}; diff --git a/tests_integration/__tests__/with-node-modules.js b/tests_integration/__tests__/with-node-modules.js new file mode 100644 index 000000000000..5894f1d336b0 --- /dev/null +++ b/tests_integration/__tests__/with-node-modules.js @@ -0,0 +1,57 @@ +"use strict"; + +const runPrettier = require("../runPrettier"); + +test("ignores node_modules by default", () => { + const { stdout, status } = runPrettier("cli/with-node-modules", [ + "**/*.js", + "-l" + ]); + const files = stdout.split("\n"); + + expect(files).not.toContain("node_modules/node-module.js"); + expect(files).toContain("regular-module.js"); + expect(files).toContain("not_node_modules/file.js"); + expect(status).toBe(1); +}); + +test("doesn't ignore node_modules with --with-node-modules flag", () => { + const { stdout, status } = runPrettier("cli/with-node-modules", [ + "**/*.js", + "-l", + "--with-node-modules" + ]); + const files = stdout.split("\n"); + + expect(files).toContain("node_modules/node-module.js"); + expect(files).toContain("regular-module.js"); + expect(files).toContain("not_node_modules/file.js"); + expect(status).toBe(1); +}); + +test("ignores node_modules by default for file list", () => { + const { stdout, status } = runPrettier("cli/with-node-modules", [ + "node_modules/node-module.js", + "regular-module.js", + "-l" + ]); + const files = stdout.split("\n"); + + expect(files).not.toContain("node_modules/node-module.js"); + expect(files).toContain("regular-module.js"); + expect(status).toBe(1); +}); + +test("doesn't ignore node_modules with --with-node-modules flag for file list", () => { + const { stdout, status } = runPrettier("cli/with-node-modules", [ + "node_modules/node-module.js", + "regular-module.js", + "-l", + "--with-node-modules" + ]); + const files = stdout.split("\n"); + + expect(files).toContain("node_modules/node-module.js"); + expect(files).toContain("regular-module.js"); + expect(status).toBe(1); +}); diff --git a/tests_integration/cli/with-node-modules/not_node_modules/file.js b/tests_integration/cli/with-node-modules/not_node_modules/file.js new file mode 100644 index 000000000000..ad9a93a7c160 --- /dev/null +++ b/tests_integration/cli/with-node-modules/not_node_modules/file.js @@ -0,0 +1 @@ +'use strict'; diff --git a/tests_integration/cli/with-node-modules/regular-module.js b/tests_integration/cli/with-node-modules/regular-module.js new file mode 100644 index 000000000000..ad9a93a7c160 --- /dev/null +++ b/tests_integration/cli/with-node-modules/regular-module.js @@ -0,0 +1 @@ +'use strict'; diff --git a/tests_integration/runPrettier.js b/tests_integration/runPrettier.js new file mode 100644 index 000000000000..6f31030375fa --- /dev/null +++ b/tests_integration/runPrettier.js @@ -0,0 +1,30 @@ +/* + * runPrettier – spawns `prettier` process. + * Adopted from Jest's integration tests suite. + */ +"use strict"; + +const path = require("path"); +const { sync: spawnSync } = require("cross-spawn"); + +const PRETTIER_PATH = path.resolve(__dirname, "../bin/prettier.js"); + +// return the result of the spawned process: +// [ 'status', 'signal', 'output', 'pid', 'stdout', 'stderr', +// 'envPairs', 'options', 'args', 'file' ] +function runPrettier(dir, args) { + const isRelative = dir[0] !== "/"; + + if (isRelative) { + dir = path.resolve(__dirname, dir); + } + + const result = spawnSync(PRETTIER_PATH, args || [], { cwd: dir }); + + result.stdout = result.stdout && result.stdout.toString(); + result.stderr = result.stderr && result.stderr.toString(); + + return result; +} + +module.exports = runPrettier; diff --git a/yarn.lock b/yarn.lock index dcfc69b8b870..a1ded6383743 100644 --- a/yarn.lock +++ b/yarn.lock @@ -592,6 +592,14 @@ create-hmac@^1.1.0, create-hmac@^1.1.2: create-hash "^1.1.0" inherits "^2.0.1" +cross-spawn@^5.1.0: + version "5.1.0" + resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-5.1.0.tgz#e8bd0efee58fcff6f8f94510a0a554bbfa235449" + dependencies: + lru-cache "^4.0.1" + shebang-command "^1.2.0" + which "^1.2.9" + cryptiles@2.x.x: version "2.0.5" resolved "https://registry.yarnpkg.com/cryptiles/-/cryptiles-2.0.5.tgz#3bdfecdc608147c1c67202fa291e7dca59eaa3b8" @@ -1843,6 +1851,13 @@ loose-envify@^1.0.0: dependencies: js-tokens "^3.0.0" +lru-cache@^4.0.1: + version "4.0.2" + resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-4.0.2.tgz#1d17679c069cda5d040991a09dbc2c0db377e55e" + dependencies: + pseudomap "^1.0.1" + yallist "^2.0.0" + magic-string@^0.16.0: version "0.16.0" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.16.0.tgz#970ebb0da7193301285fb1aa650f39bdd81eb45a" @@ -2189,6 +2204,10 @@ prr@~0.0.0: version "0.0.0" resolved "https://registry.yarnpkg.com/prr/-/prr-0.0.0.tgz#1a84b85908325501411853d0081ee3fa86e2926a" +pseudomap@^1.0.1: + version "1.0.2" + resolved "https://registry.yarnpkg.com/pseudomap/-/pseudomap-1.0.2.tgz#f052a28da70e618917ef0a8ac34c1ae5a68286b3" + public-encrypt@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/public-encrypt/-/public-encrypt-4.0.0.tgz#39f699f3a46560dd5ebacbca693caf7c65c18cc6" @@ -2467,6 +2486,16 @@ sha.js@^2.3.6: dependencies: inherits "^2.0.1" +shebang-command@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-1.2.0.tgz#44aac65b695b03398968c39f363fee5deafdf1ea" + dependencies: + shebang-regex "^1.0.0" + +shebang-regex@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/shebang-regex/-/shebang-regex-1.0.0.tgz#da42f49740c0b42db2ca9728571cb190c98efea3" + shelljs@^0.7.5: version "0.7.7" resolved "https://registry.yarnpkg.com/shelljs/-/shelljs-0.7.7.tgz#b2f5c77ef97148f4b4f6e22682e10bba8667cff1" @@ -2782,7 +2811,7 @@ which-module@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/which-module/-/which-module-1.0.0.tgz#bba63ca861948994ff307736089e3b96026c2a4f" -which@^1.2.12: +which@^1.2.12, which@^1.2.9: version "1.2.14" resolved "https://registry.yarnpkg.com/which/-/which-1.2.14.tgz#9a87c4378f03e827cecaf1acdf56c736c01c14e5" dependencies: @@ -2840,6 +2869,10 @@ y18n@^3.2.1: version "3.2.1" resolved "https://registry.yarnpkg.com/y18n/-/y18n-3.2.1.tgz#6d15fba884c08679c0d77e88e7759e811e07fa41" +yallist@^2.0.0: + version "2.1.2" + resolved "https://registry.yarnpkg.com/yallist/-/yallist-2.1.2.tgz#1c11f9218f076089a47dd512f93c6699a6a81d52" + yargs-parser@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-5.0.0.tgz#275ecf0d7ffe05c77e64e7c86e4cd94bf0e1228a" From 8b2f935307dff21e72c553947c1d4f5f216d46fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 21:55:17 +0200 Subject: [PATCH 3/7] Refactor tests to snapshots --- package.json | 2 +- .../__snapshots__/with-node-modules.js.snap | 27 +++++++++++++++++++ .../__tests__/with-node-modules.js | 20 +++++--------- 3 files changed, 34 insertions(+), 15 deletions(-) create mode 100644 tests_integration/__tests__/__snapshots__/with-node-modules.js.snap diff --git a/package.json b/package.json index 093e96c8120d..2ba129ba3b75 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "snapshotSerializers": [ "/tests_config/raw-serializer.js" ], - "testRegex": "jsfmt\\.spec\\.js$|__tests__/(.*).js", + "testRegex": "jsfmt\\.spec\\.js$|__tests__/.*\\.js$", "testPathIgnorePatterns": [ "tests/new_react", "tests/more_react", diff --git a/tests_integration/__tests__/__snapshots__/with-node-modules.js.snap b/tests_integration/__tests__/__snapshots__/with-node-modules.js.snap new file mode 100644 index 000000000000..5fb7217a7f33 --- /dev/null +++ b/tests_integration/__tests__/__snapshots__/with-node-modules.js.snap @@ -0,0 +1,27 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`doesn't ignore node_modules with --with-node-modules flag 1`] = ` +"node_modules/node-module.js +not_node_modules/file.js +regular-module.js +" +`; + +exports[`doesn't ignore node_modules with --with-node-modules flag for file list 1`] = ` +"node_modules/node-module.js +not_node_modules/file.js +regular-module.js +" +`; + +exports[`ignores node_modules by default 1`] = ` +"not_node_modules/file.js +regular-module.js +" +`; + +exports[`ignores node_modules by default for file list 1`] = ` +"not_node_modules/file.js +regular-module.js +" +`; diff --git a/tests_integration/__tests__/with-node-modules.js b/tests_integration/__tests__/with-node-modules.js index 5894f1d336b0..df4d380f187b 100644 --- a/tests_integration/__tests__/with-node-modules.js +++ b/tests_integration/__tests__/with-node-modules.js @@ -7,11 +7,8 @@ test("ignores node_modules by default", () => { "**/*.js", "-l" ]); - const files = stdout.split("\n"); - expect(files).not.toContain("node_modules/node-module.js"); - expect(files).toContain("regular-module.js"); - expect(files).toContain("not_node_modules/file.js"); + expect(stdout).toMatchSnapshot(); expect(status).toBe(1); }); @@ -21,37 +18,32 @@ test("doesn't ignore node_modules with --with-node-modules flag", () => { "-l", "--with-node-modules" ]); - const files = stdout.split("\n"); - expect(files).toContain("node_modules/node-module.js"); - expect(files).toContain("regular-module.js"); - expect(files).toContain("not_node_modules/file.js"); + expect(stdout).toMatchSnapshot(); expect(status).toBe(1); }); test("ignores node_modules by default for file list", () => { const { stdout, status } = runPrettier("cli/with-node-modules", [ "node_modules/node-module.js", + "not_node_modules/file.js", "regular-module.js", "-l" ]); - const files = stdout.split("\n"); - expect(files).not.toContain("node_modules/node-module.js"); - expect(files).toContain("regular-module.js"); + expect(stdout).toMatchSnapshot(); expect(status).toBe(1); }); test("doesn't ignore node_modules with --with-node-modules flag for file list", () => { const { stdout, status } = runPrettier("cli/with-node-modules", [ "node_modules/node-module.js", + "not_node_modules/file.js", "regular-module.js", "-l", "--with-node-modules" ]); - const files = stdout.split("\n"); - expect(files).toContain("node_modules/node-module.js"); - expect(files).toContain("regular-module.js"); + expect(stdout).toMatchSnapshot(); expect(status).toBe(1); }); From b48c1e4ee9aabc1933cce7429e36a3508e6107c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 22:10:00 +0200 Subject: [PATCH 4/7] Disable eslint for purposely faulty files --- tests_integration/cli/with-node-modules/not_node_modules/file.js | 1 + tests_integration/cli/with-node-modules/regular-module.js | 1 + 2 files changed, 2 insertions(+) diff --git a/tests_integration/cli/with-node-modules/not_node_modules/file.js b/tests_integration/cli/with-node-modules/not_node_modules/file.js index ad9a93a7c160..3d0ca1bd1785 100644 --- a/tests_integration/cli/with-node-modules/not_node_modules/file.js +++ b/tests_integration/cli/with-node-modules/not_node_modules/file.js @@ -1 +1,2 @@ +/* eslint-disable */ 'use strict'; diff --git a/tests_integration/cli/with-node-modules/regular-module.js b/tests_integration/cli/with-node-modules/regular-module.js index ad9a93a7c160..3d0ca1bd1785 100644 --- a/tests_integration/cli/with-node-modules/regular-module.js +++ b/tests_integration/cli/with-node-modules/regular-module.js @@ -1 +1,2 @@ +/* eslint-disable */ 'use strict'; From 8670f52b40ce2c61c298a33703eda36f6731cd6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 22:20:08 +0200 Subject: [PATCH 5/7] Fix node 4 --- .../__tests__/with-node-modules.js | 20 ++++++++----------- tests_integration/runPrettier.js | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/tests_integration/__tests__/with-node-modules.js b/tests_integration/__tests__/with-node-modules.js index df4d380f187b..35abf054a6aa 100644 --- a/tests_integration/__tests__/with-node-modules.js +++ b/tests_integration/__tests__/with-node-modules.js @@ -3,40 +3,37 @@ const runPrettier = require("../runPrettier"); test("ignores node_modules by default", () => { - const { stdout, status } = runPrettier("cli/with-node-modules", [ + const result = runPrettier("cli/with-node-modules", [ "**/*.js", "-l" ]); - expect(stdout).toMatchSnapshot(); - expect(status).toBe(1); + expect(result.stdout).toMatchSnapshot(); }); test("doesn't ignore node_modules with --with-node-modules flag", () => { - const { stdout, status } = runPrettier("cli/with-node-modules", [ + const result = runPrettier("cli/with-node-modules", [ "**/*.js", "-l", "--with-node-modules" ]); - expect(stdout).toMatchSnapshot(); - expect(status).toBe(1); + expect(result.stdout).toMatchSnapshot(); }); test("ignores node_modules by default for file list", () => { - const { stdout, status } = runPrettier("cli/with-node-modules", [ + const result = runPrettier("cli/with-node-modules", [ "node_modules/node-module.js", "not_node_modules/file.js", "regular-module.js", "-l" ]); - expect(stdout).toMatchSnapshot(); - expect(status).toBe(1); + expect(result.stdout).toMatchSnapshot(); }); test("doesn't ignore node_modules with --with-node-modules flag for file list", () => { - const { stdout, status } = runPrettier("cli/with-node-modules", [ + const result = runPrettier("cli/with-node-modules", [ "node_modules/node-module.js", "not_node_modules/file.js", "regular-module.js", @@ -44,6 +41,5 @@ test("doesn't ignore node_modules with --with-node-modules flag for file list", "--with-node-modules" ]); - expect(stdout).toMatchSnapshot(); - expect(status).toBe(1); + expect(result.stdout).toMatchSnapshot(); }); diff --git a/tests_integration/runPrettier.js b/tests_integration/runPrettier.js index 6f31030375fa..682b50e68ca6 100644 --- a/tests_integration/runPrettier.js +++ b/tests_integration/runPrettier.js @@ -5,7 +5,7 @@ "use strict"; const path = require("path"); -const { sync: spawnSync } = require("cross-spawn"); +const spawnSync = require("cross-spawn").sync; const PRETTIER_PATH = path.resolve(__dirname, "../bin/prettier.js"); From f6f288a5f153525b7224773073f8af6b23e0291d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 22:22:51 +0200 Subject: [PATCH 6/7] Fix prettier --- tests_integration/__tests__/with-node-modules.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests_integration/__tests__/with-node-modules.js b/tests_integration/__tests__/with-node-modules.js index 35abf054a6aa..de23fcaaaa64 100644 --- a/tests_integration/__tests__/with-node-modules.js +++ b/tests_integration/__tests__/with-node-modules.js @@ -3,10 +3,7 @@ const runPrettier = require("../runPrettier"); test("ignores node_modules by default", () => { - const result = runPrettier("cli/with-node-modules", [ - "**/*.js", - "-l" - ]); + const result = runPrettier("cli/with-node-modules", ["**/*.js", "-l"]); expect(result.stdout).toMatchSnapshot(); }); From c285d0872dbbd39aa1a6a0ab7047242e219fe116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 23 May 2017 23:02:02 +0200 Subject: [PATCH 7/7] Gitignore only top-level node_modules --- .gitignore | 2 +- .../cli/with-node-modules/node_modules/node-module.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 tests_integration/cli/with-node-modules/node_modules/node-module.js diff --git a/.gitignore b/.gitignore index 8a7c4c92a13a..da4200be0bfa 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ -node_modules +/node_modules npm-debug.log /errors /test.js diff --git a/tests_integration/cli/with-node-modules/node_modules/node-module.js b/tests_integration/cli/with-node-modules/node_modules/node-module.js new file mode 100644 index 000000000000..3d0ca1bd1785 --- /dev/null +++ b/tests_integration/cli/with-node-modules/node_modules/node-module.js @@ -0,0 +1,2 @@ +/* eslint-disable */ +'use strict';