Skip to content

Commit

Permalink
perf: Improve performance of SourceFile#indent() and deindent().
Browse files Browse the repository at this point in the history
This will definitely be faster. No more regex, and has a reduction in string & array allocations.
  • Loading branch information
dsherret committed May 18, 2019
1 parent a2210de commit 17eefea
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ export function getBodyTextWithoutLeadingIndentation(body: Node) {
return "";

const fullText = sourceFile.getFullText().substring(startPos, endPos);

return StringUtils.indent(fullText,
-1 * textArea.getIndentationLevel(),
textArea._context.manipulationSettings.getIndentationText(),
pos => sourceFile.isInStringAtPos(pos + startPos));
return StringUtils.removeIndentation(fullText, {
indentSizeInSpaces: body._context.manipulationSettings._getIndentSizeInSpaces(),
isInStringAtPos: pos => sourceFile.isInStringAtPos(pos + startPos)
});
}
2 changes: 1 addition & 1 deletion src/compiler/ast/common/Node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ export class Node<NodeType extends ts.Node = ts.Node> implements TextRange {
if (trimLeadingIndentation) {
return StringUtils.removeIndentation(text, {
isInStringAtPos: pos => this._sourceFile.isInStringAtPos(pos + startPos),
tabSize: this._context.manipulationSettings._getTabSize()
indentSizeInSpaces: this._context.manipulationSettings._getIndentSizeInSpaces()
});
}
else {
Expand Down
8 changes: 5 additions & 3 deletions src/compiler/ast/module/SourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,12 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {

const startLinePos = getPreviousMatchingPos(sourceFileText, positionRange[0], char => char === "\n");
const endLinePos = getNextMatchingPos(sourceFileText, positionRange[1], char => char === "\r" || char === "\n");
const indentText = this._context.manipulationSettings.getIndentationText();

const correctedText = StringUtils.indent(sourceFileText.substring(startLinePos, endLinePos),
times, indentText, pos => this.isInStringAtPos(pos + startLinePos));
const correctedText = StringUtils.indent(sourceFileText.substring(startLinePos, endLinePos), times, {
indentText: this._context.manipulationSettings.getIndentationText(),
indentSizeInSpaces: this._context.manipulationSettings._getIndentSizeInSpaces(),
isInStringAtPos: pos => this.isInStringAtPos(pos + startLinePos)
});

replaceSourceFileTextForFormatting({
sourceFile: this,
Expand Down
4 changes: 2 additions & 2 deletions src/options/ManipulationSettingsContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ export class ManipulationSettingsContainer extends SettingsContainer<Manipulatio

/**
* @internal
* Gets the tab size as represented in spaces.
* Gets the indent size as represented in spaces.
*/
_getTabSize() {
_getIndentSizeInSpaces() {
const indentationText = this.getIndentationText();
switch (indentationText) {
case IndentationText.EightSpaces:
Expand Down
85 changes: 80 additions & 5 deletions src/tests/utils/stringUtilsTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,9 @@ describe(nameof(StringUtils), () => {
});

describe(nameof(StringUtils.removeIndentation), () => {
function doTest(input: string, expectedOutput: string, options: { tabSize?: number; isInStringAtPos?: (pos: number) => boolean; } = {}) {
function doTest(input: string, expectedOutput: string, options: { indentSizeInSpaces?: number; isInStringAtPos?: (pos: number) => boolean; } = {}) {
const actualResult = StringUtils.removeIndentation(input, {
tabSize: options.tabSize || 4,
indentSizeInSpaces: options.indentSizeInSpaces || 4,
isInStringAtPos: options.isInStringAtPos || (() => false)
});

Expand Down Expand Up @@ -309,11 +309,11 @@ describe(nameof(StringUtils), () => {
});

it("should consider the first line's indent if only indented by one space and the tab size is 4", () => {
doTest(" testing\n this", "testing\n this", { tabSize: 4 });
doTest(" testing\n this", "testing\n this", { indentSizeInSpaces: 4 });
});

it("should consider the first line's indent if only indented by one space and the tab size is 2", () => {
doTest(" testing\n this", "testing\n this", { tabSize: 2 });
doTest(" testing\n this", "testing\n this", { indentSizeInSpaces: 2 });
});

it("should remove based on the minimum width", () => {
Expand All @@ -325,7 +325,7 @@ describe(nameof(StringUtils), () => {
});

it("should treat tabs based on the tab size provided when mixing spaces and tabs", () => {
doTest("{\n \t test\n }", "{\n test\n}", { tabSize: 2 });
doTest("{\n \t test\n }", "{\n test\n}", { indentSizeInSpaces: 2 });
});

it("should not deindent within strings", () => {
Expand All @@ -337,4 +337,79 @@ describe(nameof(StringUtils), () => {
doTest(str, "this is a `\n test`\nother", { isInStringAtPos: index => index >= pos && index < end });
});
});

describe(nameof(StringUtils.indent), () => {
interface TestOptions {
indentText?: string;
indentSizeInSpaces?: number;
isInStringAtPos?: (pos: number) => boolean;
}

function doTest(input: string, times: number, expectedOutput: string, options: TestOptions = {}) {
const actualResult = StringUtils.indent(input, times, {
indentSizeInSpaces: options.indentSizeInSpaces || 4,
indentText: options.indentText || " ",
isInStringAtPos: options.isInStringAtPos || (() => false)
});

expect(actualResult).to.equal(expectedOutput);
}

it("should do nothing when providing 0", () => {
const text = "t\nu\n v";
doTest(text, 0, text);
});

it("should indent the provided number of times", () => {
doTest("testing\nthis\n out", 2, " testing\n this\n out");
});

it("should indent based on the provided", () => {
doTest("testing\nthis\n out", 2, " testing\n this\n out", { indentText: " ", indentSizeInSpaces: 2 });
});

it("should indent with tabs", () => {
doTest("testing\nthis\n out", 1, "\ttesting\n\tthis\n\t out", { indentText: "\t", indentSizeInSpaces: 4 });
});

it("should not indent the last line if it's empty", () => {
doTest("testing\n", 1, " testing\n");
});

it("should not indent within strings", () => {
let text = "t`";
const pos = text.length;
text += "\nt`";
const end = text.length;
text += "\nt";

doTest(text, 1, " t`\nt`\n t", {
isInStringAtPos: p => p >= pos && p < end
});
});

it("should deindent when providing a negative number", () => {
doTest("t\n u\nv", -1, "t\nu\nv");
});

it("should deindent by the provided amount", () => {
doTest(" t\n u\n v", -1, " t\nu\nv", { indentText: " ", indentSizeInSpaces: 2 });
});

it("should deindent handling tabs and spaces", () => {
doTest("\t \tt\n\tu\n v", -2, "\tt\nu\nv");
});

it("should not deindent within strings", () => {
let text = "t`";
const pos = text.length;
text += "\n t`";
const end = text.length;
text += "\n t";

doTest(text, -1, "t`\n t`\nt", {
isInStringAtPos: p => p >= pos && p < end
});
});
});
});
78 changes: 43 additions & 35 deletions src/utils/StringUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ export class StringUtils {
return result;
}

static removeIndentation(str: string, opts: { isInStringAtPos: (pos: number) => boolean; tabSize: number; }) {
const { isInStringAtPos, tabSize } = opts;
static removeIndentation(str: string, opts: { isInStringAtPos: (pos: number) => boolean; indentSizeInSpaces: number; }) {
const { isInStringAtPos, indentSizeInSpaces } = opts;
const startPositions: number[] = [];
const endPositions: number[] = [];
let minIndentWidth: number | undefined;
Expand All @@ -149,14 +149,14 @@ export class StringUtils {
let isAtStartOfLine = str[0] === " " || str[0] === "\t";

for (let i = 0; i < str.length; i++) {
if (isAtStartOfLine)
startPositions.push(i);
else {
if (!isAtStartOfLine) {
if (str[i] === "\n" && !isInStringAtPos(i + 1))
isAtStartOfLine = true;
continue;
}

startPositions.push(i);

let spacesCount = 0;
let tabsCount = 0;

Expand All @@ -172,7 +172,7 @@ export class StringUtils {
}

// indentation for spaces rounds up to the nearest tab size multiple
const indentWidth = Math.ceil(spacesCount / tabSize) * tabSize + tabsCount * tabSize;
const indentWidth = Math.ceil(spacesCount / indentSizeInSpaces) * indentSizeInSpaces + tabsCount * indentSizeInSpaces;
if (minIndentWidth == null || indentWidth < minIndentWidth)
minIndentWidth = indentWidth;

Expand Down Expand Up @@ -203,7 +203,7 @@ export class StringUtils {
if (str[pos] === " ")
indentCount++;
else if (str[pos] === "\t")
indentCount += tabSize;
indentCount += indentSizeInSpaces;
}

lastEndPos = startPositions[i + 1] == null ? str.length : startPositions[i + 1];
Expand All @@ -216,41 +216,49 @@ export class StringUtils {
}
}

static indent(str: string, times: number, indentText: string, isInStringAtPos: (pos: number) => boolean) {
// todo: unit test this (right now it's somewhat tested indirectly)
const unIndentRegex = times > 0 ? undefined : new RegExp(getDeIndentRegexText());
const newLines: string[] = [];
let pos = 0;
static indent(str: string, times: number, options: { indentText: string; indentSizeInSpaces: number; isInStringAtPos: (pos: number) => boolean; }) {
if (times === 0)
return str;

for (const line of str.split("\n")) {
if (isInStringAtPos(pos))
newLines.push(line);
else if (times > 0)
newLines.push(indentText.repeat(times) + line);
else // negative
newLines.push(line.replace(unIndentRegex!, ""));
// this assumes that the indentText and indentSizeInSpaces are proportional
const { indentText, indentSizeInSpaces, isInStringAtPos } = options;
const fullIndentationText = times > 0 ? indentText.repeat(times) : undefined;
const totalIndentSpaces = Math.abs(times * indentSizeInSpaces);
let result = "";
let lineStart = 0;
let lineEnd = 0;

pos += line.length + 1; // +1 for \n char
for (let i = 0; i < str.length; i++) {
lineStart = i;
while (i < str.length && str[i] !== "\n")
i++;
lineEnd = i === str.length ? i : i + 1;
appendLine();
}

return newLines.join("\n");
return result;

function getDeIndentRegexText() {
let text = "^";
for (let i = 0; i < Math.abs(times); i++) {
text += "(";
if (StringUtils.isSpaces(indentText)) {
// the optional string makes it possible to unindent when a line doesn't have the full number of spaces
for (let j = 0; j < indentText.length; j++)
text += " ?";
}
else
text += indentText;
function appendLine() {
if (isInStringAtPos(lineStart))
result += str.substring(lineStart, lineEnd);
else if (times > 0)
result += fullIndentationText + str.substring(lineStart, lineEnd);
else { // negative times
let start = lineStart;
let indentSpaces = 0;
for (start = lineStart; start < str.length; start++) {
if (indentSpaces >= totalIndentSpaces)
break;

text += "|\t)?";
if (str[start] === " ")
indentSpaces++;
else if (str[start] === "\t")
indentSpaces += indentSizeInSpaces;
else
break;
}
result += str.substring(start, lineEnd);
}

return text;
}
}
}

0 comments on commit 17eefea

Please sign in to comment.