Skip to content

Commit 0b89ce4

Browse files
sheetalkamatsheetalkamat
committed
Fix rename marker and prefix, suffix locations when marker and rangeStart/end match
1 parent 600f1c2 commit 0b89ce4

File tree

34 files changed

+138
-83
lines changed

34 files changed

+138
-83
lines changed

‎src/harness/fourslashImpl.ts‎

Lines changed: 89 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,8 +1252,9 @@ export class TestState {
12521252
getDocumentSpanArray: (result: R) => readonly T[] | undefined,
12531253
skipJsonOrGetAdditionalBaseline?: true | ((result: R) => string),
12541254
documentSpanToPrefix?: (span: T) => string,
1255-
startMarker?: (span: T) => string,
1256-
endMarker?: (span: T) => string,
1255+
endMarker?: string,
1256+
startMarkerPrefix?: (span: T) => string | undefined,
1257+
endMarkerSuffix?: (span: T) => string | undefined,
12571258
ignoredDocumentSpanProperties?: readonly string[],
12581259
): string {
12591260
const spans = getDocumentSpanArray(result);
@@ -1266,8 +1267,9 @@ export class TestState {
12661267
// Skip additional marker file since its already added in prev baseline,
12671268
skipJsonOrGetAdditionalBaseline === true,
12681269
documentSpanToPrefix,
1269-
startMarker,
12701270
endMarker,
1271+
startMarkerPrefix,
1272+
endMarkerSuffix,
12711273
ignoredDocumentSpanProperties,
12721274
);
12731275
if (skipJsonOrGetAdditionalBaseline === true) return baselineContent;
@@ -1282,8 +1284,9 @@ export class TestState {
12821284
markerOrRange: MarkerOrNameOrRange | undefined,
12831285
skipMarkerOnlyFile?: boolean,
12841286
documentSpanToPrefix?: (span: T) => string,
1285-
startMarker?: (span: T) => string,
1286-
endMarker?: (span: T) => string,
1287+
endMarker?: string,
1288+
startMarkerPrefix?: (span: T) => string | undefined,
1289+
endMarkerSuffix?: (span: T) => string | undefined,
12871290
ignoredDocumentSpanProperties?: readonly string[],
12881291
) {
12891292
const marker: Marker | undefined = markerOrRange !== undefined ?
@@ -1335,22 +1338,64 @@ export class TestState {
13351338
group: readonly T[],
13361339
) {
13371340
let newContent = `=== ${fileName} ===\n`;
1338-
const details: [location: number, locationMarker: string, span?: T, type?: "textStart" | "textEnd" | "contextStart" | "contextEnd"][] = [];
1341+
type Detail = [location: number, locationMarker: string, span?: T, type?: "textStart" | "textEnd" | "contextStart" | "contextEnd"];
1342+
const detailPrefixes = new Map<Detail, string>();
1343+
const detailSuffixes = new Map<Detail, string>();
1344+
const details: Detail[] = [];
13391345
if (fileName === marker?.fileName) details.push([marker.position, refType]);
13401346
let canDetermineContextIdInline = true;
13411347
for (const span of group) {
1348+
const contextSpanIndex = details.length;
13421349
if (span.contextSpan) {
13431350
details.push([span.contextSpan.start, "<|", span, "contextStart"]);
13441351
if (canDetermineContextIdInline && span.contextSpan.start > span.textSpan.start) {
13451352
// Need to do explicit pass to determine contextId since contextId starts after textStart
13461353
canDetermineContextIdInline = false;
13471354
}
13481355
}
1356+
const textSpanIndex = details.length;
1357+
const textSpanEnd = span.textSpan.start + span.textSpan.length;
13491358
details.push(
1350-
[span.textSpan.start, startMarker?.(span) || "[|", span, "textStart"],
1351-
[span.textSpan.start + span.textSpan.length, endMarker?.(span) || "|]", span, "textEnd"],
1359+
[span.textSpan.start, "[|", span, "textStart"],
1360+
[textSpanEnd, endMarker || "|]", span, "textEnd"],
13521361
);
1353-
if (span.contextSpan) details.push([span.contextSpan.start + span.contextSpan.length, "|>", span, "contextEnd"]);
1362+
let contextSpanEnd: number | undefined;
1363+
if (span.contextSpan) {
1364+
contextSpanEnd = span.contextSpan.start + span.contextSpan.length;
1365+
details.push([contextSpanEnd, "|>", span, "contextEnd"]);
1366+
}
1367+
1368+
const startPrefix = startMarkerPrefix?.(span);
1369+
if (startPrefix) {
1370+
if (fileName === marker?.fileName && span.textSpan.start === marker?.position) {
1371+
ts.Debug.assert(!detailPrefixes.has(details[0]), "Expected only single prefix at marker location");
1372+
detailPrefixes.set(details[0], startPrefix);
1373+
}
1374+
else if (span.contextSpan?.start === span.textSpan.start) {
1375+
// Write it at contextSpan instead of textSpan
1376+
detailPrefixes.set(details[contextSpanIndex], startPrefix);
1377+
}
1378+
else {
1379+
// At textSpan
1380+
detailPrefixes.set(details[textSpanIndex], startPrefix);
1381+
}
1382+
}
1383+
1384+
const endSuffix = endMarkerSuffix?.(span);
1385+
if (endSuffix) {
1386+
if (fileName === marker?.fileName && textSpanEnd === marker?.position) {
1387+
ts.Debug.assert(!detailSuffixes.has(details[0]), "Expected only single suffix at marker location");
1388+
detailSuffixes.set(details[0], endSuffix);
1389+
}
1390+
else if (contextSpanEnd === textSpanEnd) {
1391+
// Write it at contextSpan instead of textSpan
1392+
detailSuffixes.set(details[textSpanIndex + 2], endSuffix);
1393+
}
1394+
else {
1395+
// At textSpan
1396+
detailSuffixes.set(details[textSpanIndex + 1], endSuffix);
1397+
}
1398+
}
13541399
}
13551400
let pos = 0;
13561401
const sortedDetails = ts.stableSort(details, (a, b) => ts.compareValues(a[0], b[0]));
@@ -1370,7 +1415,8 @@ export class TestState {
13701415
// Stable sort should handle first two cases but with that marker will be before rangeEnd if locations match
13711416
// So we will defer writing marker in this case by checking and finding index of rangeEnd if same
13721417
let deferredMarkerIndex: number | undefined;
1373-
sortedDetails.forEach(([location, locationType, span, type], index) => {
1418+
sortedDetails.forEach((detail, index) => {
1419+
const [location, locationType, span, type] = detail;
13741420
if (!foundMarker && !span && deferredMarkerIndex === undefined) {
13751421
foundMarker = true;
13761422
// If this is marker position and its same as textEnd and/or contextEnd we want to write marker after those
@@ -1386,30 +1432,38 @@ export class TestState {
13861432
// Defer writing marker position to deffered marker index
13871433
if (deferredMarkerIndex !== undefined) return;
13881434
}
1389-
newContent += content.slice(pos, location) + locationType;
1435+
newContent += content.slice(pos, location);
13901436
pos = location;
1391-
if (!span) return;
1392-
switch (type) {
1393-
case "textStart":
1394-
let text = convertDocumentSpanToString(span, documentSpanToPrefix?.(span), ignoredDocumentSpanProperties);
1395-
const contextId = spanToContextId.get(span);
1396-
if (contextId !== undefined) {
1397-
text = `contextId: ${contextId}` + (text ? ", " : "") + text;
1398-
}
1399-
if (text) newContent += `{| ${text} |}`;
1400-
break;
1401-
case "contextStart":
1402-
if (canDetermineContextIdInline) {
1403-
spanToContextId.set(span, currentContextId);
1404-
currentContextId++;
1405-
}
1406-
break;
1407-
}
1408-
if (deferredMarkerIndex === index) {
1409-
// Write the marker
1410-
newContent += refType;
1411-
deferredMarkerIndex = undefined;
1437+
// Prefix
1438+
const prefix = detailPrefixes.get(detail);
1439+
if (prefix) newContent += prefix;
1440+
newContent += locationType;
1441+
if (span) {
1442+
switch (type) {
1443+
case "textStart":
1444+
let text = convertDocumentSpanToString(span, documentSpanToPrefix?.(span), ignoredDocumentSpanProperties);
1445+
const contextId = spanToContextId.get(span);
1446+
if (contextId !== undefined) {
1447+
text = `contextId: ${contextId}` + (text ? ", " : "") + text;
1448+
}
1449+
if (text) newContent += `{| ${text} |}`;
1450+
break;
1451+
case "contextStart":
1452+
if (canDetermineContextIdInline) {
1453+
spanToContextId.set(span, currentContextId);
1454+
currentContextId++;
1455+
}
1456+
break;
1457+
}
1458+
if (deferredMarkerIndex === index) {
1459+
// Write the marker
1460+
newContent += refType;
1461+
deferredMarkerIndex = undefined;
1462+
detail = details[0]; // Marker detail
1463+
}
14121464
}
1465+
const suffix = detailSuffixes.get(detail);
1466+
if (suffix) newContent += suffix;
14131467
});
14141468
newContent += content.slice(pos);
14151469
return readableJsoncBaseline(newContent);
@@ -1639,8 +1693,9 @@ export class TestState {
16391693
ts.identity,
16401694
/*skipJsonOrGetAdditionalBaseline*/ undefined,
16411695
/*documentSpanToPrefix*/ undefined,
1642-
span => (span.prefixText ? `/*START PREFIX*/${span.prefixText}` : "") + "[|",
1643-
span => "RENAME|]" + (span.suffixText ? `${span.suffixText}/*END SUFFIX*/` : ""),
1696+
"RENAME|]",
1697+
span => span.prefixText ? `/*START PREFIX*/${span.prefixText}` : "",
1698+
span => span.suffixText ? `${span.suffixText}/*END SUFFIX*/` : "",
16441699
["prefixText", "suffixText"],
16451700
);
16461701
}

‎tests/baselines/reference/findAllReferencesDynamicImport3.baseline.jsonc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@
287287

288288
// === /tests/cases/fourslash/foo.ts ===
289289
// export function bar() { return "bar"; }
290-
// import('./foo').then((<|{ /*RENAME*//*START PREFIX*/bar: [|{| contextId: 0 |}barRENAME|] }|>) => undefined);
290+
// import('./foo').then((<|{ /*START PREFIX*/bar: /*RENAME*/[|{| contextId: 0 |}barRENAME|] }|>) => undefined);
291291

292292
[
293293
{

‎tests/baselines/reference/findAllRefsPrefixSuffixPreference.baseline.jsonc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,7 @@
12081208
// const x = {
12091209
// z: 'value'
12101210
// }
1211-
// <|const { /*RENAME*//*START PREFIX*/z: [|{| contextId: 0 |}zRENAME|] } = x;|>
1211+
// <|const { /*START PREFIX*/z: /*RENAME*/[|{| contextId: 0 |}zRENAME|] } = x;|>
12121212
// log([|zRENAME|]);
12131213

12141214
[
@@ -1310,7 +1310,7 @@
13101310
// === /file1.ts ===
13111311
// declare function log(s: string | number): void;
13121312
// const q = 1;
1313-
// <|export { /*RENAME*//*START PREFIX*/q as [|{| contextId: 0 |}qRENAME|] };|>
1313+
// <|export { /*START PREFIX*/q as /*RENAME*/[|{| contextId: 0 |}qRENAME|] };|>
13141314
// const x = {
13151315
// z: 'value'
13161316
// }
@@ -1359,7 +1359,7 @@
13591359
// providePrefixAndSuffixTextForRename: true
13601360
// === /file2.ts ===
13611361
// declare function log(s: string | number): void;
1362-
// <|import { /*RENAME*//*START PREFIX*/q as [|{| contextId: 0 |}qRENAME|] } from "./file1";|>
1362+
// <|import { /*START PREFIX*/q as /*RENAME*/[|{| contextId: 0 |}qRENAME|] } from "./file1";|>
13631363
// log([|qRENAME|] + 1);
13641364

13651365
[

‎tests/baselines/reference/findAllRefsReExportLocal.baseline.jsonc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2670,7 +2670,7 @@
26702670

26712671
// === /a.ts ===
26722672
// var x;
2673-
// <|export { /*RENAME*//*START PREFIX*/x as [|{| contextId: 0 |}xRENAME|] };|>
2673+
// <|export { /*START PREFIX*/x as /*RENAME*/[|{| contextId: 0 |}xRENAME|] };|>
26742674
// export { x as y };
26752675

26762676
// === /b.ts ===
@@ -2711,7 +2711,7 @@
27112711
]
27122712

27132713
// === /b.ts ===
2714-
// <|import { /*RENAME*//*START PREFIX*/x as [|{| contextId: 0 |}xRENAME|], y } from "./a";|>
2714+
// <|import { /*START PREFIX*/x as /*RENAME*/[|{| contextId: 0 |}xRENAME|], y } from "./a";|>
27152715
// [|xRENAME|]; y;
27162716

27172717
[
@@ -2804,7 +2804,7 @@
28042804
]
28052805

28062806
// === /b.ts ===
2807-
// <|import { x, /*RENAME*//*START PREFIX*/y as [|{| contextId: 0 |}yRENAME|] } from "./a";|>
2807+
// <|import { x, /*START PREFIX*/y as /*RENAME*/[|{| contextId: 0 |}yRENAME|] } from "./a";|>
28082808
// x; [|yRENAME|];
28092809

28102810
[

‎tests/baselines/reference/findAllRefsReExportRightNameWrongSymbol.baseline.jsonc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,7 +1362,7 @@
13621362

13631363
// === /c.ts ===
13641364
// export { x } from "./b";
1365-
// <|import { /*RENAME*//*START PREFIX*/x as [|{| contextId: 0 |}xRENAME|] } from "./a";|>
1365+
// <|import { /*START PREFIX*/x as /*RENAME*/[|{| contextId: 0 |}xRENAME|] } from "./a";|>
13661366
// [|xRENAME|];
13671367

13681368
[
@@ -1449,7 +1449,7 @@
14491449
]
14501450

14511451
// === /c.ts ===
1452-
// <|export { /*RENAME*//*START PREFIX*/x as [|{| contextId: 0 |}xRENAME|] } from "./b";|>
1452+
// <|export { /*START PREFIX*/x as /*RENAME*/[|{| contextId: 0 |}xRENAME|] } from "./b";|>
14531453
// import { x } from "./a";
14541454
// x;
14551455

@@ -1483,7 +1483,7 @@
14831483
]
14841484

14851485
// === /d.ts ===
1486-
// <|import { /*RENAME*//*START PREFIX*/x as [|{| contextId: 0 |}xRENAME|] } from "./c";|>
1486+
// <|import { /*START PREFIX*/x as /*RENAME*/[|{| contextId: 0 |}xRENAME|] } from "./c";|>
14871487

14881488
[
14891489
{

‎tests/baselines/reference/findAllRefsReExports.baseline.jsonc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8501,7 +8501,7 @@
85018501
]
85028502

85038503
// === /e.ts ===
8504-
// <|import { /*RENAME*//*START PREFIX*/bar as [|{| contextId: 0 |}barRENAME|] } from "./b";|>
8504+
// <|import { /*START PREFIX*/bar as /*RENAME*/[|{| contextId: 0 |}barRENAME|] } from "./b";|>
85058505
// import baz from "./c";
85068506
// import { default as bang } from "./c";
85078507
// import boom from "./d";

‎tests/baselines/reference/findAllRefsReExportsUseInImportType.baseline.jsonc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,7 +1312,7 @@
13121312

13131313
// === /foo/types/index.ts ===
13141314
// import * as foo from './types';
1315-
// <|export { /*RENAME*//*START PREFIX*/foo as [|{| contextId: 0 |}fooRENAME|] };|>
1315+
// <|export { /*START PREFIX*/foo as /*RENAME*/[|{| contextId: 0 |}fooRENAME|] };|>
13161316

13171317
// === /app.ts ===
13181318
// <|import { [|{| contextId: 1 |}fooRENAME|] } from './foo/types';|>
@@ -1361,7 +1361,7 @@
13611361
]
13621362

13631363
// === /app.ts ===
1364-
// <|import { /*RENAME*//*START PREFIX*/foo as [|{| contextId: 0 |}fooRENAME|] } from './foo/types';|>
1364+
// <|import { /*START PREFIX*/foo as /*RENAME*/[|{| contextId: 0 |}fooRENAME|] } from './foo/types';|>
13651365
// export type fullType = [|fooRENAME|].Full;
13661366
// type namespaceImport = typeof import('./foo/types');
13671367
// type fullType2 = import('./foo/types').foo.Full;

‎tests/baselines/reference/highlightsForExportFromUnfoundModule.baseline.jsonc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// === RenameOptions ===
22
// === /tests/cases/fourslash/b.js ===
33
// <|export {
4-
// /*RENAME*//*START PREFIX*/foo as [|{| contextId: 0 |}fooRENAME|]
4+
// /*START PREFIX*/foo as /*RENAME*/[|{| contextId: 0 |}fooRENAME|]
55
// } from './a';|>
66

77
[

‎tests/baselines/reference/renameBindingElementInitializerProperty.baseline.jsonc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// === /tests/cases/fourslash/renameBindingElementInitializerProperty.ts ===
2-
// function f(<|{/*RENAME*//*START PREFIX*/required: [|{| contextId: 0 |}requiredRENAME|], optional = [|requiredRENAME|]}: {required: number, optional?: number}|>) {
2+
// function f(<|{/*START PREFIX*/required: /*RENAME*/[|{| contextId: 0 |}requiredRENAME|], optional = [|requiredRENAME|]}: {required: number, optional?: number}|>) {
33
// console.log("required", [|requiredRENAME|]);
44
// console.log("optional", optional);
55
// }

‎tests/baselines/reference/renameCrossJsTs01.baseline.jsonc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
]
3939

4040
// === /tests/cases/fourslash/b.ts ===
41-
// <|import { /*RENAME*//*START PREFIX*/area as [|{| contextId: 0 |}areaRENAME|] } from './a';|>
41+
// <|import { /*START PREFIX*/area as /*RENAME*/[|{| contextId: 0 |}areaRENAME|] } from './a';|>
4242
// var t = [|areaRENAME|](10);
4343

4444
[

0 commit comments

Comments
 (0)