From 230e1fcf7ecebb28e364a1fb777ee0c74504f021 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 24 Oct 2022 17:21:09 +1030 Subject: [PATCH] feat(eslint-plugin): [no-unsafe-declaration-merging] switch to use scope analysis instead of type information --- .../rules/no-unsafe-declaration-merging.ts | 45 ++++++++++++++----- .../no-unsafe-declaration-merging.test.ts | 16 +++---- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unsafe-declaration-merging.ts b/packages/eslint-plugin/src/rules/no-unsafe-declaration-merging.ts index 7982fe5c5f4..89d68db6e67 100644 --- a/packages/eslint-plugin/src/rules/no-unsafe-declaration-merging.ts +++ b/packages/eslint-plugin/src/rules/no-unsafe-declaration-merging.ts @@ -1,5 +1,6 @@ +import type { Scope } from '@typescript-eslint/scope-manager'; import type { TSESTree } from '@typescript-eslint/utils'; -import * as ts from 'typescript'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as util from '../util'; @@ -10,7 +11,7 @@ export default util.createRule({ docs: { description: 'Disallow unsafe declaration merging', recommended: 'strict', - requiresTypeChecking: true, + requiresTypeChecking: false, }, messages: { unsafeMerging: @@ -20,17 +21,22 @@ export default util.createRule({ }, defaultOptions: [], create(context) { - const parserServices = util.getParserServices(context); - const checker = parserServices.program.getTypeChecker(); - function checkUnsafeDeclaration( + scope: Scope, node: TSESTree.Identifier, - unsafeKind: ts.SyntaxKind, + unsafeKind: AST_NODE_TYPES, ): void { - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); - const type = checker.getTypeAtLocation(tsNode); - const symbol = type.getSymbol(); - if (symbol?.declarations?.some(decl => decl.kind === unsafeKind)) { + const variable = scope.set.get(node.name); + if (!variable) { + return; + } + + const defs = variable.defs; + if (defs.length <= 1) { + return; + } + + if (defs.some(def => def.node.type === unsafeKind)) { context.report({ node, messageId: 'unsafeMerging', @@ -41,11 +47,26 @@ export default util.createRule({ return { ClassDeclaration(node): void { if (node.id) { - checkUnsafeDeclaration(node.id, ts.SyntaxKind.InterfaceDeclaration); + // by default eslint returns the inner class scope for the ClassDeclaration node + // but we want the outer scope within which merged variables will sit + const currentScope = context.getScope().upper; + if (currentScope == null) { + return; + } + + checkUnsafeDeclaration( + currentScope, + node.id, + AST_NODE_TYPES.TSInterfaceDeclaration, + ); } }, TSInterfaceDeclaration(node): void { - checkUnsafeDeclaration(node.id, ts.SyntaxKind.ClassDeclaration); + checkUnsafeDeclaration( + context.getScope(), + node.id, + AST_NODE_TYPES.ClassDeclaration, + ); }, }; }, diff --git a/packages/eslint-plugin/tests/rules/no-unsafe-declaration-merging.test.ts b/packages/eslint-plugin/tests/rules/no-unsafe-declaration-merging.test.ts index e92c1ed36e7..1feabbe8158 100644 --- a/packages/eslint-plugin/tests/rules/no-unsafe-declaration-merging.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unsafe-declaration-merging.test.ts @@ -80,23 +80,19 @@ class Foo {} }, { code: ` -namespace Foo { - export interface Bar {} -} -namespace Foo { - export class Bar {} -} +class Foo {} +interface Foo {} `, errors: [ { messageId: 'unsafeMerging', - line: 3, - column: 20, + line: 2, + column: 7, }, { messageId: 'unsafeMerging', - line: 6, - column: 16, + line: 3, + column: 11, }, ], },