Skip to content

Commit

Permalink
feat(eslint-plugin): [no-shadow] exclude external type declaration me…
Browse files Browse the repository at this point in the history
…rging (#3959)

* fix(eslint-plugin): [no-shadow] exclude external type declaration merging

* fix(eslint-plugin): [no-shadow] typo

* Apply suggestions from code review

Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>

* fix(eslint-plugin): lint - remove isStringLiteral (no longer used)

Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
  • Loading branch information
idan-at and JoshuaKGoldberg committed Nov 1, 2021
1 parent 5229597 commit a93cebf
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 2 deletions.
49 changes: 47 additions & 2 deletions packages/eslint-plugin/src/rules/no-shadow.ts
Expand Up @@ -94,10 +94,10 @@ export default util.createRule<Options, MessageIds>({
}

function isTypeImport(
definition: Definition,
definition?: Definition,
): definition is ImportBindingDefinition {
return (
definition.type === DefinitionType.ImportBinding &&
definition?.type === DefinitionType.ImportBinding &&
definition.parent.importKind === 'type'
);
}
Expand Down Expand Up @@ -224,6 +224,47 @@ export default util.createRule<Options, MessageIds>({
);
}

function isImportDeclaration(
definition:
| TSESTree.ImportDeclaration
| TSESTree.TSImportEqualsDeclaration,
): definition is TSESTree.ImportDeclaration {
return definition.type === AST_NODE_TYPES.ImportDeclaration;
}

function isExternalModuleDeclarationWithName(
scope: TSESLint.Scope.Scope,
name: string,
): boolean {
return (
scope.type === ScopeType.tsModule &&
scope.block.type === AST_NODE_TYPES.TSModuleDeclaration &&
scope.block.id.type === AST_NODE_TYPES.Literal &&
scope.block.id.value === name
);
}

function isExternalDeclarationMerging(
scope: TSESLint.Scope.Scope,
variable: TSESLint.Scope.Variable,
shadowed: TSESLint.Scope.Variable,
): boolean {
const [firstDefinition] = shadowed.defs;
const [secondDefinition] = variable.defs;

return (
isTypeImport(firstDefinition) &&
isImportDeclaration(firstDefinition.parent) &&
isExternalModuleDeclarationWithName(
scope,
firstDefinition.parent.source.value,
) &&
secondDefinition.node.type === AST_NODE_TYPES.TSInterfaceDeclaration &&
secondDefinition.node.parent?.type ===
AST_NODE_TYPES.ExportNamedDeclaration
);
}

/**
* Check if variable name is allowed.
* @param variable The variable to check.
Expand Down Expand Up @@ -403,6 +444,10 @@ export default util.createRule<Options, MessageIds>({
continue;
}

if (isExternalDeclarationMerging(scope, variable, shadowed)) {
continue;
}

const isESLintGlobal = 'writeable' in shadowed;
if (
(shadowed.identifiers.length > 0 ||
Expand Down
87 changes: 87 additions & 0 deletions packages/eslint-plugin/tests/rules/no-shadow.test.ts
Expand Up @@ -51,6 +51,15 @@ class Foo {
}
interface Foo {
prop2: string;
}
`,
`
import type { Foo } from 'bar';
declare module 'bar' {
export interface Foo {
x: string;
}
}
`,
// type value shadowing
Expand Down Expand Up @@ -1442,5 +1451,83 @@ function doThing(foo: number, bar: number) {}
},
],
},
{
code: `
interface Foo {}
declare module 'bar' {
export interface Foo {
x: string;
}
}
`,
errors: [
{
messageId: 'noShadow',
data: { name: 'Foo' },
type: AST_NODE_TYPES.Identifier,
line: 5,
column: 20,
},
],
},
{
code: `
import type { Foo } from 'bar';
declare module 'baz' {
export interface Foo {
x: string;
}
}
`,
errors: [
{
messageId: 'noShadow',
data: { name: 'Foo' },
type: AST_NODE_TYPES.Identifier,
line: 5,
column: 20,
},
],
},
{
code: `
import type { Foo } from 'bar';
declare module 'bar' {
export type Foo = string;
}
`,
errors: [
{
messageId: 'noShadow',
data: { name: 'Foo' },
type: AST_NODE_TYPES.Identifier,
line: 5,
column: 15,
},
],
},
{
code: `
import type { Foo } from 'bar';
declare module 'bar' {
interface Foo {
x: string;
}
}
`,
errors: [
{
messageId: 'noShadow',
data: { name: 'Foo' },
type: AST_NODE_TYPES.Identifier,
line: 5,
column: 13,
},
],
},
],
});

0 comments on commit a93cebf

Please sign in to comment.