Skip to content

Commit

Permalink
prefer-event-target: Ignore EventEmitter from @angular/core and…
Browse files Browse the repository at this point in the history
… `eventemitter3` (#2197)
  • Loading branch information
fisker authored Sep 23, 2023
1 parent fa431ff commit 1629ebe
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 17 deletions.
78 changes: 77 additions & 1 deletion rules/prefer-event-target.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,82 @@
'use strict';
const {findVariable} = require('@eslint-community/eslint-utils');
const {getAncestor} = require('./utils/index.js');
const {isStaticRequire, isStringLiteral, isMemberExpression} = require('./ast/index.js');

const MESSAGE_ID = 'prefer-event-target';
const messages = {
[MESSAGE_ID]: 'Prefer `EventTarget` over `EventEmitter`.',
};

const packagesShouldBeIgnored = new Set([
'@angular/core',
'eventemitter3',
]);

const isConstVariableDeclarationId = node =>
node.parent.type === 'VariableDeclarator'
&& node.parent.id === node
&& node.parent.parent.type === 'VariableDeclaration'
&& node.parent.parent.kind === 'const'
&& node.parent.parent.declarations.includes(node.parent);

function isAwaitImportOrRequireFromIgnoredPackages(node) {
if (!node) {
return false;
}

let source;
if (isStaticRequire(node)) {
[source] = node.arguments;
} else if (node.type === 'AwaitExpression' && node.argument.type === 'ImportExpression') {
({source} = node.argument);
}

if (isStringLiteral(source) && packagesShouldBeIgnored.has(source.value)) {
return true;
}

return false;
}

function isFromIgnoredPackage(node) {
if (!node) {
return false;
}

const importDeclaration = getAncestor(node, 'ImportDeclaration');
if (packagesShouldBeIgnored.has(importDeclaration?.source.value)) {
return true;
}

// `const {EventEmitter} = ...`
if (
node.parent.type === 'Property'
&& node.parent.value === node
&& node.parent.key.type === 'Identifier'
&& node.parent.key.name === 'EventEmitter'
&& node.parent.parent.type === 'ObjectPattern'
&& node.parent.parent.properties.includes(node.parent)
&& isConstVariableDeclarationId(node.parent.parent)
&& isAwaitImportOrRequireFromIgnoredPackages(node.parent.parent.parent.init)
) {
return true;
}

// `const EventEmitter = (...).EventEmitter`
if (
isConstVariableDeclarationId(node)
&& isMemberExpression(node.parent.init, {property: 'EventEmitter', optional: false, computed: false})
&& isAwaitImportOrRequireFromIgnoredPackages(node.parent.init.object)
) {
return true;
}

return false;
}

/** @param {import('eslint').Rule.RuleContext} context */
const create = () => ({
const create = context => ({
Identifier(node) {
if (!(
node.name === 'EventEmitter'
Expand All @@ -21,6 +91,12 @@ const create = () => ({
return;
}

const scope = context.sourceCode.getScope(node);
const variableNode = findVariable(scope, node)?.defs[0]?.name;
if (isFromIgnoredPackage(variableNode)) {
return;
}

return {
node,
messageId: MESSAGE_ID,
Expand Down
20 changes: 20 additions & 0 deletions rules/utils/get-ancestor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
'use strict';

// TODO: Support more types
function getPredicate(options) {
if (typeof options === 'string') {
return node => node.type === options;
}
}

function getAncestor(node, options) {
const predicate = getPredicate(options);

for (;node.parent; node = node.parent) {
if (predicate(node)) {
return node;
}
}
}

module.exports = getAncestor;
1 change: 1 addition & 0 deletions rules/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,6 @@ module.exports = {
shouldAddParenthesesToSpreadElementArgument: require('./should-add-parentheses-to-spread-element-argument.js'),
singular: require('./singular.js'),
toLocation: require('./to-location.js'),
getAncestor: require('./get-ancestor.js'),
};

35 changes: 21 additions & 14 deletions test/prefer-event-target.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,24 @@ test.snapshot({
'const Foo = class EventEmitter extends Foo {}',
'new Foo(EventEmitter)',
'new foo.EventEmitter()',
...[
'import {EventEmitter} from "@angular/core";',
'const {EventEmitter} = require("@angular/core");',
'const EventEmitter = require("@angular/core").EventEmitter;',
'import {EventEmitter} from "eventemitter3";',
'const {EventEmitter} = await import("eventemitter3");',
'const EventEmitter = (await import("eventemitter3")).EventEmitter;',
].map(code => outdent`
${code}
class Foo extends EventEmitter {}
`),
'EventTarget()',
'new EventTarget',
'const target = new EventTarget;',
'const target = EventTarget()',
'const target = new Foo(EventEmitter);',
'EventEmitter()',
'const emitter = EventEmitter()',
],
invalid: [
'class Foo extends EventEmitter {}',
Expand All @@ -28,21 +46,10 @@ test.snapshot({
removeListener() {}
}
`,
],
});

test.snapshot({
valid: [
'EventTarget()',
'new EventTarget',
'const target = new EventTarget;',
'const target = EventTarget()',
'const target = new Foo(EventEmitter);',
'EventEmitter()',
'const emitter = EventEmitter()',
],
invalid: [
'new EventEmitter',
'const emitter = new EventEmitter;',
// For coverage
'for (const {EventEmitter} of []) {new EventEmitter}',
'for (const EventEmitter of []) {new EventEmitter}',
],
});
24 changes: 22 additions & 2 deletions test/snapshots/prefer-event-target.mjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ Generated by [AVA](https://avajs.dev).
4 | }␊
`

## Invalid #1
## Invalid #5
1 | new EventEmitter

> Error 1/1
Expand All @@ -60,7 +60,7 @@ Generated by [AVA](https://avajs.dev).
| ^^^^^^^^^^^^ Prefer \`EventTarget\` over \`EventEmitter\`.␊
`

## Invalid #2
## Invalid #6
1 | const emitter = new EventEmitter;

> Error 1/1
Expand All @@ -69,3 +69,23 @@ Generated by [AVA](https://avajs.dev).
> 1 | const emitter = new EventEmitter;␊
| ^^^^^^^^^^^^ Prefer \`EventTarget\` over \`EventEmitter\`.␊
`

## Invalid #7
1 | for (const {EventEmitter} of []) {new EventEmitter}

> Error 1/1
`␊
> 1 | for (const {EventEmitter} of []) {new EventEmitter}␊
| ^^^^^^^^^^^^ Prefer \`EventTarget\` over \`EventEmitter\`.␊
`

## Invalid #8
1 | for (const EventEmitter of []) {new EventEmitter}

> Error 1/1
`␊
> 1 | for (const EventEmitter of []) {new EventEmitter}␊
| ^^^^^^^^^^^^ Prefer \`EventTarget\` over \`EventEmitter\`.␊
`
Binary file modified test/snapshots/prefer-event-target.mjs.snap
Binary file not shown.

0 comments on commit 1629ebe

Please sign in to comment.