Skip to content

When every import is unused, error on the entire import declaration #22386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
4 commits merged into from
Mar 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
48 changes: 44 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21682,18 +21682,58 @@ namespace ts {

function checkUnusedModuleMembers(node: ModuleDeclaration | SourceFile): void {
if (compilerOptions.noUnusedLocals && !(node.flags & NodeFlags.Ambient)) {
// Ideally we could use the ImportClause directly as a key, but must wait until we have full ES6 maps. So must store key along with value.
const unusedImports = createMap<[ImportClause, ImportedDeclaration[]]>();
node.locals.forEach(local => {
if (!local.isReferenced && !local.exportSymbol) {
for (const declaration of local.declarations) {
if (!isAmbientModule(declaration)) {
errorUnusedLocal(declaration, symbolName(local));
if (local.isReferenced || local.exportSymbol) return;
for (const declaration of local.declarations) {
if (isAmbientModule(declaration)) continue;
if (isImportedDeclaration(declaration)) {
const importClause = importClauseFromImported(declaration);
const key = String(getNodeId(importClause));
const group = unusedImports.get(key);
if (group) {
group[1].push(declaration);
}
else {
unusedImports.set(key, [importClause, [declaration]]);
}
}
else {
errorUnusedLocal(declaration, symbolName(local));
}
}
});

unusedImports.forEach(([importClause, unuseds]) => {
const importDecl = importClause.parent;
if (forEachImportedDeclaration(importClause, d => !contains(unuseds, d))) {
for (const unused of unuseds) errorUnusedLocal(unused, idText(unused.name));
}
else if (unuseds.length === 1) {
error(importDecl, Diagnostics._0_is_declared_but_its_value_is_never_read, idText(first(unuseds).name));
}
else {
error(importDecl, Diagnostics.All_imports_in_import_declaration_are_unused, showModuleSpecifier(importDecl));
}
});
}
}

type ImportedDeclaration = ImportClause | ImportSpecifier | NamespaceImport;
function isImportedDeclaration(node: Node): node is ImportedDeclaration {
return node.kind === SyntaxKind.ImportClause || node.kind === SyntaxKind.ImportSpecifier || node.kind === SyntaxKind.NamespaceImport;
}
function importClauseFromImported(decl: ImportedDeclaration): ImportClause {
return decl.kind === SyntaxKind.ImportClause ? decl : decl.kind === SyntaxKind.NamespaceImport ? decl.parent : decl.parent.parent;
}

function forEachImportedDeclaration<T>(importClause: ImportClause, cb: (im: ImportedDeclaration) => T | undefined): T | undefined {
const { name: defaultName, namedBindings } = importClause;
return (defaultName && cb(importClause)) ||
namedBindings && (namedBindings.kind === SyntaxKind.NamespaceImport ? cb(namedBindings) : forEach(namedBindings.elements, cb));
}

function checkBlock(node: Block) {
// Grammar checking for SyntaxKind.Block
if (node.kind === SyntaxKind.Block) {
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3480,6 +3480,10 @@
"category": "Message",
"code": 6191
},
"All imports in import declaration are unused.": {
"category": "Error",
"code": 6192
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down Expand Up @@ -3851,6 +3855,10 @@
"category": "Message",
"code": 90004
},
"Remove import from '{0}'": {
"category": "Message",
"code": 90005
},
"Implement interface '{0}'": {
"category": "Message",
"code": 90006
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1204,7 +1204,7 @@ namespace ts {
&& (<PropertyAccessExpression | ElementAccessExpression>node).expression.kind === SyntaxKind.ThisKeyword;
}

export function getEntityNameFromTypeNode(node: TypeNode): EntityNameOrEntityNameExpression {
export function getEntityNameFromTypeNode(node: TypeNode): EntityNameOrEntityNameExpression {
switch (node.kind) {
case SyntaxKind.TypeReference:
return (<TypeReferenceNode>node).typeName;
Expand Down Expand Up @@ -3728,6 +3728,10 @@ namespace ts {
export function isUMDExportSymbol(symbol: Symbol) {
return symbol && symbol.declarations && symbol.declarations[0] && isNamespaceExportDeclaration(symbol.declarations[0]);
}

export function showModuleSpecifier({ moduleSpecifier }: ImportDeclaration): string {
return isStringLiteral(moduleSpecifier) ? moduleSpecifier.text : getTextOfNode(moduleSpecifier);
}
}

namespace ts {
Expand Down
26 changes: 23 additions & 3 deletions src/services/codefixes/fixUnusedIdentifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,19 @@ namespace ts.codefix {
const errorCodes = [
Diagnostics._0_is_declared_but_its_value_is_never_read.code,
Diagnostics.Property_0_is_declared_but_its_value_is_never_read.code,
Diagnostics.All_imports_in_import_declaration_are_unused.code,
];
registerCodeFix({
errorCodes,
getCodeActions(context) {
const { sourceFile } = context;
const { errorCode, sourceFile } = context;
const importDecl = tryGetFullImport(sourceFile, context.span.start);
if (importDecl) {
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Remove_import_from_0), [showModuleSpecifier(importDecl)]);
const changes = textChanges.ChangeTracker.with(context, t => t.deleteNode(sourceFile, importDecl));
return [{ description, changes, fixId: fixIdDelete }];
}

const token = getToken(sourceFile, textSpanEnd(context.span));
const result: CodeFixAction[] = [];

Expand All @@ -19,7 +27,7 @@ namespace ts.codefix {
result.push({ description, changes: deletion, fixId: fixIdDelete });
}

const prefix = textChanges.ChangeTracker.with(context, t => tryPrefixDeclaration(t, context.errorCode, sourceFile, token));
const prefix = textChanges.ChangeTracker.with(context, t => tryPrefixDeclaration(t, errorCode, sourceFile, token));
if (prefix.length) {
const description = formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Prefix_0_with_an_underscore), [token.getText()]);
result.push({ description, changes: prefix, fixId: fixIdPrefix });
Expand All @@ -38,14 +46,26 @@ namespace ts.codefix {
}
break;
case fixIdDelete:
tryDeleteDeclaration(changes, sourceFile, token);
const importDecl = tryGetFullImport(diag.file!, diag.start!);
if (importDecl) {
changes.deleteNode(sourceFile, importDecl);
}
else {
tryDeleteDeclaration(changes, sourceFile, token);
}
break;
default:
Debug.fail(JSON.stringify(context.fixId));
}
}),
});

// Sometimes the diagnostic span is an entire ImportDeclaration, so we should remove the whole thing.
function tryGetFullImport(sourceFile: SourceFile, pos: number): ImportDeclaration | undefined {
const startToken = getTokenAtPosition(sourceFile, pos, /*includeJsDocComment*/ false);
return startToken.kind === SyntaxKind.ImportKeyword ? tryCast(startToken.parent, isImportDeclaration) : undefined;
}

function getToken(sourceFile: SourceFile, pos: number): Node {
const token = findPrecedingToken(pos, sourceFile);
// this handles var ["computed"] = 12;
Expand Down
2 changes: 1 addition & 1 deletion src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ namespace ts.textChanges {

export function getAdjustedStartPosition(sourceFile: SourceFile, node: Node, options: ConfigurableStart, position: Position) {
if (options.useNonAdjustedStartPosition) {
return node.getStart();
return node.getStart(sourceFile);
}
const fullStart = node.getFullStart();
const start = node.getStart(sourceFile);
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/extendsUntypedModule.errors.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/a.ts(2,8): error TS6133: 'Bar' is declared but its value is never read.
/a.ts(2,1): error TS6133: 'Bar' is declared but its value is never read.


==== /a.ts (1 errors) ====
import Foo from "foo";
import Bar from "bar"; // error: unused
~~~
~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'Bar' is declared but its value is never read.
export class A extends Foo { }

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/unusedImports1.errors.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tests/cases/compiler/file2.ts(1,9): error TS6133: 'Calculator' is declared but its value is never read.
tests/cases/compiler/file2.ts(1,1): error TS6133: 'Calculator' is declared but its value is never read.


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

==== tests/cases/compiler/file2.ts (1 errors) ====
import {Calculator} from "./file1"
~~~~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'Calculator' is declared but its value is never read.
19 changes: 8 additions & 11 deletions tests/baselines/reference/unusedImports12.errors.txt
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
tests/cases/compiler/a.ts(1,10): error TS6133: 'Member' is declared but its value is never read.
tests/cases/compiler/a.ts(2,8): error TS6133: 'd' is declared but its value is never read.
tests/cases/compiler/a.ts(2,13): error TS6133: 'M' is declared but its value is never read.
tests/cases/compiler/a.ts(3,8): error TS6133: 'ns' is declared but its value is never read.
tests/cases/compiler/a.ts(1,1): error TS6133: 'Member' is declared but its value is never read.
tests/cases/compiler/a.ts(2,1): error TS6192: All imports in import declaration are unused.
tests/cases/compiler/a.ts(3,1): error TS6133: 'ns' is declared but its value is never read.
tests/cases/compiler/a.ts(4,1): error TS6133: 'r' is declared but its value is never read.


==== tests/cases/compiler/a.ts (5 errors) ====
==== tests/cases/compiler/a.ts (4 errors) ====
import { Member } from './b';
~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'Member' is declared but its value is never read.
import d, { Member as M } from './b';
~
!!! error TS6133: 'd' is declared but its value is never read.
~~~~~~~~~~~
!!! error TS6133: 'M' is declared but its value is never read.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6192: All imports in import declaration are unused.
import * as ns from './b';
~~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'ns' is declared but its value is never read.
import r = require("./b");
~~~~~~~~
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/unusedImports2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tests/cases/compiler/file2.ts(2,9): error TS6133: 'test' is declared but its value is never read.
tests/cases/compiler/file2.ts(2,1): error TS6133: 'test' is declared but its value is never read.


==== tests/cases/compiler/file1.ts (0 errors) ====
Expand All @@ -13,7 +13,7 @@ tests/cases/compiler/file2.ts(2,9): error TS6133: 'test' is declared but its val
==== tests/cases/compiler/file2.ts (1 errors) ====
import {Calculator} from "./file1"
import {test} from "./file1"
~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'test' is declared but its value is never read.

var x = new Calculator();
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/unusedImports6.errors.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tests/cases/compiler/file2.ts(1,8): error TS6133: 'd' is declared but its value is never read.
tests/cases/compiler/file2.ts(1,1): error TS6133: 'd' is declared but its value is never read.


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

==== tests/cases/compiler/file2.ts (1 errors) ====
import d from "./file1"
~
~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'd' is declared but its value is never read.


Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/unusedImports7.errors.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tests/cases/compiler/file2.ts(1,8): error TS6133: 'n' is declared but its value is never read.
tests/cases/compiler/file2.ts(1,1): error TS6133: 'n' is declared but its value is never read.


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

==== tests/cases/compiler/file2.ts (1 errors) ====
import * as n from "./file1"
~~~~~~
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6133: 'n' is declared but its value is never read.


Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/b.ts(1,1): error TS6192: All imports in import declaration are unused.
/b.ts(2,1): error TS6192: All imports in import declaration are unused.
/b.ts(4,14): error TS6133: 'a2' is declared but its value is never read.
/b.ts(4,23): error TS6133: 'b2' is declared but its value is never read.
/b.ts(6,12): error TS6133: 'ns2' is declared but its value is never read.
/b.ts(8,8): error TS6133: 'd5' is declared but its value is never read.


==== /a.ts (0 errors) ====
export const a = 0;
export const b = 0;
export default 0;

==== /b.ts (6 errors) ====
import d1, { a as a1, b as b1 } from "./a";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6192: All imports in import declaration are unused.
import d2, * as ns from "./a";
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS6192: All imports in import declaration are unused.

import d3, { a as a2, b as b2 } from "./a";
~~~~~~~
!!! error TS6133: 'a2' is declared but its value is never read.
~~~~~~~
!!! error TS6133: 'b2' is declared but its value is never read.
d3;
import d4, * as ns2 from "./a";
~~~~~~~~
!!! error TS6133: 'ns2' is declared but its value is never read.
d4;
import d5, * as ns3 from "./a";
~~
!!! error TS6133: 'd5' is declared but its value is never read.
ns3;

34 changes: 34 additions & 0 deletions tests/baselines/reference/unusedImports_entireImportDeclaration.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//// [tests/cases/compiler/unusedImports_entireImportDeclaration.ts] ////

//// [a.ts]
export const a = 0;
export const b = 0;
export default 0;

//// [b.ts]
import d1, { a as a1, b as b1 } from "./a";
import d2, * as ns from "./a";

import d3, { a as a2, b as b2 } from "./a";
d3;
import d4, * as ns2 from "./a";
d4;
import d5, * as ns3 from "./a";
ns3;


//// [a.js]
"use strict";
exports.__esModule = true;
exports.a = 0;
exports.b = 0;
exports["default"] = 0;
//// [b.js]
"use strict";
exports.__esModule = true;
var a_1 = require("./a");
a_1["default"];
var a_2 = require("./a");
a_2["default"];
var ns3 = require("./a");
ns3;
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
=== /a.ts ===
export const a = 0;
>a : Symbol(a, Decl(a.ts, 0, 12))

export const b = 0;
>b : Symbol(b, Decl(a.ts, 1, 12))

export default 0;

=== /b.ts ===
import d1, { a as a1, b as b1 } from "./a";
>d1 : Symbol(d1, Decl(b.ts, 0, 6))
>a : Symbol(a1, Decl(b.ts, 0, 12))
>a1 : Symbol(a1, Decl(b.ts, 0, 12))
>b : Symbol(b1, Decl(b.ts, 0, 21))
>b1 : Symbol(b1, Decl(b.ts, 0, 21))

import d2, * as ns from "./a";
>d2 : Symbol(d2, Decl(b.ts, 1, 6))
>ns : Symbol(ns, Decl(b.ts, 1, 10))

import d3, { a as a2, b as b2 } from "./a";
>d3 : Symbol(d3, Decl(b.ts, 3, 6))
>a : Symbol(a2, Decl(b.ts, 3, 12))
>a2 : Symbol(a2, Decl(b.ts, 3, 12))
>b : Symbol(b2, Decl(b.ts, 3, 21))
>b2 : Symbol(b2, Decl(b.ts, 3, 21))

d3;
>d3 : Symbol(d3, Decl(b.ts, 3, 6))

import d4, * as ns2 from "./a";
>d4 : Symbol(d4, Decl(b.ts, 5, 6))
>ns2 : Symbol(ns2, Decl(b.ts, 5, 10))

d4;
>d4 : Symbol(d4, Decl(b.ts, 5, 6))

import d5, * as ns3 from "./a";
>d5 : Symbol(d5, Decl(b.ts, 7, 6))
>ns3 : Symbol(ns3, Decl(b.ts, 7, 10))

ns3;
>ns3 : Symbol(ns3, Decl(b.ts, 7, 10))

Loading