Implement ts.OrganizeImports.removeUnusedImports

TODO: Still need to add support for organizing imports in ambient
modules

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";
+
+<div/>;
+`,
+                },
+                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();
             }

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<ImportDeclaration>) {
-        return oldImports; // TODO (https://github.com/Microsoft/TypeScript/issues/10020)
+    function removeUnusedImports(oldImports: ReadonlyArray<ImportDeclaration>, 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<ImportDeclaration>) {
+        /*
+         * 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<ImportDeclaration>) {
             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

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<ApplyCodeActionCommandResult>;

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;

tests/baselines/reference/organizeImports/JsxFactoryUnusedTs.ts

@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+// ==ORGANIZED==
+

tests/baselines/reference/organizeImports/JsxFactoryUnusedTsx.ts

@@ -0,0 +1,7 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+// ==ORGANIZED==
+
+import { React } from "react";

tests/baselines/reference/organizeImports/JsxFactoryUsed.ts

@@ -0,0 +1,11 @@
+// ==ORIGINAL==
+
+import { React, Other } from "react";
+
+<div/>;
+
+// ==ORGANIZED==
+
+import { React } from "react";
+
+<div/>;

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==
+

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();

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==
+

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();