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

Author: Andy

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)}`);
                 }

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.)

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",
+});

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();`,
 ]);

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();`,
 ]);

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();`,
 ]);

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();`,
 ]);