From 99b04c702edf5e7fd72827aa0769d77df5331734 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sat, 10 Feb 2018 11:34:00 -0800
Subject: [PATCH 1/9] Refactored insertNodeInListAfter in preparation for
 alphabetizing

---
 src/services/textChanges.ts | 240 +++++++++++++++++++-----------------
 1 file changed, 124 insertions(+), 116 deletions(-)

diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index 994d4fcb6bfc8..04806eb3a8016 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -443,6 +443,128 @@ namespace ts.textChanges {
             throw Debug.failBadSyntaxKind(node); // We haven't handled this kind of node yet -- add it
         }
 
+        private insertNodeInListAfterNotLast(sourceFile: SourceFile, containingList: NodeArray<Node>, nextToken: Node, index: number, newNode: Node): void {
+            // for list
+            // a, b, c
+            // create change for adding 'e' after 'a' as
+            // - find start of next element after a (it is b)
+            // - use this start as start and end position in final change
+            // - build text of change by formatting the text of node + separator + whitespace trivia of b
+
+            // in multiline case it will work as
+            //   a,
+            //   b,
+            //   c,
+            // result - '*' denotes leading trivia that will be inserted after new text (displayed as '#')
+            //   a,*
+            // ***insertedtext<separator>#
+            // ###b,
+            //   c,
+            // find line and character of the next element
+            const lineAndCharOfNextElement = getLineAndCharacterOfPosition(sourceFile, skipWhitespacesAndLineBreaks(sourceFile.text, containingList[index + 1].getFullStart()));
+            // find line and character of the token that precedes next element (usually it is separator)
+            const lineAndCharOfNextToken = getLineAndCharacterOfPosition(sourceFile, nextToken.end);
+            let prefix: string;
+            let startPos: number;
+            if (lineAndCharOfNextToken.line === lineAndCharOfNextElement.line) {
+                // next element is located on the same line with separator:
+                // a,$$$$b
+                //  ^    ^
+                //  |    |-next element
+                //  |-separator
+                // where $$$ is some leading trivia
+                // for a newly inserted node we'll maintain the same relative position comparing to separator and replace leading trivia with spaces
+                // a,    x,$$$$b
+                //  ^    ^     ^
+                //  |    |     |-next element
+                //  |    |-new inserted node padded with spaces
+                //  |-separator
+                startPos = nextToken.end;
+                prefix = spaces(lineAndCharOfNextElement.character - lineAndCharOfNextToken.character);
+            }
+            else {
+                // next element is located on different line that separator
+                // let insert position be the beginning of the line that contains next element
+                startPos = getStartPositionOfLine(lineAndCharOfNextElement.line, sourceFile);
+            }
+
+            this.changes.push({
+                kind: ChangeKind.ReplaceWithSingleNode,
+                sourceFile,
+                range: { pos: startPos, end: containingList[index + 1].getStart(sourceFile) },
+                node: newNode,
+                options: {
+                    prefix,
+                    // write separator and leading trivia of the next element as suffix
+                    suffix: `${tokenToString(nextToken.kind)}${sourceFile.text.substring(nextToken.end, containingList[index + 1].getStart(sourceFile))}`
+                }
+            });
+        }
+
+        private insertNodeInListAfterLast(sourceFile: SourceFile, containingList: NodeArray<Node>, after: Node, end: number, index: number, newNode: Node): void {
+            const afterStart = after.getStart(sourceFile);
+            const afterStartLinePosition = getLineStartPositionForPosition(afterStart, sourceFile);
+
+            let separator: SyntaxKind.CommaToken | SyntaxKind.SemicolonToken;
+            let multilineList = false;
+
+            // insert element after the last element in the list that has more than one item
+            // pick the element preceding the after element to:
+            // - pick the separator
+            // - determine if list is a multiline
+            if (containingList.length === 1) {
+                // if list has only one element then we'll format is as multiline if node has comment in trailing trivia, or as singleline otherwise
+                // i.e. var x = 1 // this is x
+                //     | new element will be inserted at this position
+                separator = SyntaxKind.CommaToken;
+            }
+            else {
+                // element has more than one element, pick separator from the list
+                const tokenBeforeInsertPosition = findPrecedingToken(after.pos, sourceFile);
+                separator = isSeparator(after, tokenBeforeInsertPosition) ? tokenBeforeInsertPosition.kind : SyntaxKind.CommaToken;
+                // determine if list is multiline by checking lines of after element and element that precedes it.
+                const afterMinusOneStartLinePosition = getLineStartPositionForPosition(containingList[index - 1].getStart(sourceFile), sourceFile);
+                multilineList = afterMinusOneStartLinePosition !== afterStartLinePosition;
+            }
+            if (hasCommentsBeforeLineBreak(sourceFile.text, after.end)) {
+                // in this case we'll always treat containing list as multiline
+                multilineList = true;
+            }
+            if (multilineList) {
+                // insert separator immediately following the 'after' node to preserve comments in trailing trivia
+                this.changes.push({
+                    kind: ChangeKind.ReplaceWithSingleNode,
+                    sourceFile,
+                    range: { pos: end, end },
+                    node: createToken(separator),
+                    options: {}
+                });
+                // use the same indentation as 'after' item
+                const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(afterStartLinePosition, afterStart, sourceFile, this.formatContext.options);
+                // insert element before the line break on the line that contains 'after' element
+                let insertPos = skipTrivia(sourceFile.text, end, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false);
+                if (insertPos !== end && isLineBreak(sourceFile.text.charCodeAt(insertPos - 1))) {
+                    insertPos--;
+                }
+                this.changes.push({
+                    kind: ChangeKind.ReplaceWithSingleNode,
+                    sourceFile,
+                    range: { pos: insertPos, end: insertPos },
+                    node: newNode,
+                    options: { indentation, prefix: this.newLineCharacter }
+                });
+            }
+            else {
+                this.changes.push({
+                    kind: ChangeKind.ReplaceWithSingleNode,
+                    sourceFile,
+                    range: { pos: end, end },
+                    node: newNode,
+                    options: { prefix: `${tokenToString(separator)} ` }
+                });
+            }
+        }
+
         /**
          * This function should be used to insert nodes in lists when nodes don't carry separators as the part of the node range,
          * i.e. arguments in arguments lists, parameters in parameter lists etc.
@@ -464,125 +586,11 @@ namespace ts.textChanges {
                 // use next sibling as an anchor
                 const nextToken = getTokenAtPosition(sourceFile, after.end, /*includeJsDocComment*/ false);
                 if (nextToken && isSeparator(after, nextToken)) {
-                    // for list
-                    // a, b, c
-                    // create change for adding 'e' after 'a' as
-                    // - find start of next element after a (it is b)
-                    // - use this start as start and end position in final change
-                    // - build text of change by formatting the text of node + separator + whitespace trivia of b
-
-                    // in multiline case it will work as
-                    //   a,
-                    //   b,
-                    //   c,
-                    // result - '*' denotes leading trivia that will be inserted after new text (displayed as '#')
-                    //   a,*
-                    // ***insertedtext<separator>#
-                    // ###b,
-                    //   c,
-                    // find line and character of the next element
-                    const lineAndCharOfNextElement = getLineAndCharacterOfPosition(sourceFile, skipWhitespacesAndLineBreaks(sourceFile.text, containingList[index + 1].getFullStart()));
-                    // find line and character of the token that precedes next element (usually it is separator)
-                    const lineAndCharOfNextToken = getLineAndCharacterOfPosition(sourceFile, nextToken.end);
-                    let prefix: string;
-                    let startPos: number;
-                    if (lineAndCharOfNextToken.line === lineAndCharOfNextElement.line) {
-                        // next element is located on the same line with separator:
-                        // a,$$$$b
-                        //  ^    ^
-                        //  |    |-next element
-                        //  |-separator
-                        // where $$$ is some leading trivia
-                        // for a newly inserted node we'll maintain the same relative position comparing to separator and replace leading trivia with spaces
-                        // a,    x,$$$$b
-                        //  ^    ^     ^
-                        //  |    |     |-next element
-                        //  |    |-new inserted node padded with spaces
-                        //  |-separator
-                        startPos = nextToken.end;
-                        prefix = spaces(lineAndCharOfNextElement.character - lineAndCharOfNextToken.character);
-                    }
-                    else {
-                        // next element is located on different line that separator
-                        // let insert position be the beginning of the line that contains next element
-                        startPos = getStartPositionOfLine(lineAndCharOfNextElement.line, sourceFile);
-                    }
-
-                    this.changes.push({
-                        kind: ChangeKind.ReplaceWithSingleNode,
-                        sourceFile,
-                        range: { pos: startPos, end: containingList[index + 1].getStart(sourceFile) },
-                        node: newNode,
-                        options: {
-                            prefix,
-                            // write separator and leading trivia of the next element as suffix
-                            suffix: `${tokenToString(nextToken.kind)}${sourceFile.text.substring(nextToken.end, containingList[index + 1].getStart(sourceFile))}`
-                        }
-                    });
+                    this.insertNodeInListAfterNotLast(sourceFile, containingList, nextToken, index, newNode);
                 }
             }
             else {
-                const afterStart = after.getStart(sourceFile);
-                const afterStartLinePosition = getLineStartPositionForPosition(afterStart, sourceFile);
-
-                let separator: SyntaxKind.CommaToken | SyntaxKind.SemicolonToken;
-                let multilineList = false;
-
-                // insert element after the last element in the list that has more than one item
-                // pick the element preceding the after element to:
-                // - pick the separator
-                // - determine if list is a multiline
-                if (containingList.length === 1) {
-                    // if list has only one element then we'll format is as multiline if node has comment in trailing trivia, or as singleline otherwise
-                    // i.e. var x = 1 // this is x
-                    //     | new element will be inserted at this position
-                    separator = SyntaxKind.CommaToken;
-                }
-                else {
-                    // element has more than one element, pick separator from the list
-                    const tokenBeforeInsertPosition = findPrecedingToken(after.pos, sourceFile);
-                    separator = isSeparator(after, tokenBeforeInsertPosition) ? tokenBeforeInsertPosition.kind : SyntaxKind.CommaToken;
-                    // determine if list is multiline by checking lines of after element and element that precedes it.
-                    const afterMinusOneStartLinePosition = getLineStartPositionForPosition(containingList[index - 1].getStart(sourceFile), sourceFile);
-                    multilineList = afterMinusOneStartLinePosition !== afterStartLinePosition;
-                }
-                if (hasCommentsBeforeLineBreak(sourceFile.text, after.end)) {
-                    // in this case we'll always treat containing list as multiline
-                    multilineList = true;
-                }
-                if (multilineList) {
-                    // insert separator immediately following the 'after' node to preserve comments in trailing trivia
-                    this.changes.push({
-                        kind: ChangeKind.ReplaceWithSingleNode,
-                        sourceFile,
-                        range: { pos: end, end },
-                        node: createToken(separator),
-                        options: {}
-                    });
-                    // use the same indentation as 'after' item
-                    const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(afterStartLinePosition, afterStart, sourceFile, this.formatContext.options);
-                    // insert element before the line break on the line that contains 'after' element
-                    let insertPos = skipTrivia(sourceFile.text, end, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false);
-                    if (insertPos !== end && isLineBreak(sourceFile.text.charCodeAt(insertPos - 1))) {
-                        insertPos--;
-                    }
-                    this.changes.push({
-                        kind: ChangeKind.ReplaceWithSingleNode,
-                        sourceFile,
-                        range: { pos: insertPos, end: insertPos },
-                        node: newNode,
-                        options: { indentation, prefix: this.newLineCharacter }
-                    });
-                }
-                else {
-                    this.changes.push({
-                        kind: ChangeKind.ReplaceWithSingleNode,
-                        sourceFile,
-                        range: { pos: end, end },
-                        node: newNode,
-                        options: { prefix: `${tokenToString(separator)} ` }
-                    });
-                }
+                this.insertNodeInListAfterLast(sourceFile, containingList, after, end, index, newNode);
             }
             return this;
         }

