diff --git a/src/harness/unittests/organizeImports.ts b/src/harness/unittests/organizeImports.ts
index 8d7b0904eb7..0b9781462a4 100644
--- a/src/harness/unittests/organizeImports.ts
+++ b/src/harness/unittests/organizeImports.ts
@@ -175,6 +175,17 @@ export default function F2();
`,
};
+ const reactLibFile = {
+ path: "/react.ts",
+ content: `
+export const React = {
+createElement: (_type, _props, _children) => {},
+};
+
+export const Other = 1;
+`,
+ };
+
// Don't bother to actually emit a baseline for this.
it("NoImports", () => {
const testFile = {
@@ -198,6 +209,30 @@ NS.F1();
D();
F1();
F2();
+`,
+ },
+ libFile);
+
+ testOrganizeImports("Unused_Some",
+ {
+ path: "/test.ts",
+ content: `
+import { F1, F2 } from "lib";
+import * as NS from "lib";
+import D from "lib";
+
+D();
+`,
+ },
+ libFile);
+
+ testOrganizeImports("Unused_All",
+ {
+ path: "/test.ts",
+ content: `
+import { F1, F2 } from "lib";
+import * as NS from "lib";
+import D from "lib";
`,
},
libFile);
@@ -217,7 +252,7 @@ D();
},
libFile);
- // tslint:disable no-invalid-template-strings
+ // tslint:disable no-invalid-template-strings
testOrganizeImports("MoveToTop_Invalid",
{
path: "/test.ts",
@@ -234,7 +269,7 @@ D();
`,
},
libFile);
- // tslint:enable no-invalid-template-strings
+ // tslint:enable no-invalid-template-strings
testOrganizeImports("CoalesceMultipleModules",
{
@@ -244,10 +279,11 @@ import { d } from "lib1";
import { b } from "lib1";
import { c } from "lib2";
import { a } from "lib2";
+a + b + c + d;
`,
},
- { path: "/lib1.ts", content: "" },
- { path: "/lib2.ts", content: "" });
+ { path: "/lib1.ts", content: "export const b = 1, d = 2;" },
+ { path: "/lib2.ts", content: "export const a = 3, c = 4;" });
testOrganizeImports("CoalesceTrivia",
{
@@ -273,6 +309,56 @@ F2();
{ path: "/lib1.ts", content: "" },
{ path: "/lib2.ts", content: "" });
+ testOrganizeImports("UnusedTrivia1",
+ {
+ path: "/test.ts",
+ content: `
+/*A*/import /*B*/ { /*C*/ F1 /*D*/ } /*E*/ from /*F*/ "lib" /*G*/;/*H*/ //I
+`,
+ },
+ libFile);
+
+ testOrganizeImports("UnusedTrivia2",
+ {
+ path: "/test.ts",
+ content: `
+/*A*/import /*B*/ { /*C*/ F1 /*D*/, /*E*/ F2 /*F*/ } /*G*/ from /*H*/ "lib" /*I*/;/*J*/ //K
+
+F1();
+`,
+ },
+ libFile);
+
+ testOrganizeImports("JsxFactoryUsed",
+ {
+ path: "/test.tsx",
+ content: `
+import { React, Other } from "react";
+
+
;
+`,
+ },
+ reactLibFile);
+
+ // This is descriptive, rather than normative
+ testOrganizeImports("JsxFactoryUnusedTsx",
+ {
+ path: "/test.tsx",
+ content: `
+import { React, Other } from "react";
+`,
+ },
+ reactLibFile);
+
+ testOrganizeImports("JsxFactoryUnusedTs",
+ {
+ path: "/test.ts",
+ content: `
+import { React, Other } from "react";
+`,
+ },
+ reactLibFile);
+
function testOrganizeImports(testName: string, testFile: TestFSWithWatch.FileOrFolder, ...otherFiles: TestFSWithWatch.FileOrFolder[]) {
it(testName, () => runBaseline(`organizeImports/${testName}.ts`, testFile, ...otherFiles));
}
@@ -298,6 +384,7 @@ F2();
function makeLanguageService(...files: TestFSWithWatch.FileOrFolder[]) {
const host = projectSystem.createServerHost(files);
const projectService = projectSystem.createProjectService(host, { useSingleInferredProject: true });
+ projectService.setCompilerOptionsForInferredProjects({ jsx: files.some(f => f.path.endsWith("x")) ? JsxEmit.React : JsxEmit.None });
files.forEach(f => projectService.openClientFile(f.path));
return projectService.inferredProjects[0].getLanguageService();
}
diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts
index e75db1aa59d..4a03864207e 100644
--- a/src/services/organizeImports.ts
+++ b/src/services/organizeImports.ts
@@ -1,9 +1,17 @@
/* @internal */
namespace ts.OrganizeImports {
+
+ /**
+ * Organize imports by:
+ * 1) Removing unused imports
+ * 2) Coalescing imports from the same module
+ * 3) Sorting imports
+ */
export function organizeImports(
sourceFile: SourceFile,
formatContext: formatting.FormatContext,
- host: LanguageServiceHost) {
+ host: LanguageServiceHost,
+ program: Program) {
// TODO (https://github.com/Microsoft/TypeScript/issues/10020): sort *within* ambient modules (find using isAmbientModule)
@@ -21,7 +29,7 @@ namespace ts.OrganizeImports {
const newImportDecls = flatMap(sortedImportGroups, importGroup =>
getExternalModuleName(importGroup[0].moduleSpecifier)
- ? coalesceImports(removeUnusedImports(importGroup))
+ ? coalesceImports(removeUnusedImports(importGroup, sourceFile, program))
: importGroup);
const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext });
@@ -47,8 +55,73 @@ namespace ts.OrganizeImports {
return changeTracker.getChanges();
}
- function removeUnusedImports(oldImports: ReadonlyArray) {
- return oldImports; // TODO (https://github.com/Microsoft/TypeScript/issues/10020)
+ function removeUnusedImports(oldImports: ReadonlyArray, sourceFile: SourceFile, program: Program) {
+ const typeChecker = program.getTypeChecker();
+ const jsxNamespace = typeChecker.getJsxNamespace();
+ const jsxContext = sourceFile.languageVariant === LanguageVariant.JSX && program.getCompilerOptions().jsx;
+
+ const usedImports: ImportDeclaration[] = [];
+
+ for (const importDecl of oldImports) {
+ const {importClause} = importDecl;
+
+ if (!importClause) {
+ // Imports without import clauses are assumed to be included for their side effects and are not removed.
+ usedImports.push(importDecl);
+ continue;
+ }
+
+ let { name, namedBindings } = importClause;
+
+ // Default import
+ if (name && !isDeclarationUsed(name)) {
+ name = undefined;
+ }
+
+ if (namedBindings) {
+ if (isNamespaceImport(namedBindings)) {
+ // Namespace import
+ if (!isDeclarationUsed(namedBindings.name)) {
+ namedBindings = undefined;
+ }
+ }
+ else {
+ // List of named imports
+ const newElements = namedBindings.elements.filter(e => isDeclarationUsed(e.propertyName || e.name));
+ if (newElements.length < namedBindings.elements.length) {
+ namedBindings = newElements.length
+ ? updateNamedImports(namedBindings, newElements)
+ : undefined;
+ }
+ }
+ }
+
+ if (name || namedBindings) {
+ usedImports.push(updateImportDeclarationAndClause(importDecl, name, namedBindings));
+ }
+ }
+
+ return usedImports;
+
+ function isDeclarationUsed(identifier: Identifier) {
+ const symbol = typeChecker.getSymbolAtLocation(identifier);
+
+ // Be lenient with invalid code.
+ if (symbol === undefined) {
+ return true;
+ }
+
+ // The JSX factory symbol is always used.
+ if (jsxContext && symbol.name === jsxNamespace) {
+ return true;
+ }
+
+ const entries = FindAllReferences.getReferenceEntriesForNode(identifier.pos, identifier, program, [sourceFile], {
+ isCancellationRequested: () => false,
+ throwIfCancellationRequested: () => { /*noop*/ },
+ });
+ return entries.length > 1;
+ }
}
function getExternalModuleName(specifier: Expression) {
@@ -66,7 +139,7 @@ namespace ts.OrganizeImports {
return importGroup;
}
- const { importWithoutClause, defaultImports, namespaceImports, namedImports } = getImportParts(importGroup);
+ const { importWithoutClause, defaultImports, namespaceImports, namedImports } = getCategorizedImports(importGroup);
const coalescedImports: ImportDeclaration[] = [];
@@ -78,19 +151,20 @@ namespace ts.OrganizeImports {
// produce two import declarations in this special case.
if (defaultImports.length === 1 && namespaceImports.length === 1 && namedImports.length === 0) {
// Add the namespace import to the existing default ImportDeclaration.
- const defaultImportClause = defaultImports[0].parent as ImportClause;
+ const defaultImport = defaultImports[0];
coalescedImports.push(
- updateImportDeclarationAndClause(defaultImportClause, defaultImportClause.name, namespaceImports[0]));
+ updateImportDeclarationAndClause(defaultImport, defaultImport.importClause.name, namespaceImports[0].importClause.namedBindings));
return coalescedImports;
}
- const sortedNamespaceImports = stableSort(namespaceImports, (n1, n2) => compareIdentifiers(n1.name, n2.name));
+ const sortedNamespaceImports = stableSort(namespaceImports, (i1, i2) =>
+ compareIdentifiers((i1.importClause.namedBindings as NamespaceImport).name, (i2.importClause.namedBindings as NamespaceImport).name));
for (const namespaceImport of sortedNamespaceImports) {
// Drop the name, if any
coalescedImports.push(
- updateImportDeclarationAndClause(namespaceImport.parent, /*name*/ undefined, namespaceImport));
+ updateImportDeclarationAndClause(namespaceImport, /*name*/ undefined, namespaceImport.importClause.namedBindings));
}
if (defaultImports.length === 0 && namedImports.length === 0) {
@@ -100,41 +174,48 @@ namespace ts.OrganizeImports {
let newDefaultImport: Identifier | undefined;
const newImportSpecifiers: ImportSpecifier[] = [];
if (defaultImports.length === 1) {
- newDefaultImport = defaultImports[0];
+ newDefaultImport = defaultImports[0].importClause.name;
}
else {
for (const defaultImport of defaultImports) {
newImportSpecifiers.push(
- createImportSpecifier(createIdentifier("default"), defaultImport));
+ createImportSpecifier(createIdentifier("default"), defaultImport.importClause.name));
}
}
- newImportSpecifiers.push(...flatMap(namedImports, n => n.elements));
+ newImportSpecifiers.push(...flatMap(namedImports, i => (i.importClause.namedBindings as NamedImports).elements));
const sortedImportSpecifiers = stableSort(newImportSpecifiers, (s1, s2) =>
compareIdentifiers(s1.propertyName || s1.name, s2.propertyName || s2.name) ||
compareIdentifiers(s1.name, s2.name));
- const importClause = defaultImports.length > 0
- ? defaultImports[0].parent as ImportClause
- : namedImports[0].parent;
+ const importDecl = defaultImports.length > 0
+ ? defaultImports[0]
+ : namedImports[0];
const newNamedImports = sortedImportSpecifiers.length === 0
? undefined
: namedImports.length === 0
? createNamedImports(sortedImportSpecifiers)
- : updateNamedImports(namedImports[0], sortedImportSpecifiers);
+ : updateNamedImports(namedImports[0].importClause.namedBindings as NamedImports, sortedImportSpecifiers);
coalescedImports.push(
- updateImportDeclarationAndClause(importClause, newDefaultImport, newNamedImports));
+ updateImportDeclarationAndClause(importDecl, newDefaultImport, newNamedImports));
return coalescedImports;
- function getImportParts(importGroup: ReadonlyArray) {
+ /*
+ * Returns entire import declarations because they may already have been rewritten and
+ * may lack parent pointers. The desired parts can easily be recovered based on the
+ * categorization.
+ *
+ * NB: There may be overlap between `defaultImports` and `namespaceImports`/`namedImports`.
+ */
+ function getCategorizedImports(importGroup: ReadonlyArray) {
let importWithoutClause: ImportDeclaration | undefined;
- const defaultImports: Identifier[] = [];
- const namespaceImports: NamespaceImport[] = [];
- const namedImports: NamedImports[] = [];
+ const defaultImports: ImportDeclaration[] = [];
+ const namespaceImports: ImportDeclaration[] = [];
+ const namedImports: ImportDeclaration[] = [];
for (const importDeclaration of importGroup) {
if (importDeclaration.importClause === undefined) {
@@ -147,15 +228,15 @@ namespace ts.OrganizeImports {
const { name, namedBindings } = importDeclaration.importClause;
if (name) {
- defaultImports.push(name);
+ defaultImports.push(importDeclaration);
}
if (namedBindings) {
if (isNamespaceImport(namedBindings)) {
- namespaceImports.push(namedBindings);
+ namespaceImports.push(importDeclaration);
}
else {
- namedImports.push(namedBindings);
+ namedImports.push(importDeclaration);
}
}
}
@@ -171,20 +252,19 @@ namespace ts.OrganizeImports {
function compareIdentifiers(s1: Identifier, s2: Identifier) {
return compareStringsCaseSensitive(s1.text, s2.text);
}
+ }
- function updateImportDeclarationAndClause(
- importClause: ImportClause,
- name: Identifier | undefined,
- namedBindings: NamedImportBindings | undefined) {
+ function updateImportDeclarationAndClause(
+ importDeclaration: ImportDeclaration,
+ name: Identifier | undefined,
+ namedBindings: NamedImportBindings | undefined) {
- const importDeclaration = importClause.parent;
- return updateImportDeclaration(
- importDeclaration,
- importDeclaration.decorators,
- importDeclaration.modifiers,
- updateImportClause(importClause, name, namedBindings),
- importDeclaration.moduleSpecifier);
- }
+ return updateImportDeclaration(
+ importDeclaration,
+ importDeclaration.decorators,
+ importDeclaration.modifiers,
+ updateImportClause(importDeclaration.importClause, name, namedBindings),
+ importDeclaration.moduleSpecifier);
}
/* internal */ // Exported for testing
diff --git a/src/services/services.ts b/src/services/services.ts
index b5d6f0ec2a6..1aecef09e90 100644
--- a/src/services/services.ts
+++ b/src/services/services.ts
@@ -1855,7 +1855,7 @@ namespace ts {
const sourceFile = getValidSourceFile(scope.fileName);
const formatContext = formatting.getFormatContext(formatOptions);
- return OrganizeImports.organizeImports(sourceFile, formatContext, host);
+ return OrganizeImports.organizeImports(sourceFile, formatContext, host, program);
}
function applyCodeActionCommand(action: CodeActionCommand): Promise;
diff --git a/tests/baselines/reference/organizeImports/CoalesceMultipleModules.ts b/tests/baselines/reference/organizeImports/CoalesceMultipleModules.ts
index 6278722f2a9..c321f0f8fc5 100644
--- a/tests/baselines/reference/organizeImports/CoalesceMultipleModules.ts
+++ b/tests/baselines/reference/organizeImports/CoalesceMultipleModules.ts
@@ -4,8 +4,10 @@ import { d } from "lib1";
import { b } from "lib1";
import { c } from "lib2";
import { a } from "lib2";
+a + b + c + d;
// ==ORGANIZED==
import { b, d } from "lib1";
import { a, c } from "lib2";
+a + b + c + d;
diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedTs.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedTs.ts
new file mode 100644
index 00000000000..60afb95a192
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedTs.ts
@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+// ==ORGANIZED==
+
diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts b/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts
new file mode 100644
index 00000000000..6a97e7f660a
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts
@@ -0,0 +1,7 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+// ==ORGANIZED==
+
+import { React } from "react";
diff --git a/tests/baselines/reference/organizeImports/JsxFactoryUsed.ts b/tests/baselines/reference/organizeImports/JsxFactoryUsed.ts
new file mode 100644
index 00000000000..430a5b12cd4
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/JsxFactoryUsed.ts
@@ -0,0 +1,11 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+;
+
+// ==ORGANIZED==
+
+import { React } from "react";
+
+;
diff --git a/tests/baselines/reference/organizeImports/UnusedTrivia1.ts b/tests/baselines/reference/organizeImports/UnusedTrivia1.ts
new file mode 100644
index 00000000000..7c25f40e6c2
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/UnusedTrivia1.ts
@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+
+/*A*/import /*B*/ { /*C*/ F1 /*D*/ } /*E*/ from /*F*/ "lib" /*G*/;/*H*/ //I
+
+// ==ORGANIZED==
+
diff --git a/tests/baselines/reference/organizeImports/UnusedTrivia2.ts b/tests/baselines/reference/organizeImports/UnusedTrivia2.ts
new file mode 100644
index 00000000000..f853015303e
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/UnusedTrivia2.ts
@@ -0,0 +1,12 @@
+// ==ORIGINAL==
+
+/*A*/import /*B*/ { /*C*/ F1 /*D*/, /*E*/ F2 /*F*/ } /*G*/ from /*H*/ "lib" /*I*/;/*J*/ //K
+
+F1();
+
+// ==ORGANIZED==
+
+/*A*/ import { /*C*/ F1 /*D*/ } /*G*/ from "lib" /*I*/; /*J*/ //K
+
+
+F1();
diff --git a/tests/baselines/reference/organizeImports/Unused_All.ts b/tests/baselines/reference/organizeImports/Unused_All.ts
new file mode 100644
index 00000000000..bef217724b7
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/Unused_All.ts
@@ -0,0 +1,8 @@
+// ==ORIGINAL==
+
+import { F1, F2 } from "lib";
+import * as NS from "lib";
+import D from "lib";
+
+// ==ORGANIZED==
+
diff --git a/tests/baselines/reference/organizeImports/Unused_Some.ts b/tests/baselines/reference/organizeImports/Unused_Some.ts
new file mode 100644
index 00000000000..2f0aaa6d499
--- /dev/null
+++ b/tests/baselines/reference/organizeImports/Unused_Some.ts
@@ -0,0 +1,13 @@
+// ==ORIGINAL==
+
+import { F1, F2 } from "lib";
+import * as NS from "lib";
+import D from "lib";
+
+D();
+
+// ==ORGANIZED==
+
+import D from "lib";
+
+D();