diff --git a/lib/linter.js b/lib/linter.js index a8fa58b9d82..8f1520aafb4 100755 --- a/lib/linter.js +++ b/lib/linter.js @@ -9,8 +9,7 @@ // Requirements //------------------------------------------------------------------------------ -const EventEmitter = require("events").EventEmitter, - eslintScope = require("eslint-scope"), +const eslintScope = require("eslint-scope"), levn = require("levn"), lodash = require("lodash"), blankScriptAST = require("../conf/blank-script.json"), @@ -20,6 +19,7 @@ const EventEmitter = require("events").EventEmitter, validator = require("./config/config-validator"), Environments = require("./config/environments"), applyDisableDirectives = require("./util/apply-disable-directives"), + createEmitter = require("./util/safe-emitter"), NodeEventGenerator = require("./util/node-event-generator"), SourceCode = require("./util/source-code"), Traverser = require("./util/traverser"), @@ -817,7 +817,7 @@ module.exports = class Linter { disableDirectives = []; } - const emitter = new EventEmitter().setMaxListeners(Infinity); + const emitter = createEmitter(); const traverser = new Traverser(); const ecmaFeatures = config.parserOptions.ecmaFeatures || {}; const ecmaVersion = config.parserOptions.ecmaVersion || 5; @@ -860,7 +860,7 @@ module.exports = class Linter { */ _linter: { report() {}, - on: emitter.on.bind(emitter) + on: emitter.on } } ) diff --git a/lib/util/node-event-generator.js b/lib/util/node-event-generator.js index 568a3b7fe10..05b343ae9f2 100644 --- a/lib/util/node-event-generator.js +++ b/lib/util/node-event-generator.js @@ -194,7 +194,7 @@ const parseSelector = lodash.memoize(rawSelector => { * * ```ts * interface EventGenerator { - * emitter: EventEmitter; + * emitter: SafeEmitter; * enterNode(node: ASTNode): void; * leaveNode(node: ASTNode): void; * } @@ -203,8 +203,10 @@ const parseSelector = lodash.memoize(rawSelector => { class NodeEventGenerator { /** - * @param {EventEmitter} emitter - An event emitter which is the destination of events. This emitter must already + * @param {SafeEmitter} emitter + * An SafeEmitter which is the destination of events. This emitter must already * have registered listeners for all of the events that it needs to listen for. + * (See lib/util/safe-emitter.js for more details on `SafeEmitter`.) * @returns {NodeEventGenerator} new instance */ constructor(emitter) { @@ -215,23 +217,7 @@ class NodeEventGenerator { this.anyTypeEnterSelectors = []; this.anyTypeExitSelectors = []; - const eventNames = typeof emitter.eventNames === "function" - - // Use the built-in eventNames() function if available (Node 6+) - ? emitter.eventNames() - - /* - * Otherwise, use the private _events property. - * Using a private property isn't ideal here, but this seems to - * be the best way to get a list of event names without overriding - * addEventListener, which would hurt performance. This property - * is widely used and unlikely to be removed in a future version - * (see https://github.com/nodejs/node/issues/1817). Also, future - * node versions will have eventNames() anyway. - */ - : Object.keys(emitter._events); // eslint-disable-line no-underscore-dangle - - eventNames.forEach(rawSelector => { + emitter.eventNames().forEach(rawSelector => { const selector = parseSelector(rawSelector); if (selector.listenerTypes) { diff --git a/lib/util/safe-emitter.js b/lib/util/safe-emitter.js new file mode 100644 index 00000000000..242cbe42959 --- /dev/null +++ b/lib/util/safe-emitter.js @@ -0,0 +1,54 @@ +/** + * @fileoverview A variant of EventEmitter which does not give listeners information about each other + * @author Teddy Katz + */ + +"use strict"; + +//------------------------------------------------------------------------------ +// Typedefs +//------------------------------------------------------------------------------ + +/** + * An object describing an AST selector + * @typedef {Object} SafeEmitter + * @property {function(eventName: string, listenerFunc: Function): void} on Adds a listener for a given event name + * @property {function(eventName: string, arg1?: any, arg2?: any, arg3?: any)} emit Emits an event with a given name. + * This calls all the listeners that were listening for that name, with `arg1`, `arg2`, and `arg3` as arguments. + * @property {function(): string[]} eventNames Gets the list of event names that have registered listeners. + */ + +/** + * Creates an object which can listen for and emit events. + * This is similar to the EventEmitter API in Node's standard library, but it has a few differences. + * The goal is to allow multiple modules to attach arbitrary listeners to the same emitter, without + * letting the modules know about each other at all. + * 1. It has no special keys like `error` and `newListener`, which would allow modules to detect when + * another module throws an error or registers a listener. + * 2. It calls listener functions without any `this` value. (`EventEmitter` calls listeners with a + * `this` value of the emitter instance, which would give listeners access to other listeners.) + * 3. Events can be emitted with at most 3 arguments. (For example: when using `emitter.emit('foo', a, b, c)`, + * the arguments `a`, `b`, and `c` will be passed to the listener functions.) + * @returns {SafeEmitter} An emitter + */ +module.exports = () => { + const listeners = Object.create(null); + + return Object.freeze({ + on(eventName, listener) { + if (eventName in listeners) { + listeners[eventName].push(listener); + } else { + listeners[eventName] = [listener]; + } + }, + emit(eventName, a, b, c) { + if (eventName in listeners) { + listeners[eventName].forEach(listener => listener(a, b, c)); + } + }, + eventNames() { + return Object.keys(listeners); + } + }); +}; diff --git a/tests/lib/code-path-analysis/code-path-analyzer.js b/tests/lib/code-path-analysis/code-path-analyzer.js index 82384b15731..e9878d6677b 100644 --- a/tests/lib/code-path-analysis/code-path-analyzer.js +++ b/tests/lib/code-path-analysis/code-path-analyzer.js @@ -10,11 +10,11 @@ //------------------------------------------------------------------------------ const assert = require("assert"), - EventEmitter = require("events").EventEmitter, fs = require("fs"), path = require("path"), Linter = require("../../../lib/linter"), EventGeneratorTester = require("../../../tools/internal-testers/event-generator-tester"), + createEmitter = require("../../../lib/util/safe-emitter"), debug = require("../../../lib/code-path-analysis/debug-helpers"), CodePath = require("../../../lib/code-path-analysis/code-path"), CodePathAnalyzer = require("../../../lib/code-path-analysis/code-path-analyzer"), @@ -55,7 +55,7 @@ function getExpectedDotArrows(source) { describe("CodePathAnalyzer", () => { EventGeneratorTester.testEventGeneratorInterface( - new CodePathAnalyzer(new NodeEventGenerator(new EventEmitter())) + new CodePathAnalyzer(new NodeEventGenerator(createEmitter())) ); describe("interface of code paths", () => { diff --git a/tests/lib/linter.js b/tests/lib/linter.js index eda0bd57174..a6352c0f962 100644 --- a/tests/lib/linter.js +++ b/tests/lib/linter.js @@ -100,6 +100,23 @@ describe("Linter", () => { linter.verify(code, config, filename, true); }, "Intentional error."); }); + + it("does not call rule listeners with a `this` value", () => { + const spy = sandbox.spy(); + + linter.defineRule("checker", () => ({ Program: spy })); + linter.verify("foo", { rules: { checker: "error" } }); + assert(spy.calledOnce); + assert.strictEqual(spy.firstCall.thisValue, void 0); + }); + + it("does not allow listeners to use special EventEmitter values", () => { + const spy = sandbox.spy(); + + linter.defineRule("checker", () => ({ newListener: spy })); + linter.verify("foo", { rules: { checker: "error", "no-undef": "error" } }); + assert(spy.notCalled); + }); }); describe("context.getSourceLines()", () => { diff --git a/tests/lib/util/node-event-generator.js b/tests/lib/util/node-event-generator.js index b59fabce7ea..67b6fa53a06 100644 --- a/tests/lib/util/node-event-generator.js +++ b/tests/lib/util/node-event-generator.js @@ -9,11 +9,11 @@ //------------------------------------------------------------------------------ const assert = require("assert"), - EventEmitter = require("events").EventEmitter, sinon = require("sinon"), espree = require("espree"), estraverse = require("estraverse"), EventGeneratorTester = require("../../../tools/internal-testers/event-generator-tester"), + createEmitter = require("../../../lib/util/safe-emitter"), NodeEventGenerator = require("../../../lib/util/node-event-generator"); //------------------------------------------------------------------------------ @@ -30,16 +30,16 @@ const ESPREE_CONFIG = { describe("NodeEventGenerator", () => { EventGeneratorTester.testEventGeneratorInterface( - new NodeEventGenerator(new EventEmitter()) + new NodeEventGenerator(createEmitter()) ); describe("entering a single AST node", () => { let emitter, generator; beforeEach(() => { - emitter = new EventEmitter(); + emitter = Object.create(createEmitter(), { emit: { value: sinon.spy() } }); + ["Foo", "Bar", "Foo > Bar", "Foo:exit"].forEach(selector => emitter.on(selector, () => {})); - emitter.emit = sinon.spy(emitter.emit); generator = new NodeEventGenerator(emitter); }); @@ -82,13 +82,15 @@ describe("NodeEventGenerator", () => { */ function getEmissions(ast, possibleQueries) { const emissions = []; - const emitter = new EventEmitter(); + const emitter = Object.create(createEmitter(), { + emit: { + value: (selector, node) => emissions.push([selector, node]) + } + }); possibleQueries.forEach(query => emitter.on(query, () => {})); const generator = new NodeEventGenerator(emitter); - emitter.emit = (selector, node) => emissions.push([selector, node]); - estraverse.traverse(ast, { enter(node, parent) { node.parent = parent; @@ -308,7 +310,7 @@ describe("NodeEventGenerator", () => { describe("parsing an invalid selector", () => { it("throws a useful error", () => { - const emitter = new EventEmitter(); + const emitter = createEmitter(); emitter.on("Foo >", () => {}); assert.throws( diff --git a/tests/lib/util/safe-emitter.js b/tests/lib/util/safe-emitter.js new file mode 100644 index 00000000000..7b127ea89fd --- /dev/null +++ b/tests/lib/util/safe-emitter.js @@ -0,0 +1,43 @@ +/** + * @fileoverview Tests for safe-emitter + * @author Teddy Katz + */ + +"use strict"; + +const createEmitter = require("../../../lib/util/safe-emitter"); +const assert = require("chai").assert; + +describe("safe-emitter", () => { + describe("emit() and on()", () => { + it("allows listeners to be registered calls them when emitted", () => { + const emitter = createEmitter(); + const colors = []; + + emitter.on("foo", () => colors.push("red")); + emitter.on("foo", () => colors.push("blue")); + emitter.on("bar", () => colors.push("green")); + + emitter.emit("foo"); + assert.deepEqual(colors, ["red", "blue"]); + + emitter.on("bar", color => colors.push(color)); + emitter.emit("bar", "yellow"); + + assert.deepEqual(colors, ["red", "blue", "green", "yellow"]); + }); + + it("calls listeners with no `this` value", () => { + const emitter = createEmitter(); + let called = false; + + emitter.on("foo", function() { + assert.strictEqual(this, void 0); // eslint-disable-line no-invalid-this + called = true; + }); + + emitter.emit("foo"); + assert(called); + }); + }); +});