Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

no-duplicate-imports: Allow duplicate imports from separate ambient module declarations #3398

Merged
merged 2 commits into from Oct 26, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions src/rules/noDuplicateImportsRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { findImports, ImportKind } from "tsutils";
import { isLiteralExpression, isModuleBlock, isModuleDeclaration } from "tsutils";
import * as ts from "typescript";
import * as Lint from "../index";

Expand Down Expand Up @@ -44,13 +44,38 @@ export class Rule extends Lint.Rules.AbstractRule {
}
}

function walk(ctx: Lint.WalkContext<void>) {
const seen = new Set<string>();
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>): void {
walkWorker(ctx, ctx.sourceFile.statements, new Set());
}

function walkWorker(ctx: Lint.WalkContext<void>, statements: ReadonlyArray<ts.Statement>, seen: Set<string>): void {
for (const statement of statements) {
const im = tryGetImportSpecifier(statement);
if (im !== undefined && isLiteralExpression(im)) {
const { text } = im;
if (seen.has(text)) {
ctx.addFailureAtNode(im.parent!, Rule.FAILURE_STRING(text));
}
seen.add(text);
}

if (isModuleDeclaration(statement) && statement.body !== undefined && isModuleBlock(statement.body)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could add an additional check here if statement.name.kind === ts.SyntaxKind.StringLiteral, because namespaces cannot contain import statements.
and then you probably don't need the isModuleBlock check.

// 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.statements, ts.isExternalModule(ctx.sourceFile) ? seen : new Set());
}
}
}

function tryGetImportSpecifier(statement: ts.Statement): ts.Expression | undefined {
switch (statement.kind) {
case ts.SyntaxKind.ImportDeclaration:
return (statement as ts.ImportDeclaration).moduleSpecifier;
case ts.SyntaxKind.ImportEqualsDeclaration:
const ref = (statement as ts.ImportEqualsDeclaration).moduleReference;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't check ImportEqualsDeclaration before your change. That's not a problem, but should be mentioned in the changelog.

Copy link
Author

@ghost ghost Oct 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was right before because you might want named imports from a module in addition to importing it as a function. Removed this part.

return ref.kind === ts.SyntaxKind.ExternalModuleReference ? ref.expression : undefined;
default:
return undefined;
}
}
8 changes: 8 additions & 0 deletions test/rules/no-duplicate-imports/test2.d.ts.lint
Original file line number Diff line number Diff line change
@@ -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';
}
11 changes: 11 additions & 0 deletions test/rules/no-duplicate-imports/test3.d.ts.lint
Original file line number Diff line number Diff line change
@@ -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.]
}