Skip to content

Commit a048d67

Browse files
author
Andy Hanson
committed
Test for (and fix) order of import fixes
1 parent 9e7ff9f commit a048d67

8 files changed

+61
-17
lines changed

src/harness/fourslash.ts

+3-5
Original file line numberDiff line numberDiff line change
@@ -2587,12 +2587,10 @@ Actual: ${stringify(fullActual)}`);
25872587
actualTextArray.push(text);
25882588
scriptInfo.updateContent(originalContent);
25892589
}
2590-
const sortedExpectedArray = expectedTextArray.sort();
2591-
const sortedActualArray = actualTextArray.sort();
2592-
if (sortedExpectedArray.length !== sortedActualArray.length) {
2593-
this.raiseError(`Expected ${sortedExpectedArray.length} import fixes, got ${sortedActualArray.length}`);
2590+
if (expectedTextArray.length !== actualTextArray.length) {
2591+
this.raiseError(`Expected ${expectedTextArray.length} import fixes, got ${actualTextArray.length}`);
25942592
}
2595-
ts.zipWith(sortedExpectedArray, sortedActualArray, (expected, actual, index) => {
2593+
ts.zipWith(expectedTextArray, actualTextArray, (expected, actual, index) => {
25962594
if (expected !== actual) {
25972595
this.raiseError(`Import fix at index ${index} doesn't match.\n${showTextDiff(expected, actual)}`);
25982596
}

src/services/codefixes/importFixes.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ namespace ts.codefix {
390390
In this case we should prefer using the relative path "../a" instead of the baseUrl path "foo/a".
391391
*/
392392
const pathFromSourceToBaseUrl = getRelativePath(baseUrl, sourceDirectory, getCanonicalFileName);
393-
const relativeFirst = getRelativePathNParents(pathFromSourceToBaseUrl) < getRelativePathNParents(relativePath);
393+
const relativeFirst = getRelativePathNParents(relativePath) < getRelativePathNParents(pathFromSourceToBaseUrl);
394394
return relativeFirst ? [relativePath, importRelativeToBaseUrl] : [importRelativeToBaseUrl, relativePath];
395395
}));
396396
// Only return results for the re-export with the shortest possible path (and also give the other path even if that's long.)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
////{
5+
//// "compilerOptions": {
6+
//// "baseUrl": "."
7+
//// }
8+
////}
9+
10+
// @Filename: /src/a.ts
11+
////export const foo = 0;
12+
13+
// @Filename: /src/b.ts
14+
////fo/**/
15+
16+
// Test that it prefers a relative import (see sourceDisplay).
17+
goTo.marker("");
18+
verify.completionListContains({ name: "foo", source: "/src/a" }, "const foo: 0", "", "const", undefined, /*hasAction*/ true, {
19+
includeExternalModuleExports: true,
20+
sourceDisplay: "./a",
21+
});

tests/cases/fourslash/importNameCodeFixExistingImport2.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99

1010
verify.importFixAtPosition([
1111
`import * as ns from "./module";
12+
ns.f1();`,
13+
`import * as ns from "./module";
1214
import { f1 } from "./module";
1315
f1();`,
14-
`import * as ns from "./module";
15-
ns.f1();`,
1616
]);

tests/cases/fourslash/importNameCodeFixNewImportBaseUrl0.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
//// export function f1() { };
1414

1515
verify.importFixAtPosition([
16-
`import { f1 } from "./a/b";
16+
`import { f1 } from "b";
1717
1818
f1();`,
19-
`import { f1 } from "b";
19+
`import { f1 } from "./a/b";
2020
21-
f1();`
21+
f1();`,
2222
]);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
////{
5+
//// "compilerOptions": {
6+
//// "baseUrl": "."
7+
//// }
8+
////}
9+
10+
// @Filename: /src/a.ts
11+
////export const xxx = 0;
12+
13+
// @Filename: /src/b.ts
14+
////[|xxx;|]
15+
16+
goTo.file("/src/b.ts");
17+
// Order the local import first because it's simpler.
18+
verify.importFixAtPosition([
19+
`import { xxx } from "./a";
20+
21+
xxx;`,
22+
`import { xxx } from "src/a";
23+
24+
xxx;`
25+
]);

tests/cases/fourslash/importNameCodeFixOptionalImport0.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212

1313
verify.importFixAtPosition([
1414
`import * as ns from "./foo";
15-
import { foo } from "./foo";
16-
foo();`,
15+
ns.foo();`,
1716

1817
`import * as ns from "./foo";
19-
ns.foo();`,
18+
import { foo } from "./foo";
19+
foo();`,
2020
]);

tests/cases/fourslash/importNameCodeFixOptionalImport1.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77
//// export function foo() {};
88

99
// @Filename: a/foo.ts
10-
//// export { foo } from "bar";
10+
//// export { foo } from "bar";
1111

1212
verify.importFixAtPosition([
13-
`import { foo } from "./foo";
13+
`import { foo } from "bar";
1414
1515
foo();`,
1616

17-
`import { foo } from "bar";
17+
`import { foo } from "./foo";
1818
1919
foo();`,
2020
]);

0 commit comments

Comments
 (0)