microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-05-05 04:25:39 -05:00
Code Issues Packages Projects Releases 212 Wiki Activity

Fix bad line number assertion in ScriptInfo.positionToLineOffset

Browse Source 
This is the line number side of ecddf8468f (from #21924, fixing #21818).
But the code is slightly improved for both cases: instead of testing
that `leaf` is defined, check whether `lineCount` is zero, and if it is,
return `〈1,0〉` for the one-based line and zero-based column numbers.
(The requirement of `lineCount > 0` is also seen in the fact
that `lineNumberToInfo` expects a "*One*BasedLine" argument.)

I've stared at this code way too much, since I think that there is
something more fundamentally wrong here.  E.g., `EditWalker` only
`push`es to `startPath` but never pops even a `children`-less node that
is left after deleting the whole contents.  But I can't figure out the
overall structure, which is also why the test that I added is not
great (see the comment there; also, #21924 is dealing with the same
problem and didn't add a test).

Fixes #44518.
This commit is contained in:
Eli Barzilay
2021-06-25 13:45:43 -04:00
parent 5cea46c537
commit f47ddbc3b0
2 changed files with 20 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/server/scriptVersionCache.ts
View File

@@ -67,10 +67,8 @@ namespace ts.server {
}
const lm = LineIndex.linesFromText(insertedText);
const lines = lm.lines;
if (lines.length > 1) {
if (lines[lines.length - 1] === "") {
lines.pop();
}
if (lines.length > 1 && lines[lines.length - 1] === "") {
lines.pop();
}
let branchParent: LineNode | undefined;
let lastZeroCount: LineCollection | undefined;
@@ -683,8 +681,12 @@ namespace ts.server {
}
// Skipped all children
const { leaf } = this.lineNumberToInfo(this.lineCount(), 0);
return { oneBasedLine: this.lineCount(), zeroBasedColumn: leaf ? leaf.charCount() : 0, lineText: undefined };
const lineCount = this.lineCount();
if (lineCount === 0) { // it's empty! (and lineNumberToInfo expects a one-based line)
return { oneBasedLine: 1, zeroBasedColumn: 0, lineText: undefined };
}
const leaf = Debug.checkDefined(this.lineNumberToInfo(lineCount, 0).leaf);
return { oneBasedLine: lineCount, zeroBasedColumn: leaf.charCount(), lineText: undefined };
}
/**

12
src/testRunner/unittests/tsserver/versionCache.ts
View File

@@ -52,6 +52,18 @@ var q:Point=<Point>p;`;
assert.deepEqual(lineIndex.positionToLineOffset(0), { line: 1, offset: 1 });
});
it("handles emptying whole file (GH#44518)", () => {
// See below for the main thing that this tests; it would be better to have a test
// that uses `ScriptInfo.positionToLineOffset` but I couldn't find away to do that
const { lines } = server.LineIndex.linesFromText("function foo() {\n\ndsa\n\n}\n\nfo(dsa\n\n\n ");
const lineIndex = new server.LineIndex();
lineIndex.load(lines);
const snapshot = lineIndex.edit(0, 39);
assert.equal(snapshot.getText(0, snapshot.getLength()), "");
// line must always be >=1, otherwise the failIfInvalidLocation(location) assertion in ScriptInfo.positionToLineOffset will fail
assert.deepEqual(snapshot.positionToLineOffset(0), { line: 1, offset: 1 });
});
it(`change 9 1 0 1 {"y"}`, () => {
validateEditAtLineCharIndex(9, 1, 0, "y");
});