Fixup line and offset of rename location of refactor (microsoft#19265)

Andy · web-flow · commit c1b4d59752bf · 2017-10-19T14:31:23.000-07:00
* Fixup line and offset of rename location of refactor

* Fixes

* Handle "\r" only documents

* Update api baselines

* Fix error if an edit comes *after* the rename location

* Add bounds check

* Simpler implementation: get new text, then calculate line starts the usual way
diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts
@@ -351,7 +351,7 @@ namespace ts {
     /**
      * We assume the first line starts at position 0 and 'position' is non-negative.
      */
-    export function computeLineAndCharacterOfPosition(lineStarts: ReadonlyArray<number>, position: number) {
+    export function computeLineAndCharacterOfPosition(lineStarts: ReadonlyArray<number>, position: number): LineAndCharacter {
         let lineNumber = binarySearch(lineStarts, position);
         if (lineNumber < 0) {
             // If the actual position was not found,
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -3811,9 +3811,10 @@ namespace ts {
     }
 
     export interface LineAndCharacter {
+        /** 0-based. */
         line: number;
         /*
-         * This value denotes the character position in line and is different from the 'column' because of tab characters.
+         * 0-based. This value denotes the character position in line and is different from the 'column' because of tab characters.
          */
         character: number;
     }
diff --git a/src/harness/unittests/session.ts b/src/harness/unittests/session.ts
@@ -661,4 +661,28 @@ namespace ts.server {
             session.consumeQueue();
         });
     });
+
+    describe("helpers", () => {
+        it(getLocationInNewDocument.name, () => {
+            const text = `// blank line\nconst x = 0;`;
+            const renameLocationInOldText = text.indexOf("0");
+            const fileName = "/a.ts";
+            const edits: ts.FileTextChanges = {
+                fileName,
+                textChanges: [
+                    {
+                        span: { start: 0, length: 0 },
+                        newText: "const newLocal = 0;\n\n",
+                    },
+                    {
+                        span: { start: renameLocationInOldText, length: 1 },
+                        newText: "newLocal",
+                    },
+                ],
+            };
+            const renameLocationInNewText = renameLocationInOldText + edits.textChanges[0].newText.length;
+            const res = getLocationInNewDocument(text, fileName, renameLocationInNewText, [edits]);
+            assert.deepEqual(res, { line: 4, offset: 11 });
+        });
+    });
 }
diff --git a/src/server/session.ts b/src/server/session.ts
@@ -1514,16 +1514,18 @@ namespace ts.server {
             }
 
             if (simplifiedResult) {
-                const file = result.renameFilename;
-                let location: protocol.Location | undefined;
-                if (file !== undefined && result.renameLocation !== undefined) {
-                    const renameScriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(file));
-                    location = renameScriptInfo.positionToLineOffset(result.renameLocation);
+                const { renameFilename, renameLocation, edits } = result;
+                let mappedRenameLocation: protocol.Location | undefined;
+                if (renameFilename !== undefined && renameLocation !== undefined) {
+                    const renameScriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(renameFilename));
+                    const snapshot = renameScriptInfo.getSnapshot();
+                    const oldText = snapshot.getText(0, snapshot.getLength());
+                    mappedRenameLocation = getLocationInNewDocument(oldText, renameFilename, renameLocation, edits);
                 }
                 return {
-                    renameLocation: location,
-                    renameFilename: file,
-                    edits: result.edits.map(change => this.mapTextChangesToCodeEdits(project, change))
+                    renameLocation: mappedRenameLocation,
+                    renameFilename,
+                    edits: edits.map(change => this.mapTextChangesToCodeEdits(project, change))
                 };
             }
             else {
@@ -2040,4 +2042,26 @@ namespace ts.server {
         response?: {};
         responseRequired?: boolean;
     }
+
+    /* @internal */ // Exported only for tests
+    export function getLocationInNewDocument(oldText: string, renameFilename: string, renameLocation: number, edits: ReadonlyArray<FileTextChanges>): protocol.Location {
+        const newText = applyEdits(oldText, renameFilename, edits);
+        const { line, character } = computeLineAndCharacterOfPosition(computeLineStarts(newText), renameLocation);
+        return { line: line + 1, offset: character + 1 };
+    }
+
+    function applyEdits(text: string, textFilename: string, edits: ReadonlyArray<FileTextChanges>): string {
+        for (const { fileName, textChanges } of edits) {
+            if (fileName !== textFilename) {
+                continue;
+            }
+
+            for (let i = textChanges.length - 1; i >= 0; i--) {
+                const { newText, span: { start, length } } = textChanges[i];
+                text = text.slice(0, start) + newText + text.slice(start + length);
+            }
+        }
+
+        return text;
+    }
 }
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -2307,6 +2307,7 @@ declare namespace ts {
         LineFeed = 1,
     }
     interface LineAndCharacter {
+        /** 0-based. */
         line: number;
         character: number;
     }
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -2307,6 +2307,7 @@ declare namespace ts {
         LineFeed = 1,
     }
     interface LineAndCharacter {
+        /** 0-based. */
         line: number;
         character: number;
     }

Original file line number	Diff line number	Diff line change
`@@ -3811,9 +3811,10 @@ namespace ts {`
`3811`	`3811`	`}`
`3812`	`3812`
`3813`	`3813`	`export interface LineAndCharacter {`
	`3814`	`+ /** 0-based. */`
`3814`	`3815`	`line: number;`
`3815`	`3816`	`/*`
`3816`		`- * This value denotes the character position in line and is different from the 'column' because of tab characters.`
	`3817`	`+ * 0-based. This value denotes the character position in line and is different from the 'column' because of tab characters.`
`3817`	`3818`	`*/`
`3818`	`3819`	`character: number;`
`3819`	`3820`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2307,6 +2307,7 @@ declare namespace ts {`
`2307`	`2307`	`LineFeed = 1,`
`2308`	`2308`	`}`
`2309`	`2309`	`interface LineAndCharacter {`
	`2310`	`+ /** 0-based. */`
`2310`	`2311`	`line: number;`
`2311`	`2312`	`character: number;`
`2312`	`2313`	`}`