Prevent libs from messing with stdout (codeclimate#229)

* Revert "Revert "Prevent errors from unsupported plugins/rules/modules (codeclimate#215)""

This reverts commit 659472f.

* Prevent libs from messing with stdout

* Keep it testable with no performance penalty

Author: filipesperandio

.codeclimate.yml

@@ -15,3 +15,4 @@ ratings:
 exclude_paths:
 - "node_modules/**"
 - "test/**"
+- "integration/**"

Makefile

@@ -1,12 +1,25 @@
-.PHONY: image test citest
+.PHONY: image test citest integration
 
 IMAGE_NAME ?= codeclimate/codeclimate-eslint
 
 NPM_TEST_TARGET ?= test
+NPM_INTEGRATION_TARGET ?= integration
+
+DEBUG ?= false
+ifeq ($(DEBUG),true)
+	NPM_TEST_TARGET = test.debug
+	NPM_INTEGRATION_TARGET = integration.debug
+endif
 
 image:
 	docker build --rm -t $(IMAGE_NAME) .
 
+integration: image
+	docker run -ti --rm \
+		--volume $(PWD):/code \
+		--workdir /code \
+		$(IMAGE_NAME) npm run $(NPM_INTEGRATION_TARGET)
+
 test: image
 	docker run -ti --rm \
 		--volume $(PWD):/code \
@@ -16,4 +29,4 @@ test: image
 citest:
 	docker run --rm \
 		--workdir /usr/src/app \
-		$(IMAGE_NAME) npm run test
+		$(IMAGE_NAME) sh -c "npm run test && npm run integration"

bin/eslint.js

@@ -1,243 +1,7 @@
 #!/usr/src/app/bin/node_gc
 
-var CODE_DIR = "/code";
+const CODE_DIR = "/code";
 process.chdir(CODE_DIR);
 
-// Redirect `console.log` so that we are the only ones
-// writing to STDOUT
-var stdout = console.log;
-console.log = console.error;
-
-var eslint = require('../lib/eslint-patch')(require('eslint'));
-
-var CLIEngine = eslint.CLIEngine;
-var docs = eslint.docs;
-var fs = require("fs");
-var glob = require("glob");
-var options = { extensions: [".js"], ignore: true, reset: false, useEslintrc: true };
-var cli; // instantiation delayed until after options are (potentially) modified
-var debug = false;
-var BatchSanitizer = require("../lib/batch_sanitizer");
-var ignoreWarnings = false;
-var ESLINT_WARNING_SEVERITY = 1;
-var checks = require("../lib/checks");
-var validateConfig = require("../lib/validate_config");
-var computeFingerprint = require("../lib/compute_fingerprint");
-const ConfigUpgrader = require("../lib/config_upgrader");
-
-// a wrapper for emitting perf timing
-function runWithTiming(name, fn) {
-  var start = new Date()
-    , rv = fn()
-    , duration = (new Date() - start) / 1000;
-  if (debug) {
-    console.error("eslint.timing." + name + ": " + duration + "s");
-  }
-  return rv;
-}
-
-function contentBody(check) {
-  var content = docs.get(check) || "For more information visit ";
-  return content + "Source: http://eslint.org/docs/rules/\n";
-}
-
-function buildIssueJson(message, path) {
-  // ESLint doesn't emit a ruleId in the
-  // case of a fatal error (such as an invalid
-  // token)
-  var checkName = message.ruleId;
-  if(message.fatal) {
-    checkName = "fatal";
-  }
-  var line = message.line || 1;
-  var column = message.column || 1;
-
-  var issue = {
-    type: "issue",
-    categories: checks.categories(checkName),
-    check_name: checkName,
-    description: message.message,
-    content: {
-      body: contentBody(checkName)
-    },
-    location: {
-      path: path,
-      positions: {
-        begin: {
-          line: line,
-          column: column
-        },
-        end: {
-          line: line,
-          column: column
-        }
-      }
-    },
-    remediation_points: checks.remediationPoints(checkName, message, cli.getConfigForFile(path))
-  };
-
-  var fingerprint = computeFingerprint(path, checkName, message.message);
-
-  if (fingerprint) {
-    issue["fingerprint"] = fingerprint;
-  }
-
-  return JSON.stringify(issue);
-}
-
-function isFileWithMatchingExtension(file, extensions) {
-  var stats = fs.lstatSync(file);
-  var extension = "." + file.split(".").pop();
-  return (
-    stats.isFile() &&
-    !stats.isSymbolicLink()
-    && extensions.indexOf(extension) >= 0
-  );
-}
-
-function isFileIgnoredByLibrary(file) {
-  return cli.isPathIgnored(file);
-}
-
-function prunePathsWithinSymlinks(paths) {
-  // Extracts symlinked paths and filters them out, including any child paths
-  var symlinks = paths.filter(function(path) {
-    return fs.lstatSync(path).isSymbolicLink();
-  });
-
-  return paths.filter(function(path) {
-    var withinSymlink = false;
-    symlinks.forEach(function(symlink) {
-      if (path.indexOf(symlink) === 0) {
-        withinSymlink = true;
-      }
-    });
-    return !withinSymlink;
-  });
-}
-
-function inclusionBasedFileListBuilder(includePaths) {
-  // Uses glob to expand the files and directories in includePaths, filtering
-  // down to match the list of desired extensions.
-  return function(extensions) {
-    var analysisFiles = [];
-
-    includePaths.forEach(function(fileOrDirectory, i) {
-      if ((/\/$/).test(fileOrDirectory)) {
-        // if it ends in a slash, expand and push
-        var filesInThisDirectory = glob.sync(
-          fileOrDirectory + "/**/**"
-        );
-        prunePathsWithinSymlinks(filesInThisDirectory).forEach(function(file, j){
-          if (!isFileIgnoredByLibrary(file) && isFileWithMatchingExtension(file, extensions)) {
-            analysisFiles.push(file);
-          }
-        });
-      } else {
-        if (!isFileIgnoredByLibrary(fileOrDirectory) && isFileWithMatchingExtension(fileOrDirectory, extensions)) {
-          analysisFiles.push(fileOrDirectory);
-        }
-      }
-    });
-
-    return analysisFiles;
-  };
-}
-
-var buildFileList;
-runWithTiming("engineConfig", function () {
-  if (fs.existsSync("/config.json")) {
-    var engineConfig = JSON.parse(fs.readFileSync("/config.json"));
-
-    if (engineConfig.include_paths) {
-      buildFileList = inclusionBasedFileListBuilder(
-        engineConfig.include_paths
-      );
-    } else {
-      // No explicit includes, let's try with everything
-      buildFileList = inclusionBasedFileListBuilder(["./"]);
-    }
-
-    var userConfig = engineConfig.config || {};
-    if (userConfig.config) {
-      options.configFile = CODE_DIR + "/" + userConfig.config;
-      options.useEslintrc = false;
-    }
-
-    if (userConfig.extensions) {
-      options.extensions = userConfig.extensions;
-    }
-
-    if (userConfig.ignore_path) {
-      options.ignorePath = userConfig.ignore_path;
-    }
-
-    if (userConfig.ignore_warnings) {
-      ignoreWarnings = true;
-    }
-
-    if (userConfig.debug) {
-      debug = true;
-    }
-  }
-
-  cli = new CLIEngine(options);
-});
-
-var analysisFiles = runWithTiming("buildFileList", function() {
-  return buildFileList(options.extensions);
-});
-
-function analyzeFiles() {
-  var batchNum = 0
-    , batchSize = 10
-    , batchFiles
-    , batchReport
-    , sanitizedBatchFiles;
-
-  while(analysisFiles.length > 0) {
-    batchFiles = analysisFiles.splice(0, batchSize);
-    sanitizedBatchFiles = (new BatchSanitizer(batchFiles)).sanitizedFiles();
-
-    if (debug) {
-      process.stderr.write("Analyzing: " + batchFiles + "\n");
-    }
-
-    runWithTiming("analyze-batch-" + batchNum, function() {
-       batchReport = cli.executeOnFiles(sanitizedBatchFiles);
-    });
-    runWithTiming("report-batch" + batchNum, function() {
-      batchReport.results.forEach(function(result) {
-        var path = result.filePath.replace(/^\/code\//, "");
-
-        result.messages.forEach(function(message) {
-          if (ignoreWarnings && message.severity === ESLINT_WARNING_SEVERITY) { return; }
-
-          var issueJson = buildIssueJson(message, path);
-          process.stdout.write(issueJson + "\u0000\n");
-        });
-      });
-    });
-    runWithTiming("gc-batch-" + batchNum, function() {
-      batchFiles = null;
-      batchReport = null;
-      global.gc();
-    });
-
-    batchNum++;
-  }
-}
-
-if (validateConfig(options.configFile)) {
-  console.error("ESLint is running with the " + cli.getConfigForFile(null).parser + " parser.");
-
-  for (const line of ConfigUpgrader.upgradeInstructions(options.configFile, analysisFiles, process.cwd())) {
-    console.error(line);
-  }
-
-  analyzeFiles();
-} else {
-  console.error("No rules are configured. Make sure you have added a config file with rules enabled.");
-  console.error("See our documentation at https://docs.codeclimate.com/docs/eslint for more information.");
-  process.exit(1);
-}
+const ESLint = require("../lib/eslint");
+ESLint.run(console, { dir: CODE_DIR });

integration/empty_config/config.json

@@ -0,0 +1,10 @@
+{
+  "enabled": true,
+  "config": {
+    "config": "empty_config/eslintrc.yml",
+    "debug": "true"
+  },
+  "include_paths": [
+    "/usr/src/app/integration/empty_config/index.js"
+  ]
+}

integration/empty_config/index.js

@@ -0,0 +1,2 @@
+function dummy() {
+}

integration/eslint_test.js

@@ -0,0 +1,64 @@
+const sinon = require("sinon");
+const expect = require("chai").expect;
+
+const ESLint = require('../lib/eslint');
+
+describe("eslint integration", function() {
+  let consoleMock = {};
+
+  function executeConfig(configPath) {
+    return ESLint.run(consoleMock, { dir: __dirname, configPath: `${__dirname}/${configPath}`});
+  }
+
+  beforeEach(function() {
+    consoleMock.output = [];
+    consoleMock.log = function(msg) { consoleMock.output.push(msg) };
+    consoleMock.error = sinon.spy();
+  });
+
+  describe("eslintrc has not supported plugins", function() {
+    it("does not raise any error", function() {
+      this.timeout(3000);
+
+      function executeUnsupportedPlugins() {
+        executeConfig("with_unsupported_plugins/config.json");
+      }
+
+      expect(executeUnsupportedPlugins).to.not.throw();
+      expect(consoleMock.output).to.not.be.empty;
+    });
+  });
+
+  describe("validating config", function() {
+    it("warns about empty config but not raise error", function() {
+      function executeEmptyConfig() {
+        executeConfig("empty_config/config.json");
+      }
+
+      expect(executeEmptyConfig).to.not.throw();
+      sinon.assert.calledWith(consoleMock.error, 'No rules are configured. Make sure you have added a config file with rules enabled.');
+    });
+  });
+
+  describe("extends plugin", function() {
+    it("loads the plugin and does not include repeated issues of not found rules", function() {
+      this.timeout(5000);
+      executeConfig("extends_airbnb/config.json");
+
+      const ruleDefinitionIssues = consoleMock.output.filter(function(o) { return o.includes("Definition for rule"); });
+      expect(ruleDefinitionIssues).to.be.empty;
+    });
+  });
+
+  describe("output", function() {
+    it("is not messed up", function() {
+      this.timeout(5000);
+
+      executeConfig("output_mess/config.json");
+
+      expect(consoleMock.output).to.have.lengthOf(1);
+      expect(consoleMock.output[0]).to.match(/^\{.*/);
+    });
+  });
+
+});

integration/extends_airbnb/config.json

@@ -0,0 +1,10 @@
+{
+  "enabled": true,
+  "config": {
+    "config": "extends_airbnb/eslintrc.json",
+    "debug": "true"
+  },
+  "include_paths": [
+    "/usr/src/app/integration/extends_airbnb/index.js"
+  ]
+}

integration/extends_airbnb/eslintrc.json

@@ -0,0 +1,5 @@
+{
+  "extends": "airbnb",
+  "parser": "babel-eslint",
+  "rules": {}
+}