From 94bffe0a8c27d4783b451b00d446a202b5d690c7 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sat, 10 Feb 2018 11:41:59 -0800
Subject: [PATCH 2/9] Moved end into ~AfterLast (slightly simpler this way)

---
 src/services/textChanges.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index 04806eb3a8016..741c78fed149d 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -501,7 +501,8 @@ namespace ts.textChanges {
             });
         }
 
-        private insertNodeInListAfterLast(sourceFile: SourceFile, containingList: NodeArray<Node>, after: Node, end: number, index: number, newNode: Node): void {
+        private insertNodeInListAfterLast(sourceFile: SourceFile, containingList: NodeArray<Node>, after: Node, index: number, newNode: Node): void {
+            const end = after.getEnd();
             const afterStart = after.getStart(sourceFile);
             const afterStartLinePosition = getLineStartPositionForPosition(afterStart, sourceFile);
 
@@ -580,7 +581,6 @@ namespace ts.textChanges {
             if (index < 0) {
                 return this;
             }
-            const end = after.getEnd();
             if (index !== containingList.length - 1) {
                 // any element except the last one
                 // use next sibling as an anchor
@@ -590,7 +590,7 @@ namespace ts.textChanges {
                 }
             }
             else {
-                this.insertNodeInListAfterLast(sourceFile, containingList, after, end, index, newNode);
+                this.insertNodeInListAfterLast(sourceFile, containingList, after, index, newNode);
             }
             return this;
         }

From a361cb1b3e02f6589de9cef0a4745c29ec84c0e7 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sat, 10 Feb 2018 17:12:36 -0800
Subject: [PATCH 3/9] Moved getMultilineAndSeparatorOfList into its own
 function

I think I'll need it for inserting a node before the first in a list.
---
 src/services/textChanges.ts | 55 +++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index 741c78fed149d..fd3860d0aeae6 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -506,31 +506,8 @@ namespace ts.textChanges {
             const afterStart = after.getStart(sourceFile);
             const afterStartLinePosition = getLineStartPositionForPosition(afterStart, sourceFile);
 
-            let separator: SyntaxKind.CommaToken | SyntaxKind.SemicolonToken;
-            let multilineList = false;
+            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, index, after);
 
-            // insert element after the last element in the list that has more than one item
-            // pick the element preceding the after element to:
-            // - pick the separator
-            // - determine if list is a multiline
-            if (containingList.length === 1) {
-                // if list has only one element then we'll format is as multiline if node has comment in trailing trivia, or as singleline otherwise
-                // i.e. var x = 1 // this is x
-                //     | new element will be inserted at this position
-                separator = SyntaxKind.CommaToken;
-            }
-            else {
-                // element has more than one element, pick separator from the list
-                const tokenBeforeInsertPosition = findPrecedingToken(after.pos, sourceFile);
-                separator = isSeparator(after, tokenBeforeInsertPosition) ? tokenBeforeInsertPosition.kind : SyntaxKind.CommaToken;
-                // determine if list is multiline by checking lines of after element and element that precedes it.
-                const afterMinusOneStartLinePosition = getLineStartPositionForPosition(containingList[index - 1].getStart(sourceFile), sourceFile);
-                multilineList = afterMinusOneStartLinePosition !== afterStartLinePosition;
-            }
-            if (hasCommentsBeforeLineBreak(sourceFile.text, after.end)) {
-                // in this case we'll always treat containing list as multiline
-                multilineList = true;
-            }
             if (multilineList) {
                 // insert separator immediately following the 'after' node to preserve comments in trailing trivia
                 this.changes.push({
@@ -566,6 +543,36 @@ namespace ts.textChanges {
             }
         }
 
+        private getMultilineAndSeparatorOfList(sourceFile: SourceFile, containingList: NodeArray<Node>, afterStartLinePosition: number, sampleIndex: number, sampleNode: Node) {
+            let separator: SyntaxKind.CommaToken | SyntaxKind.SemicolonToken;
+            let multilineList = false;
+
+            // insert element after the last element in the list that has more than one item
+            // pick the element preceding the after element to:
+            // - pick the separator
+            // - determine if list is a multiline
+            if (containingList.length === 1) {
+                // if list has only one element then we'll format is as multiline if node has comment in trailing trivia, or as singleline otherwise
+                // i.e. var x = 1 // this is x
+                //     | new element will be inserted at this position
+                separator = SyntaxKind.CommaToken;
+            }
+            else {
+                // element has more than one element, pick separator from the list
+                const tokenBeforeInsertPosition = findPrecedingToken(sampleNode.pos, sourceFile);
+                separator = isSeparator(sampleNode, tokenBeforeInsertPosition) ? tokenBeforeInsertPosition.kind : SyntaxKind.CommaToken;
+                // determine if list is multiline by checking lines of after element and element that precedes it.
+                const afterMinusOneStartLinePosition = getLineStartPositionForPosition(containingList[sampleIndex - 1].getStart(sourceFile), sourceFile);
+                multilineList = afterMinusOneStartLinePosition !== afterStartLinePosition;
+            }
+            if (hasCommentsBeforeLineBreak(sourceFile.text, sampleNode.end)) {
+                // in this case we'll always treat containing list as multiline
+                multilineList = true;
+            }
+
+            return { multilineList, separator };
+        }
+
         /**
          * This function should be used to insert nodes in lists when nodes don't carry separators as the part of the node range,
          * i.e. arguments in arguments lists, parameters in parameter lists etc.

From 86c42fad7d552e2cce5b768f56017d50d0f17664 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sun, 11 Feb 2018 18:12:39 -0800
Subject: [PATCH 4/9] Added & used near-working alphabetical list insert

Searches through the elements until one with a "greater" (using native string compares) one is found, and inserts at that position.

Test cases 8, 10, and 11 are failing.
Copied them as 13, 14, and 15 for inserting in the middle of a list.
Copied them as 16, 17, and 18 for inserting at the end of a list.
---
 src/services/codefixes/importFixes.ts         |  4 +-
 src/services/textChanges.ts                   | 52 +++++++++++++++++++
 .../importNameCodeFixExistingImport0.ts       |  2 +-
 .../importNameCodeFixExistingImport1.ts       |  2 +-
 .../importNameCodeFixExistingImport10.ts      |  4 +-
 .../importNameCodeFixExistingImport11.ts      |  4 +-
 .../importNameCodeFixExistingImport12.ts      |  4 +-
 .../importNameCodeFixExistingImport13.ts      | 12 +++++
 .../importNameCodeFixExistingImport14.ts      | 20 +++++++
 .../importNameCodeFixExistingImport15.ts      | 20 +++++++
 .../importNameCodeFixExistingImport16.ts      | 12 +++++
 .../importNameCodeFixExistingImport17.ts      | 20 +++++++
 .../importNameCodeFixExistingImport18.ts      | 21 ++++++++
 .../importNameCodeFixExistingImport6.ts       |  2 +-
 .../importNameCodeFixExistingImport7.ts       |  2 +-
 .../importNameCodeFixExistingImport8.ts       |  2 +-
 .../importNameCodeFixExistingImport9.ts       |  2 +-
 17 files changed, 171 insertions(+), 14 deletions(-)
 create mode 100644 tests/cases/fourslash/importNameCodeFixExistingImport13.ts
 create mode 100644 tests/cases/fourslash/importNameCodeFixExistingImport14.ts
 create mode 100644 tests/cases/fourslash/importNameCodeFixExistingImport15.ts
 create mode 100644 tests/cases/fourslash/importNameCodeFixExistingImport16.ts
 create mode 100644 tests/cases/fourslash/importNameCodeFixExistingImport17.ts
 create mode 100644 tests/cases/fourslash/importNameCodeFixExistingImport18.ts

diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
index bc5bc008568e2..8c4b077dfff96 100644
--- a/src/services/codefixes/importFixes.ts
+++ b/src/services/codefixes/importFixes.ts
@@ -599,9 +599,9 @@ namespace ts.codefix {
                 const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName));
                 if (namedBindings && namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length !== 0) {
                     // There are already named imports; add another.
-                    return ChangeTracker.with(context, t => t.insertNodeInListAfter(
+                    return ChangeTracker.with(context, t => t.insertNodeInListAlphabetically(
                         sourceFile,
-                        namedBindings.elements[namedBindings.elements.length - 1],
+                        namedBindings.elements,
                         newImportSpecifier));
                 }
                 if (!namedBindings || namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length === 0) {
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index fd3860d0aeae6..c27926dd845d0 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -501,6 +501,58 @@ namespace ts.textChanges {
             });
         }
 
+        /**
+         * This function should be used to insert nodes alphabetically in lists when nodes don't carry separators as the part of the node range,
+         * e.g. export lists, import lists, etc.
+         * Linear search is used instead of binary as the (generally few) nodes are not guaranteed to be in order.
+         */
+        public insertNodeInListAlphabetically(sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier): this {
+            if (newNode.name.text < containingList[0].name.text) {
+                this.insertNodeInListBeforeFirst(sourceFile, containingList, containingList[0], newNode);
+                return this;
+            }
+
+            for (let i = 1; i < containingList.length; i += 1) {
+                if (newNode.name.text < containingList[i].name.text) {
+                    this.insertNodeInListAfter(sourceFile, containingList[i - 1], newNode);
+                    return this;
+                }
+            }
+
+            this.insertNodeInListAfter(sourceFile, containingList[containingList.length - 1], newNode);
+        }
+
+        private insertNodeInListBeforeFirst(sourceFile: SourceFile, containingList: NodeArray<Node>, first: Node, newNode: Node): void {
+            const startPosition = first.getStart(sourceFile);
+            const afterStartLinePosition = getLineStartPositionForPosition(startPosition, sourceFile);
+            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, 0, first);
+
+            if (!multilineList) {
+                this.changes.push({
+                    kind: ChangeKind.ReplaceWithSingleNode,
+                    sourceFile,
+                    range: { pos: startPosition, end: startPosition },
+                    node: newNode,
+                    options: { suffix: `${tokenToString(separator)} ` }
+                });
+                return;
+            }
+
+            const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(startPosition, afterStartLinePosition, sourceFile, this.formatContext.options);
+            let insertPos = skipTrivia(sourceFile.text, startPosition, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false);
+            if (insertPos !== startPosition && isLineBreak(sourceFile.text.charCodeAt(insertPos - 1))) {
+                insertPos--;
+            }
+
+            this.changes.push({
+                kind: ChangeKind.ReplaceWithSingleNode,
+                sourceFile,
+                range: { pos: insertPos, end: insertPos },
+                node: newNode,
+                options: { indentation, prefix: this.newLineCharacter }
+            });
+        }
+
         private insertNodeInListAfterLast(sourceFile: SourceFile, containingList: NodeArray<Node>, after: Node, index: number, newNode: Node): void {
             const end = after.getEnd();
             const afterStart = after.getStart(sourceFile);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport0.ts b/tests/cases/fourslash/importNameCodeFixExistingImport0.ts
index e5b634998783c..922285e9afca3 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport0.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport0.ts
@@ -7,4 +7,4 @@
 //// export function f1() {}
 //// export var v1 = 5;
 
-verify.importFixAtPosition([`{ v1, f1 }`]);
+verify.importFixAtPosition([`{ f1, v1 }`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport1.ts b/tests/cases/fourslash/importNameCodeFixExistingImport1.ts
index 49680692b6c58..f06ce49f0ea6b 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport1.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport1.ts
@@ -8,4 +8,4 @@
 //// export var v1 = 5;
 //// export default var d1 = 6;
 
-verify.importFixAtPosition([`{ v1, f1 }`]);
+verify.importFixAtPosition([`{ f1, v1 }`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport10.ts b/tests/cases/fourslash/importNameCodeFixExistingImport10.ts
index a9525d272da00..3faa34deaa916 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport10.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport10.ts
@@ -14,8 +14,8 @@
 
 verify.importFixAtPosition([
 `{
+    f1,
     v1,
-    v2,
-    f1
+    v2
 }`
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport11.ts b/tests/cases/fourslash/importNameCodeFixExistingImport11.ts
index 786e0e397756e..4a414ef9a36de 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport11.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport11.ts
@@ -14,8 +14,8 @@
 
 verify.importFixAtPosition([
 `{
+    f1,
     v1, v2,
-    v3,
-    f1
+    v3
 }`
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport12.ts b/tests/cases/fourslash/importNameCodeFixExistingImport12.ts
index 87ba29ab0aa21..0acdbda00b56a 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport12.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport12.ts
@@ -1,6 +1,6 @@
 /// <reference path="fourslash.ts" />
 
-//// import [|{}|] from "./module";
+//// import [|{v1, v2, v3,}|] from "./module";
 //// f1/*0*/();
 
 // @Filename: module.ts
@@ -9,4 +9,4 @@
 //// export var v2 = 5;
 //// export var v3 = 5;
 
-verify.importFixAtPosition([`{ f1 }`]);
+verify.importFixAtPosition([`{f1, v1, v2, v3}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport13.ts b/tests/cases/fourslash/importNameCodeFixExistingImport13.ts
new file mode 100644
index 0000000000000..9500242b3084d
--- /dev/null
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport13.ts
@@ -0,0 +1,12 @@
+/// <reference path="fourslash.ts" />
+
+//// import [|{v1, v3, v4,}|] from "./module";
+//// v2/*0*/();
+
+// @Filename: module.ts
+//// export var v1 = 5;
+//// export function v2() {}
+//// export var v3 = 5;
+//// export var v4 = 5;
+
+verify.importFixAtPosition([`{v1, v2, v3, v4}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport14.ts b/tests/cases/fourslash/importNameCodeFixExistingImport14.ts
new file mode 100644
index 0000000000000..3e6b557830430
--- /dev/null
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport14.ts
@@ -0,0 +1,20 @@
+/// <reference path="fourslash.ts" />
+
+//// import [|{
+////     v1,
+////     v3
+//// }|] from "./module";
+//// v2/*0*/();
+
+// @Filename: module.ts
+//// export function v2() {}
+//// export var v1 = 5;
+//// export var v3 = 5;
+
+verify.importFixAtPosition([
+`{
+    v1,
+    v2,
+    v3
+}`
+]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport15.ts b/tests/cases/fourslash/importNameCodeFixExistingImport15.ts
new file mode 100644
index 0000000000000..5ce9c06f4b8ca
--- /dev/null
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport15.ts
@@ -0,0 +1,20 @@
+/// <reference path="fourslash.ts" />
+
+////import [|{
+////    v1, v2,
+////    v4
+////}|] from "./module";
+////v3/*0*/();
+
+// @Filename: module.ts
+//// export function v3() {}
+//// export var v1 = 5;
+//// export var v2 = 5;
+//// export var v4 = 5;
+
+verify.importFixAtPosition([
+`{
+    v1, v2, v3,
+    v4
+}`
+]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport16.ts b/tests/cases/fourslash/importNameCodeFixExistingImport16.ts
new file mode 100644
index 0000000000000..cfbc79e4deb4b
--- /dev/null
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport16.ts
@@ -0,0 +1,12 @@
+/// <reference path="fourslash.ts" />
+
+//// import [|{v1, v2, v3,}|] from "./module";
+//// v4/*0*/();
+
+// @Filename: module.ts
+//// export function v4() {}
+//// export var v1 = 5;
+//// export var v2 = 5;
+//// export var v3 = 5;
+
+verify.importFixAtPosition([`{v1, v2, v3, v4}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport17.ts b/tests/cases/fourslash/importNameCodeFixExistingImport17.ts
new file mode 100644
index 0000000000000..d0bfcfe106467
--- /dev/null
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport17.ts
@@ -0,0 +1,20 @@
+/// <reference path="fourslash.ts" />
+
+//// import [|{
+////     v1,
+////     v2
+//// }|] from "./module";
+//// v3/*0*/();
+
+// @Filename: module.ts
+//// export function v3() {}
+//// export var v1 = 5;
+//// export var v2 = 5;
+
+verify.importFixAtPosition([
+`{
+    v1,
+    v2,
+    v3
+}`
+]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport18.ts b/tests/cases/fourslash/importNameCodeFixExistingImport18.ts
new file mode 100644
index 0000000000000..60ba577ecde90
--- /dev/null
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport18.ts
@@ -0,0 +1,21 @@
+/// <reference path="fourslash.ts" />
+
+////import [|{
+////    v1, v2,
+////    v3
+////}|] from "./module";
+////v4/*0*/();
+
+// @Filename: module.ts
+//// export function v4() {}
+//// export var v1 = 5;
+//// export var v2 = 5;
+//// export var v3 = 5;
+
+verify.importFixAtPosition([
+`{
+    v1, v2,
+    v3,
+    v4
+}`
+]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport6.ts b/tests/cases/fourslash/importNameCodeFixExistingImport6.ts
index 0d3636e443955..7b2bd7d5383f5 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport6.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport6.ts
@@ -10,4 +10,4 @@
 //// export var v1 = 5;
 //// export function f1();
 
-verify.importFixAtPosition([`{ v1, f1 }`]);
+verify.importFixAtPosition([`{ f1, v1 }`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport7.ts b/tests/cases/fourslash/importNameCodeFixExistingImport7.ts
index f0b1a77f69012..8a8ceb926c230 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport7.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport7.ts
@@ -7,4 +7,4 @@
 //// export var v1 = 5;
 //// export function f1();
 
-verify.importFixAtPosition([`{ v1, f1 }`]);
+verify.importFixAtPosition([`{ f1, v1 }`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport8.ts b/tests/cases/fourslash/importNameCodeFixExistingImport8.ts
index a1b676a8b8309..0acdbda00b56a 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport8.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport8.ts
@@ -9,4 +9,4 @@
 //// export var v2 = 5;
 //// export var v3 = 5;
 
-verify.importFixAtPosition([`{v1, v2, v3, f1,}`]);
+verify.importFixAtPosition([`{f1, v1, v2, v3}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport9.ts b/tests/cases/fourslash/importNameCodeFixExistingImport9.ts
index 67960de32d4c2..bf7709e12d954 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport9.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport9.ts
@@ -11,6 +11,6 @@
 
 verify.importFixAtPosition([
 `{
-    v1, f1
+    f1, v1
 }`
 ]);

From 6f3dae936f2580e09952f02dd4390e15f8e261ec Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sun, 11 Feb 2018 19:45:27 -0800
Subject: [PATCH 5/9] Fixed some bugs in alphabetical insertion

* Mismatched test case commas in 8, 12, 13, 16
* Mismatched test case endlines in 11, 15
* Was using `sampleIndex - 1` but passing `0` as that index for the before-first insertion. Switched to just using `sampleIndex`.
* Used last node instead of end of the first node for the ending range when determining line separator style for inserting after the first.
* Inserting before the first in a multiline list needs to insert at position - indentation

Also forgot to `return this;` at the end.
---
 src/services/textChanges.ts                     | 17 ++++++++---------
 .../importNameCodeFixExistingImport10.ts        |  1 -
 .../importNameCodeFixExistingImport11.ts        |  3 +--
 .../importNameCodeFixExistingImport12.ts        |  2 +-
 .../importNameCodeFixExistingImport13.ts        |  2 +-
 .../importNameCodeFixExistingImport15.ts        |  3 ++-
 .../importNameCodeFixExistingImport16.ts        |  2 +-
 .../importNameCodeFixExistingImport8.ts         |  2 +-
 8 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index c27926dd845d0..26ba14982945e 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -520,12 +520,13 @@ namespace ts.textChanges {
             }
 
             this.insertNodeInListAfter(sourceFile, containingList[containingList.length - 1], newNode);
+            return this;
         }
 
         private insertNodeInListBeforeFirst(sourceFile: SourceFile, containingList: NodeArray<Node>, first: Node, newNode: Node): void {
             const startPosition = first.getStart(sourceFile);
             const afterStartLinePosition = getLineStartPositionForPosition(startPosition, sourceFile);
-            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, 0, first);
+            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, 1, first);
 
             if (!multilineList) {
                 this.changes.push({
@@ -538,18 +539,16 @@ namespace ts.textChanges {
                 return;
             }
 
-            const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(startPosition, afterStartLinePosition, sourceFile, this.formatContext.options);
-            let insertPos = skipTrivia(sourceFile.text, startPosition, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false);
-            if (insertPos !== startPosition && isLineBreak(sourceFile.text.charCodeAt(insertPos - 1))) {
-                insertPos--;
-            }
+            const last = containingList[containingList.length - 1];
+            const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(afterStartLinePosition, last.end, sourceFile, this.formatContext.options);
+            const insertPos = skipTrivia(sourceFile.text, startPosition, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false) - indentation;
 
             this.changes.push({
                 kind: ChangeKind.ReplaceWithSingleNode,
                 sourceFile,
                 range: { pos: insertPos, end: insertPos },
                 node: newNode,
-                options: { indentation, prefix: this.newLineCharacter }
+                options: { indentation, suffix: `${tokenToString(separator)}${this.newLineCharacter}` }
             });
         }
 
@@ -558,7 +557,7 @@ namespace ts.textChanges {
             const afterStart = after.getStart(sourceFile);
             const afterStartLinePosition = getLineStartPositionForPosition(afterStart, sourceFile);
 
-            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, index, after);
+            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, index - 1, after);
 
             if (multilineList) {
                 // insert separator immediately following the 'after' node to preserve comments in trailing trivia
@@ -614,7 +613,7 @@ namespace ts.textChanges {
                 const tokenBeforeInsertPosition = findPrecedingToken(sampleNode.pos, sourceFile);
                 separator = isSeparator(sampleNode, tokenBeforeInsertPosition) ? tokenBeforeInsertPosition.kind : SyntaxKind.CommaToken;
                 // determine if list is multiline by checking lines of after element and element that precedes it.
-                const afterMinusOneStartLinePosition = getLineStartPositionForPosition(containingList[sampleIndex - 1].getStart(sourceFile), sourceFile);
+                const afterMinusOneStartLinePosition = getLineStartPositionForPosition(containingList[sampleIndex].getStart(sourceFile), sourceFile);
                 multilineList = afterMinusOneStartLinePosition !== afterStartLinePosition;
             }
             if (hasCommentsBeforeLineBreak(sourceFile.text, sampleNode.end)) {
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport10.ts b/tests/cases/fourslash/importNameCodeFixExistingImport10.ts
index 3faa34deaa916..d29fd9e5682de 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport10.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport10.ts
@@ -10,7 +10,6 @@
 //// export function f1() {}
 //// export var v1 = 5;
 //// export var v2 = 5;
-//// export var v3 = 5;
 
 verify.importFixAtPosition([
 `{
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport11.ts b/tests/cases/fourslash/importNameCodeFixExistingImport11.ts
index 4a414ef9a36de..878e253680911 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport11.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport11.ts
@@ -14,8 +14,7 @@
 
 verify.importFixAtPosition([
 `{
-    f1,
-    v1, v2,
+    f1, v1, v2,
     v3
 }`
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport12.ts b/tests/cases/fourslash/importNameCodeFixExistingImport12.ts
index 0acdbda00b56a..39a2ca0d59a11 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport12.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport12.ts
@@ -9,4 +9,4 @@
 //// export var v2 = 5;
 //// export var v3 = 5;
 
-verify.importFixAtPosition([`{f1, v1, v2, v3}`]);
+verify.importFixAtPosition([`{f1, v1, v2, v3,}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport13.ts b/tests/cases/fourslash/importNameCodeFixExistingImport13.ts
index 9500242b3084d..c9d242ffdc7e0 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport13.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport13.ts
@@ -9,4 +9,4 @@
 //// export var v3 = 5;
 //// export var v4 = 5;
 
-verify.importFixAtPosition([`{v1, v2, v3, v4}`]);
+verify.importFixAtPosition([`{v1, v2, v3, v4,}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport15.ts b/tests/cases/fourslash/importNameCodeFixExistingImport15.ts
index 5ce9c06f4b8ca..1788831aebd2d 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport15.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport15.ts
@@ -14,7 +14,8 @@
 
 verify.importFixAtPosition([
 `{
-    v1, v2, v3,
+    v1, v2,
+    v3,
     v4
 }`
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport16.ts b/tests/cases/fourslash/importNameCodeFixExistingImport16.ts
index cfbc79e4deb4b..a365136607d55 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport16.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport16.ts
@@ -9,4 +9,4 @@
 //// export var v2 = 5;
 //// export var v3 = 5;
 
-verify.importFixAtPosition([`{v1, v2, v3, v4}`]);
+verify.importFixAtPosition([`{v1, v2, v3, v4,}`]);
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport8.ts b/tests/cases/fourslash/importNameCodeFixExistingImport8.ts
index 0acdbda00b56a..39a2ca0d59a11 100644
--- a/tests/cases/fourslash/importNameCodeFixExistingImport8.ts
+++ b/tests/cases/fourslash/importNameCodeFixExistingImport8.ts
@@ -9,4 +9,4 @@
 //// export var v2 = 5;
 //// export var v3 = 5;
 
-verify.importFixAtPosition([`{f1, v1, v2, v3}`]);
+verify.importFixAtPosition([`{f1, v1, v2, v3,}`]);

From 372b73e925826a13a2a17aefa62a41536aae1306 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sun, 11 Feb 2018 19:52:31 -0800
Subject: [PATCH 6/9] Fixed remaining test cases :)

---
 .../fourslash/completionsImport_named_addToNamedImports.ts      | 2 +-
 .../fourslash/completionsImport_named_didNotExistBefore.ts      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts b/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts
index fa1be5ca7d2a5..1883a43922001 100644
--- a/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts
+++ b/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts
@@ -18,6 +18,6 @@ verify.applyCodeActionFromCompletion("", {
     name: "foo",
     source: "/a",
     description: `Add 'foo' to existing import declaration from "./a"`,
-    newFileContent: `import { x, foo } from "./a";
+    newFileContent: `import { foo, x } from "./a";
 f;`,
 });
diff --git a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts
index a69c0ab19ca86..e44a42c3d51b5 100644
--- a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts
+++ b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts
@@ -18,6 +18,6 @@ verify.applyCodeActionFromCompletion("", {
     name: "Test1",
     source: "/a",
     description: `Add 'Test1' to existing import declaration from "./a"`,
-    newFileContent: `import { Test2, Test1 } from "./a";
+    newFileContent: `import { Test1, Test2 } from "./a";
 t`,
 });

From b8d284e63e647504cbb8c7a641a62cde0770a055 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sun, 11 Feb 2018 21:41:06 -0800
Subject: [PATCH 7/9] Reordered textChanges insertion methods

Public first, then private.
---
 src/services/textChanges.ts | 159 ++++++++++++++++++------------------
 1 file changed, 79 insertions(+), 80 deletions(-)

diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index 26ba14982945e..6e8a9a28c873e 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -442,6 +442,85 @@ namespace ts.textChanges {
             }
             throw Debug.failBadSyntaxKind(node); // We haven't handled this kind of node yet -- add it
         }
+        /**
+         * This function should be used to insert nodes alphabetically in lists when nodes don't carry separators as the part of the node range,
+         * e.g. export lists, import lists, etc.
+         * Linear search is used instead of binary as the (generally few) nodes are not guaranteed to be in order.
+         */
+        public insertNodeInListAlphabetically(sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier): this {
+            if (newNode.name.text < containingList[0].name.text) {
+                this.insertNodeInListBeforeFirst(sourceFile, containingList, containingList[0], newNode);
+                return this;
+            }
+
+            for (let i = 1; i < containingList.length; i += 1) {
+                if (newNode.name.text < containingList[i].name.text) {
+                    this.insertNodeInListAfter(sourceFile, containingList[i - 1], newNode);
+                    return this;
+                }
+            }
+
+            this.insertNodeInListAfter(sourceFile, containingList[containingList.length - 1], newNode);
+            return this;
+        }
+
+        /**
+         * This function should be used to insert nodes in lists when nodes don't carry separators as the part of the node range,
+         * i.e. arguments in arguments lists, parameters in parameter lists etc.
+         * Note that separators are part of the node in statements and class elements.
+         */
+        public insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node) {
+            const containingList = formatting.SmartIndenter.getContainingList(after, sourceFile);
+            if (!containingList) {
+                Debug.fail("node is not a list element");
+                return this;
+            }
+            const index = indexOfNode(containingList, after);
+            if (index < 0) {
+                return this;
+            }
+            if (index !== containingList.length - 1) {
+                // any element except the last one
+                // use next sibling as an anchor
+                const nextToken = getTokenAtPosition(sourceFile, after.end, /*includeJsDocComment*/ false);
+                if (nextToken && isSeparator(after, nextToken)) {
+                    this.insertNodeInListAfterNotLast(sourceFile, containingList, nextToken, index, newNode);
+                }
+            }
+            else {
+                this.insertNodeInListAfterLast(sourceFile, containingList, after, index, newNode);
+            }
+            return this;
+        }
+
+        private insertNodeInListBeforeFirst(sourceFile: SourceFile, containingList: NodeArray<Node>, first: Node, newNode: Node): void {
+            const startPosition = first.getStart(sourceFile);
+            const afterStartLinePosition = getLineStartPositionForPosition(startPosition, sourceFile);
+            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, 1, first);
+
+            if (!multilineList) {
+                this.changes.push({
+                    kind: ChangeKind.ReplaceWithSingleNode,
+                    sourceFile,
+                    range: { pos: startPosition, end: startPosition },
+                    node: newNode,
+                    options: { suffix: `${tokenToString(separator)} ` }
+                });
+                return;
+            }
+
+            const last = containingList[containingList.length - 1];
+            const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(afterStartLinePosition, last.end, sourceFile, this.formatContext.options);
+            const insertPos = skipTrivia(sourceFile.text, startPosition, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false) - indentation;
+
+            this.changes.push({
+                kind: ChangeKind.ReplaceWithSingleNode,
+                sourceFile,
+                range: { pos: insertPos, end: insertPos },
+                node: newNode,
+                options: { indentation, suffix: `${tokenToString(separator)}${this.newLineCharacter}` }
+            });
+        }
 
         private insertNodeInListAfterNotLast(sourceFile: SourceFile, containingList: NodeArray<Node>, nextToken: Node, index: number, newNode: Node): void {
             // for list
@@ -501,57 +580,6 @@ namespace ts.textChanges {
             });
         }
 
-        /**
-         * This function should be used to insert nodes alphabetically in lists when nodes don't carry separators as the part of the node range,
-         * e.g. export lists, import lists, etc.
-         * Linear search is used instead of binary as the (generally few) nodes are not guaranteed to be in order.
-         */
-        public insertNodeInListAlphabetically(sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier): this {
-            if (newNode.name.text < containingList[0].name.text) {
-                this.insertNodeInListBeforeFirst(sourceFile, containingList, containingList[0], newNode);
-                return this;
-            }
-
-            for (let i = 1; i < containingList.length; i += 1) {
-                if (newNode.name.text < containingList[i].name.text) {
-                    this.insertNodeInListAfter(sourceFile, containingList[i - 1], newNode);
-                    return this;
-                }
-            }
-
-            this.insertNodeInListAfter(sourceFile, containingList[containingList.length - 1], newNode);
-            return this;
-        }
-
-        private insertNodeInListBeforeFirst(sourceFile: SourceFile, containingList: NodeArray<Node>, first: Node, newNode: Node): void {
-            const startPosition = first.getStart(sourceFile);
-            const afterStartLinePosition = getLineStartPositionForPosition(startPosition, sourceFile);
-            const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, 1, first);
-
-            if (!multilineList) {
-                this.changes.push({
-                    kind: ChangeKind.ReplaceWithSingleNode,
-                    sourceFile,
-                    range: { pos: startPosition, end: startPosition },
-                    node: newNode,
-                    options: { suffix: `${tokenToString(separator)} ` }
-                });
-                return;
-            }
-
-            const last = containingList[containingList.length - 1];
-            const indentation = formatting.SmartIndenter.findFirstNonWhitespaceColumn(afterStartLinePosition, last.end, sourceFile, this.formatContext.options);
-            const insertPos = skipTrivia(sourceFile.text, startPosition, /*stopAfterLineBreak*/ true, /*stopAtComments*/ false) - indentation;
-
-            this.changes.push({
-                kind: ChangeKind.ReplaceWithSingleNode,
-                sourceFile,
-                range: { pos: insertPos, end: insertPos },
-                node: newNode,
-                options: { indentation, suffix: `${tokenToString(separator)}${this.newLineCharacter}` }
-            });
-        }
-
         private insertNodeInListAfterLast(sourceFile: SourceFile, containingList: NodeArray<Node>, after: Node, index: number, newNode: Node): void {
             const end = after.getEnd();
             const afterStart = after.getStart(sourceFile);
@@ -624,35 +652,6 @@ namespace ts.textChanges {
             return { multilineList, separator };
         }
 
-        /**
-         * This function should be used to insert nodes in lists when nodes don't carry separators as the part of the node range,
-         * i.e. arguments in arguments lists, parameters in parameter lists etc.
-         * Note that separators are part of the node in statements and class elements.
-         */
-        public insertNodeInListAfter(sourceFile: SourceFile, after: Node, newNode: Node) {
-            const containingList = formatting.SmartIndenter.getContainingList(after, sourceFile);
-            if (!containingList) {
-                Debug.fail("node is not a list element");
-                return this;
-            }
-            const index = indexOfNode(containingList, after);
-            if (index < 0) {
-                return this;
-            }
-            if (index !== containingList.length - 1) {
-                // any element except the last one
-                // use next sibling as an anchor
-                const nextToken = getTokenAtPosition(sourceFile, after.end, /*includeJsDocComment*/ false);
-                if (nextToken && isSeparator(after, nextToken)) {
-                    this.insertNodeInListAfterNotLast(sourceFile, containingList, nextToken, index, newNode);
-                }
-            }
-            else {
-                this.insertNodeInListAfterLast(sourceFile, containingList, after, index, newNode);
-            }
-            return this;
-        }
-
         private finishInsertNodeAtClassStart(): void {
             this.nodesInsertedAtClassStarts.forEach(({ sourceFile, cls, members }) => {
                 const newCls = cls.kind === SyntaxKind.ClassDeclaration

From ed400e33f429c4480dce18fa9f303febff2e4565 Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sat, 3 Mar 2018 17:48:01 -0800
Subject: [PATCH 8/9] Feedback: compareStringsCaseInsensitive; function name

---
 src/services/codefixes/importFixes.ts | 2 +-
 src/services/textChanges.ts           | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
index ca9e7078b8caa..275967965aeba 100644
--- a/src/services/codefixes/importFixes.ts
+++ b/src/services/codefixes/importFixes.ts
@@ -599,7 +599,7 @@ namespace ts.codefix {
                 const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName));
                 if (namedBindings && namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length !== 0) {
                     // There are already named imports; add another.
-                    return ChangeTracker.with(context, t => t.insertNodeInListAlphabetically(
+                    return ChangeTracker.with(context, t => t.insertNodeInsideListNodesAlphabetically(
                         sourceFile,
                         namedBindings.elements,
                         newImportSpecifier));
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index f82131a4cf7e1..a3485239cb4c5 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -432,19 +432,20 @@ namespace ts.textChanges {
             }
             return Debug.failBadSyntaxKind(node); // We haven't handled this kind of node yet -- add it
         }
+
         /**
          * This function should be used to insert nodes alphabetically in lists when nodes don't carry separators as the part of the node range,
          * e.g. export lists, import lists, etc.
          * Linear search is used instead of binary as the (generally few) nodes are not guaranteed to be in order.
          */
-        public insertNodeInListAlphabetically(sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier): this {
-            if (newNode.name.text < containingList[0].name.text) {
+        public insertNodeInsideListNodesAlphabetically(sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier): this {
+            if (compareStringsCaseInsensitive(newNode.name.text, containingList[0].name.text) === Comparison.LessThan) {
                 this.insertNodeInListBeforeFirst(sourceFile, containingList, containingList[0], newNode);
                 return this;
             }
 
             for (let i = 1; i < containingList.length; i += 1) {
-                if (newNode.name.text < containingList[i].name.text) {
+                if (compareStringsCaseInsensitive(newNode.name.text, containingList[i].name.text) === Comparison.LessThan) {
                     this.insertNodeInListAfter(sourceFile, containingList[i - 1], newNode);
                     return this;
                 }

From c20b81da45502000f87c2c680d1cb55871742fcc Mon Sep 17 00:00:00 2001
From: Josh Goldberg <josh@fullscreenmario.com>
Date: Sat, 3 Mar 2018 19:59:16 -0800
Subject: [PATCH 9/9] Moved insertNodeInsideListNodesAlphabetically to
 importFixes

---
 src/services/codefixes/importFixes.ts | 26 ++++++++++++++++++++++----
 src/services/textChanges.ts           | 24 +-----------------------
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
index 275967965aeba..c86f61f2f8f18 100644
--- a/src/services/codefixes/importFixes.ts
+++ b/src/services/codefixes/importFixes.ts
@@ -599,10 +599,7 @@ namespace ts.codefix {
                 const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName));
                 if (namedBindings && namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length !== 0) {
                     // There are already named imports; add another.
-                    return ChangeTracker.with(context, t => t.insertNodeInsideListNodesAlphabetically(
-                        sourceFile,
-                        namedBindings.elements,
-                        newImportSpecifier));
+                    return insertNodeInsideListNodesAlphabetically(context, sourceFile, namedBindings.elements, newImportSpecifier);
                 }
                 if (!namedBindings || namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length === 0) {
                     return ChangeTracker.with(context, t =>
@@ -623,6 +620,27 @@ namespace ts.codefix {
         }
     }
 
+    /**
+     * This function should be used to insert nodes alphabetically in lists when nodes don't carry separators as the part of the node range,
+     * e.g. export lists, import lists, etc.
+     * Linear search is used instead of binary as the (generally few) nodes are not guaranteed to be in order.
+     */
+    function insertNodeInsideListNodesAlphabetically(context: SymbolContext, sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier) {
+        return ChangeTracker.with(context, t => {
+            if (compareStringsCaseInsensitive(newNode.name.text, containingList[0].name.text) === Comparison.LessThan) {
+                return t.insertNodeInListBeforeFirst(sourceFile, containingList, containingList[0], newNode);
+            }
+
+            for (let i = 1; i < containingList.length; i += 1) {
+                if (compareStringsCaseInsensitive(newNode.name.text, containingList[i].name.text) === Comparison.LessThan) {
+                    return t.insertNodeInListAfter(sourceFile, containingList[i - 1], newNode);
+                }
+            }
+
+            return t.insertNodeInListAfter(sourceFile, containingList[containingList.length - 1], newNode);
+        });
+    }
+
     function getCodeActionForUseExistingNamespaceImport(namespacePrefix: string, context: SymbolContext, symbolToken: Identifier): CodeFixAction {
         const { symbolName, sourceFile } = context;
 
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
index a3485239cb4c5..1001e5957a37d 100644
--- a/src/services/textChanges.ts
+++ b/src/services/textChanges.ts
@@ -433,28 +433,6 @@ namespace ts.textChanges {
             return Debug.failBadSyntaxKind(node); // We haven't handled this kind of node yet -- add it
         }
 
-        /**
-         * This function should be used to insert nodes alphabetically in lists when nodes don't carry separators as the part of the node range,
-         * e.g. export lists, import lists, etc.
-         * Linear search is used instead of binary as the (generally few) nodes are not guaranteed to be in order.
-         */
-        public insertNodeInsideListNodesAlphabetically(sourceFile: SourceFile, containingList: NodeArray<ImportSpecifier>, newNode: ImportSpecifier): this {
-            if (compareStringsCaseInsensitive(newNode.name.text, containingList[0].name.text) === Comparison.LessThan) {
-                this.insertNodeInListBeforeFirst(sourceFile, containingList, containingList[0], newNode);
-                return this;
-            }
-
-            for (let i = 1; i < containingList.length; i += 1) {
-                if (compareStringsCaseInsensitive(newNode.name.text, containingList[i].name.text) === Comparison.LessThan) {
-                    this.insertNodeInListAfter(sourceFile, containingList[i - 1], newNode);
-                    return this;
-                }
-            }
-
-            this.insertNodeInListAfter(sourceFile, containingList[containingList.length - 1], newNode);
-            return this;
-        }
-
         /**
          * This function should be used to insert nodes in lists when nodes don't carry separators as the part of the node range,
          * i.e. arguments in arguments lists, parameters in parameter lists etc.
@@ -484,7 +462,7 @@ namespace ts.textChanges {
             return this;
         }
 
-        private insertNodeInListBeforeFirst(sourceFile: SourceFile, containingList: NodeArray<Node>, first: Node, newNode: Node): void {
+        public insertNodeInListBeforeFirst(sourceFile: SourceFile, containingList: NodeArray<Node>, first: Node, newNode: Node): void {
             const startPosition = first.getStart(sourceFile);
             const afterStartLinePosition = getLineStartPositionForPosition(startPosition, sourceFile);
             const { multilineList, separator } = this.getMultilineAndSeparatorOfList(sourceFile, containingList, afterStartLinePosition, 1, first);