Fix CI detection to avoid unwanted TTY behavior (#5804)

CHANGELOG.unreleased.md

@@ -41,3 +41,11 @@ Examples:
   ```
 
 -->
+
+- CLI: Fix CI detection to avoid unwanted TTY behavior ([#5804] by [@kachkaev])
+
+  In Prettier 1.16.0 and 1.16.1, `--list-different` and `--check` logged every file in some CI environments, instead of just unformatted files.
+  This unwanted behavior is now fixed.
+
+[#5804]: https://github.com/prettier/prettier/pull/5804
+[@kachkaev]: https://github.com/kachkaev

package.json

@@ -43,6 +43,7 @@
     "html-styles": "1.0.0",
     "html-tag-names": "1.1.2",
     "ignore": "3.3.7",
+    "is-ci": "2.0.0",
     "jest-docblock": "23.2.0",
     "json-stable-stringify": "1.0.1",
     "leven": "2.1.0",

src/cli/util.js

@@ -19,6 +19,7 @@ const optionsModule = require("../main/options");
 const optionsNormalizer = require("../main/options-normalizer");
 const thirdParty = require("../common/third-party");
 const arrayify = require("../utils/arrayify");
+const isTTY = require("../utils/is-tty");
 
 const OPTION_USAGE_THRESHOLD = 25;
 const CHOICE_USAGE_MARGIN = 3;
@@ -54,7 +55,7 @@ function diff(a, b) {
 
 function handleError(context, filename, error) {
   if (error instanceof errors.UndefinedParserError) {
-    if (context.argv["write"] && process.stdout.isTTY) {
+    if (context.argv["write"] && isTTY()) {
       readline.clearLine(process.stdout, 0);
       readline.cursorTo(process.stdout, 0, null);
     }
@@ -459,7 +460,7 @@ function formatFiles(context) {
       return;
     }
 
-    if (process.stdout.isTTY) {
+    if (isTTY()) {
       // Don't use `console.log` here since we need to replace this line.
       context.logger.log(filename, { newline: false });
     }
@@ -503,7 +504,7 @@ function formatFiles(context) {
 
     const isDifferent = output !== input;
 
-    if (process.stdout.isTTY) {
+    if (isTTY()) {
       // Remove previously printed filename to log it with duration.
       readline.clearLine(process.stdout, 0);
       readline.cursorTo(process.stdout, 0, null);

src/common/third-party.js

@@ -1,11 +1,13 @@
 "use strict";
 
-const getStream = require("get-stream");
 const cosmiconfig = require("cosmiconfig");
 const findParentDir = require("find-parent-dir").sync;
+const getStream = require("get-stream");
+const isCIValue = require("is-ci");
 
 module.exports = {
-  getStream,
   cosmiconfig,
-  findParentDir
+  findParentDir,
+  getStream,
+  isCI: /* istanbul ignore next */ () => isCIValue
 };

src/utils/is-tty.js

@@ -0,0 +1,10 @@
+"use strict";
+
+const thirdParty = require("../common/third-party");
+
+// Some CI pipelines incorrectly report process.stdout.isTTY status,
+// which causes unwanted lines in the output. An additional check for isCI() helps.
+// See https://github.com/prettier/prettier/issues/5801
+module.exports = function isTTY() {
+  return process.stdout.isTTY && !thirdParty.isCI();
+};

tests_integration/__tests__/__snapshots__/check.js.snap

@@ -1,5 +1,27 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`--checks works in CI just as in a non-TTY mode (stderr) 1`] = `""`;
+
+exports[`--checks works in CI just as in a non-TTY mode (stderr) 2`] = `""`;
+
+exports[`--checks works in CI just as in a non-TTY mode (stdout) 1`] = `
+"Checking formatting...
+unformatted.js
+Code style issues found in the above file(s). Forgot to run Prettier?
+"
+`;
+
+exports[`--checks works in CI just as in a non-TTY mode (stdout) 2`] = `
+"Checking formatting...
+unformatted.js
+Code style issues found in the above file(s). Forgot to run Prettier?
+"
+`;
+
+exports[`--checks works in CI just as in a non-TTY mode (write) 1`] = `Array []`;
+
+exports[`--checks works in CI just as in a non-TTY mode (write) 2`] = `Array []`;
+
 exports[`checks stdin with --check (write) 1`] = `Array []`;
 
 exports[`checks stdin with -c (alias for --check) (write) 1`] = `Array []`;

tests_integration/__tests__/__snapshots__/list-different.js.snap

@@ -1,5 +1,23 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`--list-different works in CI just as in a non-TTY mode (stderr) 1`] = `""`;
+
+exports[`--list-different works in CI just as in a non-TTY mode (stderr) 2`] = `""`;
+
+exports[`--list-different works in CI just as in a non-TTY mode (stdout) 1`] = `
+"unformatted.js
+"
+`;
+
+exports[`--list-different works in CI just as in a non-TTY mode (stdout) 2`] = `
+"unformatted.js
+"
+`;
+
+exports[`--list-different works in CI just as in a non-TTY mode (write) 1`] = `Array []`;
+
+exports[`--list-different works in CI just as in a non-TTY mode (write) 2`] = `Array []`;
+
 exports[`checks stdin with --list-different (write) 1`] = `Array []`;
 
 exports[`checks stdin with -l (alias for --list-different) (write) 1`] = `Array []`;

tests_integration/__tests__/check.js

@@ -21,3 +21,30 @@ describe("checks stdin with -c (alias for --check)", () => {
     status: "non-zero"
   });
 });
+
+describe("--checks works in CI just as in a non-TTY mode", () => {
+  const result0 = runPrettier(
+    "cli/write",
+    ["--check", "formatted.js", "unformatted.js"],
+    {
+      env: {
+        CI: "true"
+      },
+      stdoutIsTTY: true
+    }
+  ).test({
+    status: 1
+  });
+
+  const result1 = runPrettier(
+    "cli/write",
+    ["--check", "formatted.js", "unformatted.js"],
+    {
+      stdoutIsTTY: false
+    }
+  ).test({
+    status: 1
+  });
+
+  expect(result0.stdout).toEqual(result1.stdout);
+});

tests_integration/__tests__/list-different.js

@@ -21,3 +21,30 @@ describe("checks stdin with -l (alias for --list-different)", () => {
     status: "non-zero"
   });
 });
+
+describe("--list-different works in CI just as in a non-TTY mode", () => {
+  const result0 = runPrettier(
+    "cli/write",
+    ["--list-different", "formatted.js", "unformatted.js"],
+    {
+      env: {
+        CI: "true"
+      },
+      stdoutIsTTY: true
+    }
+  ).test({
+    status: 1
+  });
+
+  const result1 = runPrettier(
+    "cli/write",
+    ["--list-different", "formatted.js", "unformatted.js"],
+    {
+      stdoutIsTTY: false
+    }
+  ).test({
+    status: 1
+  });
+
+  expect(result0.stdout).toEqual(result1.stdout);
+});

tests_integration/runPrettier.js

@@ -60,11 +60,13 @@ function runPrettier(dir, args, options) {
   const originalExitCode = process.exitCode;
   const originalStdinIsTTY = process.stdin.isTTY;
   const originalStdoutIsTTY = process.stdout.isTTY;
+  const originalEnv = process.env;
 
   process.chdir(normalizeDir(dir));
   process.stdin.isTTY = !!options.isTTY;
   process.stdout.isTTY = !!options.stdoutIsTTY;
   process.argv = ["path/to/node", "path/to/prettier/bin"].concat(args);
+  process.env = Object.assign({}, process.env, options.env);
 
   jest.resetModules();
 
@@ -74,6 +76,9 @@ function runPrettier(dir, args, options) {
   jest.spyOn(require(thirdParty), "getStream").mockImplementation(() => ({
     then: handler => handler(options.input || "")
   }));
+  jest
+    .spyOn(require(thirdParty), "isCI")
+    .mockImplementation(() => process.env.CI);
   jest
     .spyOn(require(thirdParty), "cosmiconfig")
     .mockImplementation((moduleName, options) =>
@@ -98,6 +103,7 @@ function runPrettier(dir, args, options) {
     process.exitCode = originalExitCode;
     process.stdin.isTTY = originalStdinIsTTY;
     process.stdout.isTTY = originalStdoutIsTTY;
+    process.env = originalEnv;
     jest.restoreAllMocks();
   }
 

yarn.lock

@@ -1456,6 +1456,11 @@ ci-info@^1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-1.0.0.tgz#dc5285f2b4e251821683681c381c3388f46ec534"
 
+ci-info@^2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/ci-info/-/ci-info-2.0.0.tgz#67a9e964be31a51e15e5010d58e6f12834002f46"
+  integrity sha512-5tK7EtrZ0N+OLFMthtqOj4fI2Jeb88C4CAZPu25LDVUgXJ0A3Js4PMGqrn0JU1W0Mh1/Z8wZzYPxqUrXeBboCQ==
+
 cipher-base@^1.0.0, cipher-base@^1.0.1, cipher-base@^1.0.3:
   version "1.0.3"
   resolved "https://registry.yarnpkg.com/cipher-base/-/cipher-base-1.0.3.tgz#eeabf194419ce900da3018c207d212f2a6df0a07"
@@ -3009,6 +3014,13 @@ is-callable@^1.1.1, is-callable@^1.1.3:
   version "1.1.3"
   resolved "https://registry.yarnpkg.com/is-callable/-/is-callable-1.1.3.tgz#86eb75392805ddc33af71c92a0eedf74ee7604b2"
 
+is-ci@2.0.0:
+  version "2.0.0"
+  resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-2.0.0.tgz#6bc6334181810e04b5c22b3d589fdca55026404c"
+  integrity sha512-YfJT7rkpQB0updsdHLGWrvhBJfcfzNNawYDNIyQXJz0IViGf75O8EBPKSdvw2rF+LGCsX4FZ8tcr3b19LcZq4w==
+  dependencies:
+    ci-info "^2.0.0"
+
 is-ci@^1.0.10:
   version "1.0.10"
   resolved "https://registry.yarnpkg.com/is-ci/-/is-ci-1.0.10.tgz#f739336b2632365061a9d48270cd56ae3369318e"