diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0ee0b9cc37a48..6d8d084acc580 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -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(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) { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index d8545e015290d..9f95a66711b75 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -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 @@ -3851,6 +3855,10 @@ "category": "Message", "code": 90004 }, + "Remove import from '{0}'": { + "category": "Message", + "code": 90005 + }, "Implement interface '{0}'": { "category": "Message", "code": 90006 diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 5552c6f988ad5..9e65c5e7e8602 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1204,7 +1204,7 @@ namespace ts { && (node).expression.kind === SyntaxKind.ThisKeyword; } - export function getEntityNameFromTypeNode(node: TypeNode): EntityNameOrEntityNameExpression { + export function getEntityNameFromTypeNode(node: TypeNode): EntityNameOrEntityNameExpression { switch (node.kind) { case SyntaxKind.TypeReference: return (node).typeName; @@ -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 { diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index d074f2e0e5320..ad40b62a53a54 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -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[] = []; @@ -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 }); @@ -38,7 +46,13 @@ 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)); @@ -46,6 +60,12 @@ namespace ts.codefix { }), }); + // 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; diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 0e75158c4d1c3..8121576a02994 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -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); diff --git a/tests/baselines/reference/extendsUntypedModule.errors.txt b/tests/baselines/reference/extendsUntypedModule.errors.txt index 8df9e8a1c3671..aded38736e831 100644 --- a/tests/baselines/reference/extendsUntypedModule.errors.txt +++ b/tests/baselines/reference/extendsUntypedModule.errors.txt @@ -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 { } diff --git a/tests/baselines/reference/unusedImports1.errors.txt b/tests/baselines/reference/unusedImports1.errors.txt index 36963513146a9..d329b1e89a88c 100644 --- a/tests/baselines/reference/unusedImports1.errors.txt +++ b/tests/baselines/reference/unusedImports1.errors.txt @@ -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) ==== @@ -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. \ No newline at end of file diff --git a/tests/baselines/reference/unusedImports12.errors.txt b/tests/baselines/reference/unusedImports12.errors.txt index 84f1b91884dba..5bdd99d604464 100644 --- a/tests/baselines/reference/unusedImports12.errors.txt +++ b/tests/baselines/reference/unusedImports12.errors.txt @@ -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"); ~~~~~~~~ diff --git a/tests/baselines/reference/unusedImports2.errors.txt b/tests/baselines/reference/unusedImports2.errors.txt index c037eea2f512b..65d067f34f1c7 100644 --- a/tests/baselines/reference/unusedImports2.errors.txt +++ b/tests/baselines/reference/unusedImports2.errors.txt @@ -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) ==== @@ -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(); diff --git a/tests/baselines/reference/unusedImports6.errors.txt b/tests/baselines/reference/unusedImports6.errors.txt index 28b2d1a2875c1..a211dec9b7024 100644 --- a/tests/baselines/reference/unusedImports6.errors.txt +++ b/tests/baselines/reference/unusedImports6.errors.txt @@ -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) ==== @@ -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. diff --git a/tests/baselines/reference/unusedImports7.errors.txt b/tests/baselines/reference/unusedImports7.errors.txt index 57338073efda2..52411e3403a7a 100644 --- a/tests/baselines/reference/unusedImports7.errors.txt +++ b/tests/baselines/reference/unusedImports7.errors.txt @@ -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) ==== @@ -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. \ No newline at end of file diff --git a/tests/baselines/reference/unusedImports_entireImportDeclaration.errors.txt b/tests/baselines/reference/unusedImports_entireImportDeclaration.errors.txt new file mode 100644 index 0000000000000..cd58ce714b06e --- /dev/null +++ b/tests/baselines/reference/unusedImports_entireImportDeclaration.errors.txt @@ -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; + \ No newline at end of file diff --git a/tests/baselines/reference/unusedImports_entireImportDeclaration.js b/tests/baselines/reference/unusedImports_entireImportDeclaration.js new file mode 100644 index 0000000000000..9bc77f50f7a6c --- /dev/null +++ b/tests/baselines/reference/unusedImports_entireImportDeclaration.js @@ -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; diff --git a/tests/baselines/reference/unusedImports_entireImportDeclaration.symbols b/tests/baselines/reference/unusedImports_entireImportDeclaration.symbols new file mode 100644 index 0000000000000..bd02fe90992b4 --- /dev/null +++ b/tests/baselines/reference/unusedImports_entireImportDeclaration.symbols @@ -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)) + diff --git a/tests/baselines/reference/unusedImports_entireImportDeclaration.types b/tests/baselines/reference/unusedImports_entireImportDeclaration.types new file mode 100644 index 0000000000000..82cacfd53146a --- /dev/null +++ b/tests/baselines/reference/unusedImports_entireImportDeclaration.types @@ -0,0 +1,47 @@ +=== /a.ts === +export const a = 0; +>a : 0 +>0 : 0 + +export const b = 0; +>b : 0 +>0 : 0 + +export default 0; + +=== /b.ts === +import d1, { a as a1, b as b1 } from "./a"; +>d1 : 0 +>a : 0 +>a1 : 0 +>b : 0 +>b1 : 0 + +import d2, * as ns from "./a"; +>d2 : 0 +>ns : typeof ns + +import d3, { a as a2, b as b2 } from "./a"; +>d3 : 0 +>a : 0 +>a2 : 0 +>b : 0 +>b2 : 0 + +d3; +>d3 : 0 + +import d4, * as ns2 from "./a"; +>d4 : 0 +>ns2 : typeof ns + +d4; +>d4 : 0 + +import d5, * as ns3 from "./a"; +>d5 : 0 +>ns3 : typeof ns + +ns3; +>ns3 : typeof ns + diff --git a/tests/cases/compiler/unusedImports_entireImportDeclaration.ts b/tests/cases/compiler/unusedImports_entireImportDeclaration.ts new file mode 100644 index 0000000000000..fa7fcb7f59632 --- /dev/null +++ b/tests/cases/compiler/unusedImports_entireImportDeclaration.ts @@ -0,0 +1,17 @@ +// @noUnusedLocals: true + +// @Filename: /a.ts +export const a = 0; +export const b = 0; +export default 0; + +// @Filename: /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; diff --git a/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts b/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts new file mode 100644 index 0000000000000..a75ef81b1ed39 --- /dev/null +++ b/tests/cases/fourslash/unusedImportsFS_entireImportDeclaration.ts @@ -0,0 +1,11 @@ +/// + +// @noUnusedLocals: true +// @Filename: /a.ts +////// leading trivia +////import a, { b } from "mod"; // trailing trivia + +verify.codeFix({ + description: "Remove import from 'mod'", + newFileContent: " // trailing trivia", +});