Skip to content

Commit 2e25654

Browse files
author
Andy Hanson
committed
When every import is unused, error on the entire import declaration
1 parent 1e06637 commit 2e25654

16 files changed

+275
-35
lines changed

‎src/compiler/checker.ts

+32-5
Original file line numberDiff line numberDiff line change
@@ -21682,18 +21682,45 @@ namespace ts {
2168221682

2168321683
function checkUnusedModuleMembers(node: ModuleDeclaration | SourceFile): void {
2168421684
if (compilerOptions.noUnusedLocals && !(node.flags & NodeFlags.Ambient)) {
21685+
const unusedImports: ImportedDeclaration[] = [];
2168521686
node.locals.forEach(local => {
21686-
if (!local.isReferenced && !local.exportSymbol) {
21687-
for (const declaration of local.declarations) {
21688-
if (!isAmbientModule(declaration)) {
21689-
errorUnusedLocal(declaration, symbolName(local));
21690-
}
21687+
if (local.isReferenced || local.exportSymbol) return;
21688+
for (const declaration of local.declarations) {
21689+
if (isAmbientModule(declaration)) continue;
21690+
if (isImportedDeclaration(declaration)) {
21691+
unusedImports.push(declaration);
21692+
}
21693+
else {
21694+
errorUnusedLocal(declaration, symbolName(local));
2169121695
}
2169221696
}
2169321697
});
21698+
21699+
while (unusedImports.length) {
21700+
const decl = unusedImports.pop()!;
21701+
const importClause = decl.kind === SyntaxKind.ImportClause ? decl : decl.kind === SyntaxKind.NamespaceImport ? decl.parent : decl.parent.parent;
21702+
if (!forEachImportedDeclaration(importClause, d => d !== decl && !contains(unusedImports, d))) {
21703+
forEachImportedDeclaration(importClause, d => unorderedRemoveItem(unusedImports, d));
21704+
error(importClause.parent, Diagnostics.No_import_of_0_is_used, showModuleSpecifier(importClause.parent));
21705+
}
21706+
else {
21707+
errorUnusedLocal(decl, idText(decl.name));
21708+
}
21709+
}
2169421710
}
2169521711
}
2169621712

21713+
type ImportedDeclaration = ImportClause | ImportSpecifier | NamespaceImport;
21714+
function isImportedDeclaration(node: Node): node is ImportedDeclaration {
21715+
return node.kind === SyntaxKind.ImportClause || node.kind === SyntaxKind.ImportSpecifier || node.kind === SyntaxKind.NamespaceImport;
21716+
}
21717+
21718+
function forEachImportedDeclaration<T>(importClause: ImportClause, cb: (im: ImportedDeclaration) => T | undefined): T | undefined {
21719+
const { name: defaultName, namedBindings } = importClause;
21720+
return (defaultName && cb(importClause)) ||
21721+
namedBindings && (namedBindings.kind === SyntaxKind.NamespaceImport ? cb(namedBindings) : forEach(namedBindings.elements, cb));
21722+
}
21723+
2169721724
function checkBlock(node: Block) {
2169821725
// Grammar checking for SyntaxKind.Block
2169921726
if (node.kind === SyntaxKind.Block) {

‎src/compiler/diagnosticMessages.json

+4
Original file line numberDiff line numberDiff line change
@@ -3480,6 +3480,10 @@
34803480
"category": "Message",
34813481
"code": 6191
34823482
},
3483+
"No import of '{0}' is used.": {
3484+
"category": "Error",
3485+
"code": 6192
3486+
},
34833487
"Variable '{0}' implicitly has an '{1}' type.": {
34843488
"category": "Error",
34853489
"code": 7005

‎src/compiler/utilities.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,7 @@ namespace ts {
11991199
&& (<PropertyAccessExpression | ElementAccessExpression>node).expression.kind === SyntaxKind.ThisKeyword;
12001200
}
12011201

1202-
export function getEntityNameFromTypeNode(node: TypeNode): EntityNameOrEntityNameExpression {
1202+
export function getEntityNameFromTypeNode(node: TypeNode): EntityNameOrEntityNameExpression {
12031203
switch (node.kind) {
12041204
case SyntaxKind.TypeReference:
12051205
return (<TypeReferenceNode>node).typeName;
@@ -3723,6 +3723,10 @@ namespace ts {
37233723
export function isUMDExportSymbol(symbol: Symbol) {
37243724
return symbol && symbol.declarations && symbol.declarations[0] && isNamespaceExportDeclaration(symbol.declarations[0]);
37253725
}
3726+
3727+
export function showModuleSpecifier({ moduleSpecifier }: ImportDeclaration): string {
3728+
return isStringLiteral(moduleSpecifier) ? moduleSpecifier.text : getTextOfNode(moduleSpecifier);
3729+
}
37263730
}
37273731

37283732
namespace ts {

‎src/services/codefixes/fixUnusedIdentifier.ts

+22-3
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,21 @@ namespace ts.codefix {
55
const errorCodes = [
66
Diagnostics._0_is_declared_but_its_value_is_never_read.code,
77
Diagnostics.Property_0_is_declared_but_its_value_is_never_read.code,
8+
Diagnostics.No_import_of_0_is_used.code,
89
];
910
registerCodeFix({
1011
errorCodes,
1112
getCodeActions(context) {
12-
const { sourceFile } = context;
13+
const { errorCode, sourceFile } = context;
1314
const token = getToken(sourceFile, context.span.start);
15+
16+
if (errorCode === Diagnostics.No_import_of_0_is_used.code) {
17+
const decl = getImportDeclarationAtErrorLocation(token);
18+
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Remove_declaration_for_Colon_0), [showModuleSpecifier(decl)]);
19+
const changes = textChanges.ChangeTracker.with(context, t => t.deleteNode(sourceFile, decl));
20+
return [{ description, changes, fixId: fixIdDelete }];
21+
}
22+
1423
const result: CodeFixAction[] = [];
1524

1625
const deletion = textChanges.ChangeTracker.with(context, t => tryDeleteDeclaration(t, sourceFile, token));
@@ -19,7 +28,7 @@ namespace ts.codefix {
1928
result.push({ description, changes: deletion, fixId: fixIdDelete });
2029
}
2130

22-
const prefix = textChanges.ChangeTracker.with(context, t => tryPrefixDeclaration(t, context.errorCode, sourceFile, token));
31+
const prefix = textChanges.ChangeTracker.with(context, t => tryPrefixDeclaration(t, errorCode, sourceFile, token));
2332
if (prefix.length) {
2433
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Prefix_0_with_an_underscore), [token.getText()]);
2534
result.push({ description, changes: prefix, fixId: fixIdPrefix });
@@ -38,14 +47,24 @@ namespace ts.codefix {
3847
}
3948
break;
4049
case fixIdDelete:
41-
tryDeleteDeclaration(changes, sourceFile, token);
50+
if (diag.code === Diagnostics.No_import_of_0_is_used.code) {
51+
changes.deleteNode(sourceFile, getImportDeclarationAtErrorLocation(token));
52+
}
53+
else {
54+
tryDeleteDeclaration(changes, sourceFile, token);
55+
}
4256
break;
4357
default:
4458
Debug.fail(JSON.stringify(context.fixId));
4559
}
4660
}),
4761
});
4862

63+
function getImportDeclarationAtErrorLocation(token: Node): ImportDeclaration {
64+
Debug.assert(token.kind === SyntaxKind.ImportKeyword);
65+
return cast(token.parent, isImportDeclaration);
66+
}
67+
4968
function getToken(sourceFile: SourceFile, pos: number): Node {
5069
const token = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
5170
// this handles var ["computed"] = 12;

‎tests/baselines/reference/unusedImports1.errors.txt

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
tests/cases/compiler/file2.ts(1,9): error TS6133: 'Calculator' is declared but its value is never read.
1+
tests/cases/compiler/file2.ts(1,1): error TS6192: No import of './file1' is used.
22

33

44
==== tests/cases/compiler/file1.ts (0 errors) ====
@@ -8,5 +8,5 @@ tests/cases/compiler/file2.ts(1,9): error TS6133: 'Calculator' is declared but i
88

99
==== tests/cases/compiler/file2.ts (1 errors) ====
1010
import {Calculator} from "./file1"
11-
~~~~~~~~~~
12-
!!! error TS6133: 'Calculator' is declared but its value is never read.
11+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12+
!!! error TS6192: No import of './file1' is used.

‎tests/baselines/reference/unusedImports12.errors.txt

+10-13
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,19 @@
1-
tests/cases/compiler/a.ts(1,10): error TS6133: 'Member' is declared but its value is never read.
2-
tests/cases/compiler/a.ts(2,8): error TS6133: 'd' is declared but its value is never read.
3-
tests/cases/compiler/a.ts(2,23): error TS6133: 'M' is declared but its value is never read.
4-
tests/cases/compiler/a.ts(3,13): error TS6133: 'ns' is declared but its value is never read.
1+
tests/cases/compiler/a.ts(1,1): error TS6192: No import of './b' is used.
2+
tests/cases/compiler/a.ts(2,1): error TS6192: No import of './b' is used.
3+
tests/cases/compiler/a.ts(3,1): error TS6192: No import of './b' is used.
54
tests/cases/compiler/a.ts(4,8): error TS6133: 'r' is declared but its value is never read.
65

76

8-
==== tests/cases/compiler/a.ts (5 errors) ====
7+
==== tests/cases/compiler/a.ts (4 errors) ====
98
import { Member } from './b';
10-
~~~~~~
11-
!!! error TS6133: 'Member' is declared but its value is never read.
9+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
10+
!!! error TS6192: No import of './b' is used.
1211
import d, { Member as M } from './b';
13-
~
14-
!!! error TS6133: 'd' is declared but its value is never read.
15-
~
16-
!!! error TS6133: 'M' is declared but its value is never read.
12+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
13+
!!! error TS6192: No import of './b' is used.
1714
import * as ns from './b';
18-
~~
19-
!!! error TS6133: 'ns' is declared but its value is never read.
15+
~~~~~~~~~~~~~~~~~~~~~~~~~~
16+
!!! error TS6192: No import of './b' is used.
2017
import r = require("./b");
2118
~
2219
!!! error TS6133: 'r' is declared but its value is never read.

‎tests/baselines/reference/unusedImports2.errors.txt

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
tests/cases/compiler/file2.ts(2,9): error TS6133: 'test' is declared but its value is never read.
1+
tests/cases/compiler/file2.ts(2,1): error TS6192: No import of './file1' is used.
22

33

44
==== tests/cases/compiler/file1.ts (0 errors) ====
@@ -13,8 +13,8 @@ tests/cases/compiler/file2.ts(2,9): error TS6133: 'test' is declared but its val
1313
==== tests/cases/compiler/file2.ts (1 errors) ====
1414
import {Calculator} from "./file1"
1515
import {test} from "./file1"
16-
~~~~
17-
!!! error TS6133: 'test' is declared but its value is never read.
16+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17+
!!! error TS6192: No import of './file1' is used.
1818

1919
var x = new Calculator();
2020
x.handleChar();

‎tests/baselines/reference/unusedImports6.errors.txt

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
tests/cases/compiler/file2.ts(1,8): error TS6133: 'd' is declared but its value is never read.
1+
tests/cases/compiler/file2.ts(1,1): error TS6192: No import of './file1' is used.
22

33

44
==== tests/cases/compiler/file1.ts (0 errors) ====
@@ -16,8 +16,8 @@ tests/cases/compiler/file2.ts(1,8): error TS6133: 'd' is declared but its value
1616

1717
==== tests/cases/compiler/file2.ts (1 errors) ====
1818
import d from "./file1"
19-
~
20-
!!! error TS6133: 'd' is declared but its value is never read.
19+
~~~~~~~~~~~~~~~~~~~~~~~
20+
!!! error TS6192: No import of './file1' is used.
2121

2222

2323

‎tests/baselines/reference/unusedImports7.errors.txt

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
tests/cases/compiler/file2.ts(1,13): error TS6133: 'n' is declared but its value is never read.
1+
tests/cases/compiler/file2.ts(1,1): error TS6192: No import of './file1' is used.
22

33

44
==== tests/cases/compiler/file1.ts (0 errors) ====
@@ -16,7 +16,7 @@ tests/cases/compiler/file2.ts(1,13): error TS6133: 'n' is declared but its value
1616

1717
==== tests/cases/compiler/file2.ts (1 errors) ====
1818
import * as n from "./file1"
19-
~
20-
!!! error TS6133: 'n' is declared but its value is never read.
19+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
20+
!!! error TS6192: No import of './file1' is used.
2121

2222

‎tests/baselines/reference/unusedImports_entireImportDeclaration.errors.txt

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/b.ts(1,1): error TS6192: No import of './a' is used.
2+
/b.ts(2,1): error TS6192: No import of './a' is used.
3+
/b.ts(4,19): error TS6133: 'a2' is declared but its value is never read.
4+
/b.ts(4,28): error TS6133: 'b2' is declared but its value is never read.
5+
/b.ts(6,17): error TS6133: 'ns2' is declared but its value is never read.
6+
/b.ts(8,8): error TS6133: 'd5' is declared but its value is never read.
7+
8+
9+
==== /a.ts (0 errors) ====
10+
export const a = 0;
11+
export const b = 0;
12+
export default 0;
13+
14+
==== /b.ts (6 errors) ====
15+
import d1, { a as a1, b as b1 } from "./a";
16+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17+
!!! error TS6192: No import of './a' is used.
18+
import d2, * as ns from "./a";
19+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
20+
!!! error TS6192: No import of './a' is used.
21+
22+
import d3, { a as a2, b as b2 } from "./a";
23+
~~
24+
!!! error TS6133: 'a2' is declared but its value is never read.
25+
~~
26+
!!! error TS6133: 'b2' is declared but its value is never read.
27+
d3;
28+
import d4, * as ns2 from "./a";
29+
~~~
30+
!!! error TS6133: 'ns2' is declared but its value is never read.
31+
d4;
32+
import d5, * as ns3 from "./a";
33+
~~
34+
!!! error TS6133: 'd5' is declared but its value is never read.
35+
ns3;
36+

‎tests/baselines/reference/unusedImports_entireImportDeclaration.js

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
//// [tests/cases/compiler/unusedImports_entireImportDeclaration.ts] ////
2+
3+
//// [a.ts]
4+
export const a = 0;
5+
export const b = 0;
6+
export default 0;
7+
8+
//// [b.ts]
9+
import d1, { a as a1, b as b1 } from "./a";
10+
import d2, * as ns from "./a";
11+
12+
import d3, { a as a2, b as b2 } from "./a";
13+
d3;
14+
import d4, * as ns2 from "./a";
15+
d4;
16+
import d5, * as ns3 from "./a";
17+
ns3;
18+
19+
20+
//// [a.js]
21+
"use strict";
22+
exports.__esModule = true;
23+
exports.a = 0;
24+
exports.b = 0;
25+
exports["default"] = 0;
26+
//// [b.js]
27+
"use strict";
28+
exports.__esModule = true;
29+
var a_1 = require("./a");
30+
a_1["default"];
31+
var a_2 = require("./a");
32+
a_2["default"];
33+
var ns3 = require("./a");
34+
ns3;

‎tests/baselines/reference/unusedImports_entireImportDeclaration.symbols

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
=== /a.ts ===
2+
export const a = 0;
3+
>a : Symbol(a, Decl(a.ts, 0, 12))
4+
5+
export const b = 0;
6+
>b : Symbol(b, Decl(a.ts, 1, 12))
7+
8+
export default 0;
9+
10+
=== /b.ts ===
11+
import d1, { a as a1, b as b1 } from "./a";
12+
>d1 : Symbol(d1, Decl(b.ts, 0, 6))
13+
>a : Symbol(a1, Decl(b.ts, 0, 12))
14+
>a1 : Symbol(a1, Decl(b.ts, 0, 12))
15+
>b : Symbol(b1, Decl(b.ts, 0, 21))
16+
>b1 : Symbol(b1, Decl(b.ts, 0, 21))
17+
18+
import d2, * as ns from "./a";
19+
>d2 : Symbol(d2, Decl(b.ts, 1, 6))
20+
>ns : Symbol(ns, Decl(b.ts, 1, 10))
21+
22+
import d3, { a as a2, b as b2 } from "./a";
23+
>d3 : Symbol(d3, Decl(b.ts, 3, 6))
24+
>a : Symbol(a2, Decl(b.ts, 3, 12))
25+
>a2 : Symbol(a2, Decl(b.ts, 3, 12))
26+
>b : Symbol(b2, Decl(b.ts, 3, 21))
27+
>b2 : Symbol(b2, Decl(b.ts, 3, 21))
28+
29+
d3;
30+
>d3 : Symbol(d3, Decl(b.ts, 3, 6))
31+
32+
import d4, * as ns2 from "./a";
33+
>d4 : Symbol(d4, Decl(b.ts, 5, 6))
34+
>ns2 : Symbol(ns2, Decl(b.ts, 5, 10))
35+
36+
d4;
37+
>d4 : Symbol(d4, Decl(b.ts, 5, 6))
38+
39+
import d5, * as ns3 from "./a";
40+
>d5 : Symbol(d5, Decl(b.ts, 7, 6))
41+
>ns3 : Symbol(ns3, Decl(b.ts, 7, 10))
42+
43+
ns3;
44+
>ns3 : Symbol(ns3, Decl(b.ts, 7, 10))
45+

0 commit comments

Comments
 (0)