Test for (and fix) order of import fixes (#21398)

Andy · web-flow · commit d333d889c1d4 · 2018-01-24T15:06:52.000-08:00
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -2572,12 +2572,10 @@ Actual: ${stringify(fullActual)}`);
                 actualTextArray.push(text);
                 scriptInfo.updateContent(originalContent);
             }
-            const sortedExpectedArray = expectedTextArray.sort();
-            const sortedActualArray = actualTextArray.sort();
-            if (sortedExpectedArray.length !== sortedActualArray.length) {
-                this.raiseError(`Expected ${sortedExpectedArray.length} import fixes, got ${sortedActualArray.length}`);
+            if (expectedTextArray.length !== actualTextArray.length) {
+                this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}`);
             }
-            ts.zipWith(sortedExpectedArray, sortedActualArray, (expected, actual, index) => {
+            ts.zipWith(expectedTextArray, actualTextArray, (expected, actual, index) => {
                 if (expected !== actual) {
                     this.raiseError(`Import fix at index ${index} doesn't match.\n${showTextDiff(expected, actual)}`);
                 }
diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
@@ -390,7 +390,7 @@ namespace ts.codefix {
                 In this case we should prefer using the relative path "../a" instead of the baseUrl path "foo/a".
                 */
                 const pathFromSourceToBaseUrl = getRelativePath(baseUrl, sourceDirectory, getCanonicalFileName);
-                const relativeFirst = getRelativePathNParents(pathFromSourceToBaseUrl) < getRelativePathNParents(relativePath);
+                const relativeFirst = getRelativePathNParents(relativePath) < getRelativePathNParents(pathFromSourceToBaseUrl);
                 return relativeFirst ? [relativePath, importRelativeToBaseUrl] : [importRelativeToBaseUrl, relativePath];
             }));
         // Only return results for the re-export with the shortest possible path (and also give the other path even if that's long.)
diff --git a/tests/cases/fourslash/completionsImportBaseUrl.ts b/tests/cases/fourslash/completionsImportBaseUrl.ts
@@ -0,0 +1,21 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: /tsconfig.json
+////{
+////    "compilerOptions": {
+////        "baseUrl": "."
+////    }
+////}
+
+// @Filename: /src/a.ts
+////export const foo = 0;
+
+// @Filename: /src/b.ts
+////fo/**/
+
+// Test that it prefers a relative import (see sourceDisplay).
+goTo.marker("");
+verify.completionListContains({ name: "foo", source: "/src/a" }, "const foo: 0", "", "const", undefined, /*hasAction*/ true, {
+    includeExternalModuleExports: true,
+    sourceDisplay: "./a",
+});
diff --git a/tests/cases/fourslash/importNameCodeFixExistingImport2.ts b/tests/cases/fourslash/importNameCodeFixExistingImport2.ts
@@ -9,8 +9,8 @@
 
 verify.importFixAtPosition([
 `import * as ns from "./module";
+ns.f1();`,
+`import * as ns from "./module";
 import { f1 } from "./module";
 f1();`,
-`import * as ns from "./module";
-ns.f1();`,
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixNewImportBaseUrl0.ts b/tests/cases/fourslash/importNameCodeFixNewImportBaseUrl0.ts
@@ -13,10 +13,10 @@
 //// export function f1() { };
 
 verify.importFixAtPosition([
-`import { f1 } from "./a/b";
+`import { f1 } from "b";
 
 f1();`,
-`import { f1 } from "b";
+`import { f1 } from "./a/b";
 
-f1();`
+f1();`,
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixOptionalImport0.ts b/tests/cases/fourslash/importNameCodeFixOptionalImport0.ts
@@ -12,9 +12,9 @@
 
 verify.importFixAtPosition([
 `import * as ns from "./foo";
-import { foo } from "./foo";
-foo();`,
+ns.foo();`,
 
 `import * as ns from "./foo";
-ns.foo();`,
+import { foo } from "./foo";
+foo();`,
 ]);
diff --git a/tests/cases/fourslash/importNameCodeFixOptionalImport1.ts b/tests/cases/fourslash/importNameCodeFixOptionalImport1.ts
@@ -7,14 +7,14 @@
 //// export function foo() {};
 
 // @Filename: a/foo.ts
-//// export { foo } from "bar"; 
+//// export { foo } from "bar";
 
 verify.importFixAtPosition([
-`import { foo } from "./foo";
+`import { foo } from "bar";
 
 foo();`,
 
-`import { foo } from "bar";
+`import { foo } from "./foo";
 
 foo();`,
 ]);

Original file line number	Diff line number	Diff line change
@@ -2572,12 +2572,10 @@ Actual: ${stringify(fullActual)}`);
`2572`	`2572`	`actualTextArray.push(text);`
`2573`	`2573`	`scriptInfo.updateContent(originalContent);`
`2574`	`2574`	`}`
`2575`		`- const sortedExpectedArray = expectedTextArray.sort();`
`2576`		`- const sortedActualArray = actualTextArray.sort();`
`2577`		`- if (sortedExpectedArray.length !== sortedActualArray.length) {`
`2578`		- this.raiseError(`Expected ${sortedExpectedArray.length} import fixes, got ${sortedActualArray.length}`);
	`2575`	`+ if (expectedTextArray.length !== actualTextArray.length) {`
	`2576`	+ this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}`);
`2579`	`2577`	`}`
`2580`		`- ts.zipWith(sortedExpectedArray, sortedActualArray, (expected, actual, index) => {`
	`2578`	`+ ts.zipWith(expectedTextArray, actualTextArray, (expected, actual, index) => {`
`2581`	`2579`	`if (expected !== actual) {`
`2582`	`2580`	this.raiseError(`Import fix at index ${index} doesn't match.\n${showTextDiff(expected, actual)}`);
`2583`	`2581`	`}`