Repository: eslint/eslint
Pull request #9214 (2017-09-02 23:28:55)

Chore: avoid storing list of problems on Linter instance (refs #9161)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[x] Other, please explain:

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

This updates `Linter#verify` to create a new array of problems rather than storing problems in `this.messages`. As a result, the private `messages` property can be removed.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


eslintbot: LGTM

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6c5bc73b..31665a03a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,10 @@
+v4.6.1 - September 3, 2017
+
+* bdec46d Build: avoid process leak when generating website (#9217) (Teddy Katz)
+* cb74b87 Fix: avoid adding globals when an env is used with `false` (fixes #9202) (#9203) (Teddy Katz)
+* f9b7544 Docs: Correct a typo in generator-star-spacing documentation (#9205) (Ethan Rutherford)
+* e5c5e83 Build: Fixing issue with docs generation (Fixes #9199) (#9200) (Ilya Volodin)
+
 v4.6.0 - September 1, 2017
 
 * 56dd769 Docs: fix link format in prefer-arrow-callback.md (#9198) (Vse Mozhet Byt)
diff --git a/Makefile.js b/Makefile.js
index 074739f3c..7e22731d2 100644
--- a/Makefile.js
+++ b/Makefile.js
@@ -3,7 +3,7 @@
  * @author nzakas
  */
 
-/* global cat, cd, cp, echo, exec, exit, find, ls, mkdir, pwd, rm, target, test*/
+/* global target */
 /* eslint no-use-before-define: "off", no-console: "off" */
 "use strict";
 
@@ -24,10 +24,24 @@ const lodash = require("lodash"),
     os = require("os"),
     path = require("path"),
     semver = require("semver"),
+    shell = require("shelljs"),
     ejs = require("ejs"),
     loadPerf = require("load-perf"),
     yaml = require("js-yaml");
 
+const cat = shell.cat;
+const cd = shell.cd;
+const cp = shell.cp;
+const echo = shell.echo;
+const exec = shell.exec;
+const exit = shell.exit;
+const find = shell.find;
+const ls = shell.ls;
+const mkdir = shell.mkdir;
+const pwd = shell.pwd;
+const rm = shell.rm;
+const test = shell.test;
+
 //------------------------------------------------------------------------------
 // Settings
 //------------------------------------------------------------------------------
@@ -146,7 +160,7 @@ function generateRulesIndex(basedir) {
  * @returns {string} The result of the executed command.
  */
 function execSilent(cmd) {
-    return exec(cmd, { silent: true }).output;
+    return exec(cmd, { silent: true }).stdout;
 }
 
 /**
@@ -329,7 +343,7 @@ function getFirstCommitOfFile(filePath) {
  * @param {string} filePath The file path to check.
  * @returns {string} The tag name.
  */
-function getTagOfFirstOccurrence(filePath) {
+function getFirstVersionOfFile(filePath) {
     const firstCommit = getFirstCommitOfFile(filePath);
     let tags = execSilent(`git tag --contains ${firstCommit}`);
 
@@ -343,15 +357,6 @@ function getTagOfFirstOccurrence(filePath) {
     }, []).sort(semver.compare)[0];
 }
 
-/**
- * Gets the version number where a given file was introduced first.
- * @param {string} filePath The file path to check.
- * @returns {string} The version number.
- */
-function getFirstVersionOfFile(filePath) {
-    return getTagOfFirstOccurrence(filePath);
-}
-
 /**
  * Gets the commit that deleted a file.
  * @param {string} filePath The path to the deleted file.
diff --git a/docs/rules/generator-star-spacing.md b/docs/rules/generator-star-spacing.md
index efa7a5a4e..95af6de80 100644
--- a/docs/rules/generator-star-spacing.md
+++ b/docs/rules/generator-star-spacing.md
@@ -93,7 +93,7 @@ An example of a configuration with overrides:
 ```
 
 In the example configuration above, the top level "before" and "after" options define the default behavior of
-the rule, while the "anonymous", "method", and "static" options override the default behavior.
+the rule, while the "anonymous" and "method" options override the default behavior.
 Overrides can be either an object with "before" and "after", or a shorthand string as above.
 
 ## Examples
diff --git a/lib/linter.js b/lib/linter.js
index 7e32eedb9..5011577f7 100755
--- a/lib/linter.js
+++ b/lib/linter.js
@@ -178,7 +178,7 @@ function addDeclaredGlobals(program, globalScope, config, envContext) {
 
     Object.assign(declaredGlobals, builtin);
 
-    Object.keys(config.env).forEach(name => {
+    Object.keys(config.env).filter(name => config.env[name]).forEach(name => {
         const env = envContext.get(name),
             environmentGlobals = env && env.globals;
 
@@ -761,7 +761,6 @@ const BASE_TRAVERSAL_CONTEXT = Object.freeze(
 class Linter {
 
     constructor() {
-        this.messages = [];
         this.currentConfig = null;
         this.scopeManager = null;
         this.currentFilename = null;
@@ -779,7 +778,6 @@ class Linter {
      * @returns {void}
      */
     reset() {
-        this.messages = [];
         this.currentConfig = null;
         this.scopeManager = null;
         this.traverser = null;
@@ -878,12 +876,14 @@ class Linter {
             this.sourceCode = new SourceCode(text, parseResult.ast);
         }
 
+        const problems = [];
+
         // parse global comments and modify config
         if (allowInlineConfig !== false) {
             const modifyConfigResult = modifyConfigsFromComments(this.currentFilename, this.sourceCode.ast, config, this);
 
             config = modifyConfigResult.config;
-            modifyConfigResult.problems.forEach(problem => this.messages.push(problem));
+            modifyConfigResult.problems.forEach(problem => problems.push(problem));
         }
 
         // ensure that severities are normalized in the config
@@ -955,9 +955,7 @@ class Linter {
                                 if (problem.fix && ruleCreator.meta && !ruleCreator.meta.fixable) {
                                     throw new Error("Fixable rules should export a `meta.fixable` property.");
                                 }
-                                if (!isDisabledByReportingConfig(this.reportingConfig, ruleId, problem)) {
-                                    this.messages.push(problem);
-                                }
+                                problems.push(problem);
 
                                 /*
                                  * This is used to avoid breaking rules that used monkeypatch Linter, and relied on
@@ -1039,18 +1037,9 @@ class Linter {
             }
         });
 
-        // sort by line and column
-        this.messages.sort((a, b) => {
-            const lineDiff = a.line - b.line;
-
-            if (lineDiff === 0) {
-                return a.column - b.column;
-            }
-            return lineDiff;
-
-        });
-
-        return this.messages;
+        return problems
+            .filter(problem => !isDisabledByReportingConfig(this.reportingConfig, problem.ruleId, problem))
+            .sort((problemA, problemB) => problemA.line - problemB.line || problemA.column - problemB.column);
     }
 
     /**
diff --git a/package.json b/package.json
index 723759b11..5bcfc2803 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "eslint",
-  "version": "4.6.0",
+  "version": "4.6.1",
   "author": "Nicholas C. Zakas <nicholas+npm@nczconsulting.com>",
   "description": "An AST-based pattern checker for JavaScript.",
   "bin": {


Test patch:
diff --git a/tests/lib/linter.js b/tests/lib/linter.js
index 2096f5a63..900e02d51 100644
--- a/tests/lib/linter.js
+++ b/tests/lib/linter.js
@@ -2711,35 +2711,48 @@ describe("Linter", () => {
     describe("verify()", () => {
         describe("filenames", () => {
             it("should allow filename to be passed on options object", () => {
+                const filenameChecker = sandbox.spy(context => {
+                    assert.strictEqual(context.getFilename(), "foo.js");
+                    return {};
+                });
 
-                linter.verify("foo;", {}, { filename: "foo.js" });
-                const result = linter.getFilename();
-
-                assert.equal(result, "foo.js");
+                linter.defineRule("checker", filenameChecker);
+                linter.defineRule("checker", filenameChecker);
+                linter.verify("foo;", { rules: { checker: "error" } }, { filename: "foo.js" });
+                assert(filenameChecker.calledOnce);
             });
 
             it("should allow filename to be passed as third argument", () => {
+                const filenameChecker = sandbox.spy(context => {
+                    assert.strictEqual(context.getFilename(), "bar.js");
+                    return {};
+                });
 
-                linter.verify("foo;", {}, "foo.js");
-                const result = linter.getFilename();
-
-                assert.equal(result, "foo.js");
+                linter.defineRule("checker", filenameChecker);
+                linter.verify("foo;", { rules: { checker: "error" } }, "bar.js");
+                assert(filenameChecker.calledOnce);
             });
 
             it("should default filename to <input> when options object doesn't have filename", () => {
+                const filenameChecker = sandbox.spy(context => {
+                    assert.strictEqual(context.getFilename(), "<input>");
+                    return {};
+                });
 
-                linter.verify("foo;", {}, {});
-                const result = linter.getFilename();
-
-                assert.equal(result, "<input>");
+                linter.defineRule("checker", filenameChecker);
+                linter.verify("foo;", { rules: { checker: "error" } }, {});
+                assert(filenameChecker.calledOnce);
             });
 
             it("should default filename to <input> when only two arguments are passed", () => {
+                const filenameChecker = sandbox.spy(context => {
+                    assert.strictEqual(context.getFilename(), "<input>");
+                    return {};
+                });
 
-                linter.verify("foo;", {});
-                const result = linter.getFilename();
-
-                assert.equal(result, "<input>");
+                linter.defineRule("checker", filenameChecker);
+                linter.verify("foo;", { rules: { checker: "error" } });
+                assert(filenameChecker.calledOnce);
             });
         });
 
diff --git a/tests/lib/rules/no-catch-shadow.js b/tests/lib/rules/no-catch-shadow.js
index a302ae86b..3d1024f9e 100644
--- a/tests/lib/rules/no-catch-shadow.js
+++ b/tests/lib/rules/no-catch-shadow.js
@@ -36,6 +36,10 @@ ruleTester.run("no-catch-shadow", rule, {
                 "module.exports = broken;"
             ].join("\n"),
             parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: "try {} catch (error) {}",
+            env: { shelljs: false }
         }
     ],
     invalid: [
diff --git a/tests/lib/rules/no-redeclare.js b/tests/lib/rules/no-redeclare.js
index 3d67961ed..2cd0f8ef5 100644
--- a/tests/lib/rules/no-redeclare.js
+++ b/tests/lib/rules/no-redeclare.js
@@ -35,7 +35,12 @@ ruleTester.run("no-redeclare", rule, {
         { code: "var top = 0;", env: { browser: true } },
         { code: "var top = 0;", options: [{ builtinGlobals: true }] },
         { code: "var top = 0;", options: [{ builtinGlobals: true }], parserOptions: { ecmaFeatures: { globalReturn: true } }, env: { browser: true } },
-        { code: "var top = 0;", options: [{ builtinGlobals: true }], parserOptions: { sourceType: "module" }, env: { browser: true } }
+        { code: "var top = 0;", options: [{ builtinGlobals: true }], parserOptions: { sourceType: "module" }, env: { browser: true } },
+        {
+            code: "var self = 1",
+            options: [{ builtinGlobals: true }],
+            env: { browser: false }
+        }
     ],
     invalid: [
         { code: "var a = 3; var a = 10;", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "'a' is already defined.", type: "Identifier" }] },