From 017612268826b6134baf1ace19c495d83b80a9bf Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 26 Oct 2017 13:10:29 -0700 Subject: [PATCH] no-duplicate-imports: Allow duplicate imports from separate ambient module declarations (#3398) --- src/rules/noDuplicateImportsRule.ts | 26 ++++++++++++++----- .../no-duplicate-imports/test2.d.ts.lint | 8 ++++++ .../no-duplicate-imports/test3.d.ts.lint | 11 ++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 test/rules/no-duplicate-imports/test2.d.ts.lint create mode 100644 test/rules/no-duplicate-imports/test3.d.ts.lint diff --git a/src/rules/noDuplicateImportsRule.ts b/src/rules/noDuplicateImportsRule.ts index 25a0e3fba17..3677e4da8ef 100644 --- a/src/rules/noDuplicateImportsRule.ts +++ b/src/rules/noDuplicateImportsRule.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { findImports, ImportKind } from "tsutils"; +import { isImportDeclaration, isLiteralExpression, isModuleDeclaration } from "tsutils"; import * as ts from "typescript"; import * as Lint from "../index"; @@ -44,13 +44,25 @@ export class Rule extends Lint.Rules.AbstractRule { } } -function walk(ctx: Lint.WalkContext) { - const seen = new Set(); - for (const {text, parent} of findImports(ctx.sourceFile, ImportKind.ImportDeclaration)) { - if (seen.has(text)) { - ctx.addFailureAtNode(parent!, Rule.FAILURE_STRING(text)); - } else { +function walk(ctx: Lint.WalkContext): void { + walkWorker(ctx, ctx.sourceFile.statements, new Set()); +} + +function walkWorker(ctx: Lint.WalkContext, statements: ReadonlyArray, seen: Set): void { + for (const statement of statements) { + if (isImportDeclaration(statement) && isLiteralExpression(statement.moduleSpecifier)) { + const { text } = statement.moduleSpecifier; + if (seen.has(text)) { + ctx.addFailureAtNode(statement, Rule.FAILURE_STRING(text)); + } seen.add(text); } + + if (isModuleDeclaration(statement) && statement.body !== undefined && statement.name.kind === ts.SyntaxKind.StringLiteral) { + // If this is a module augmentation, re-use `seen` since those imports could be moved outside. + // If this is an ambient module, create a fresh `seen` + // because they should have separate imports to avoid becoming augmentations. + walkWorker(ctx, (statement.body as ts.ModuleBlock).statements, ts.isExternalModule(ctx.sourceFile) ? seen : new Set()); + } } } diff --git a/test/rules/no-duplicate-imports/test2.d.ts.lint b/test/rules/no-duplicate-imports/test2.d.ts.lint new file mode 100644 index 00000000000..00ce946e7d1 --- /dev/null +++ b/test/rules/no-duplicate-imports/test2.d.ts.lint @@ -0,0 +1,8 @@ +declare module 'a' { + import foo from 'foo'; +} + +declare module 'b' { + // No error -- this is a separate ambient module declaration. + import foo from 'foo'; +} diff --git a/test/rules/no-duplicate-imports/test3.d.ts.lint b/test/rules/no-duplicate-imports/test3.d.ts.lint new file mode 100644 index 00000000000..bd64667a22f --- /dev/null +++ b/test/rules/no-duplicate-imports/test3.d.ts.lint @@ -0,0 +1,11 @@ +export {}; + +declare module 'a' { + import foo from 'foo'; +} + +declare module 'b' { + // Error because these imports could be combined in an outer scope. + import foo from 'foo'; + ~~~~~~~~~~~~~~~~~~~~~~ [Multiple imports from 'foo' can be combined into one.] +}