Skip to content

Commit d49491b

Browse files
author
Andy
authored
smartIndenter: Don't indent after control-flow ending statements like break; (microsoft#20016)
* smartIndenter: Don't indent after control-flow ending statements like `break;` * Fix bug * Fix bug for function after `return`
1 parent 09d0a67 commit d49491b

3 files changed

Lines changed: 100 additions & 70 deletions

File tree

‎src/services/formatting/smartIndenter.ts‎

Lines changed: 98 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,7 @@ namespace ts.formatting {
3636

3737
const enclosingCommentRange = getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ true, precedingToken || null); // tslint:disable-line:no-null-keyword
3838
if (enclosingCommentRange) {
39-
const previousLine = getLineAndCharacterOfPosition(sourceFile, position).line - 1;
40-
const commentStartLine = getLineAndCharacterOfPosition(sourceFile, enclosingCommentRange.pos).line;
41-
42-
Debug.assert(commentStartLine >= 0);
43-
44-
if (previousLine <= commentStartLine) {
45-
return findFirstNonWhitespaceColumn(getStartPositionOfLine(commentStartLine, sourceFile), position, sourceFile, options);
46-
}
47-
48-
const startPostionOfLine = getStartPositionOfLine(previousLine, sourceFile);
49-
const { column, character } = findFirstNonWhitespaceCharacterAndColumn(startPostionOfLine, position, sourceFile, options);
50-
51-
if (column === 0) {
52-
return column;
53-
}
54-
55-
const firstNonWhitespaceCharacterCode = sourceFile.text.charCodeAt(startPostionOfLine + character);
56-
return firstNonWhitespaceCharacterCode === CharacterCodes.asterisk ? column - 1 : column;
39+
return getCommentIndent(sourceFile, position, options, enclosingCommentRange);
5740
}
5841

5942
if (!precedingToken) {
@@ -72,20 +55,7 @@ namespace ts.formatting {
7255
// for block indentation, we should look for a line which contains something that's not
7356
// whitespace.
7457
if (options.indentStyle === IndentStyle.Block) {
75-
76-
// move backwards until we find a line with a non-whitespace character,
77-
// then find the first non-whitespace character for that line.
78-
let current = position;
79-
while (current > 0) {
80-
const char = sourceFile.text.charCodeAt(current);
81-
if (!isWhiteSpaceLike(char)) {
82-
break;
83-
}
84-
current--;
85-
}
86-
87-
const lineStart = ts.getLineStartPositionForPosition(current, sourceFile);
88-
return SmartIndenter.findFirstNonWhitespaceColumn(lineStart, current, sourceFile, options);
58+
return getBlockIndent(sourceFile, position, options);
8959
}
9060

9161
if (precedingToken.kind === SyntaxKind.CommaToken && precedingToken.parent.kind !== SyntaxKind.BinaryExpression) {
@@ -96,26 +66,60 @@ namespace ts.formatting {
9666
}
9767
}
9868

69+
return getSmartIndent(sourceFile, position, precedingToken, lineAtPosition, assumeNewLineBeforeCloseBrace, options);
70+
}
71+
72+
function getCommentIndent(sourceFile: SourceFile, position: number, options: EditorSettings, enclosingCommentRange: CommentRange): number {
73+
const previousLine = getLineAndCharacterOfPosition(sourceFile, position).line - 1;
74+
const commentStartLine = getLineAndCharacterOfPosition(sourceFile, enclosingCommentRange.pos).line;
75+
76+
Debug.assert(commentStartLine >= 0);
77+
78+
if (previousLine <= commentStartLine) {
79+
return findFirstNonWhitespaceColumn(getStartPositionOfLine(commentStartLine, sourceFile), position, sourceFile, options);
80+
}
81+
82+
const startPostionOfLine = getStartPositionOfLine(previousLine, sourceFile);
83+
const { column, character } = findFirstNonWhitespaceCharacterAndColumn(startPostionOfLine, position, sourceFile, options);
84+
85+
if (column === 0) {
86+
return column;
87+
}
88+
89+
const firstNonWhitespaceCharacterCode = sourceFile.text.charCodeAt(startPostionOfLine + character);
90+
return firstNonWhitespaceCharacterCode === CharacterCodes.asterisk ? column - 1 : column;
91+
}
92+
93+
function getBlockIndent(sourceFile: SourceFile, position: number, options: EditorSettings): number {
94+
// move backwards until we find a line with a non-whitespace character,
95+
// then find the first non-whitespace character for that line.
96+
let current = position;
97+
while (current > 0) {
98+
const char = sourceFile.text.charCodeAt(current);
99+
if (!isWhiteSpaceLike(char)) {
100+
break;
101+
}
102+
current--;
103+
}
104+
105+
const lineStart = ts.getLineStartPositionForPosition(current, sourceFile);
106+
return findFirstNonWhitespaceColumn(lineStart, current, sourceFile, options);
107+
}
108+
109+
function getSmartIndent(sourceFile: SourceFile, position: number, precedingToken: Node, lineAtPosition: number, assumeNewLineBeforeCloseBrace: boolean, options: EditorSettings): number {
99110
// try to find node that can contribute to indentation and includes 'position' starting from 'precedingToken'
100111
// if such node is found - compute initial indentation for 'position' inside this node
101-
let previous: Node;
112+
let previous: Node | undefined;
102113
let current = precedingToken;
103-
let currentStart: LineAndCharacter;
104-
let indentationDelta: number;
105-
106114
while (current) {
107-
if (positionBelongsToNode(current, position, sourceFile) && shouldIndentChildNode(current, previous)) {
108-
currentStart = getStartLineAndCharacterForNode(current, sourceFile);
109-
115+
if (positionBelongsToNode(current, position, sourceFile) && shouldIndentChildNode(current, previous, /*isNextChild*/ true)) {
116+
const currentStart = getStartLineAndCharacterForNode(current, sourceFile);
110117
const nextTokenKind = nextTokenIsCurlyBraceOnSameLineAsCursor(precedingToken, current, lineAtPosition, sourceFile);
111-
if (nextTokenKind !== NextTokenKind.Unknown) {
118+
const indentationDelta = nextTokenKind !== NextTokenKind.Unknown
112119
// handle cases when codefix is about to be inserted before the close brace
113-
indentationDelta = assumeNewLineBeforeCloseBrace && nextTokenKind === NextTokenKind.CloseBrace ? options.indentSize : 0;
114-
}
115-
else {
116-
indentationDelta = lineAtPosition !== currentStart.line ? options.indentSize : 0;
117-
}
118-
break;
120+
? assumeNewLineBeforeCloseBrace && nextTokenKind === NextTokenKind.CloseBrace ? options.indentSize : 0
121+
: lineAtPosition !== currentStart.line ? options.indentSize : 0;
122+
return getIndentationForNodeWorker(current, currentStart, /*ignoreActualIndentationRange*/ undefined, indentationDelta, sourceFile, /*isNextChild*/ true, options);
119123
}
120124

121125
// check if current node is a list item - if yes, take indentation from it
@@ -131,18 +135,13 @@ namespace ts.formatting {
131135
previous = current;
132136
current = current.parent;
133137
}
134-
135-
if (!current) {
136-
// no parent was found - return the base indentation of the SourceFile
137-
return getBaseIndentation(options);
138-
}
139-
140-
return getIndentationForNodeWorker(current, currentStart, /*ignoreActualIndentationRange*/ undefined, indentationDelta, sourceFile, options);
138+
// no parent was found - return the base indentation of the SourceFile
139+
return getBaseIndentation(options);
141140
}
142141

143142
export function getIndentationForNode(n: Node, ignoreActualIndentationRange: TextRange, sourceFile: SourceFile, options: EditorSettings): number {
144143
const start = sourceFile.getLineAndCharacterOfPosition(n.getStart(sourceFile));
145-
return getIndentationForNodeWorker(n, start, ignoreActualIndentationRange, /*indentationDelta*/ 0, sourceFile, options);
144+
return getIndentationForNodeWorker(n, start, ignoreActualIndentationRange, /*indentationDelta*/ 0, sourceFile, /*isNextChild*/ false, options);
146145
}
147146

148147
export function getBaseIndentation(options: EditorSettings) {
@@ -155,11 +154,9 @@ namespace ts.formatting {
155154
ignoreActualIndentationRange: TextRange,
156155
indentationDelta: number,
157156
sourceFile: SourceFile,
157+
isNextChild: boolean,
158158
options: EditorSettings): number {
159-
160-
let parent: Node = current.parent;
161-
let containingListOrParentStart: LineAndCharacter;
162-
159+
let parent = current.parent!;
163160
// Walk up the tree and collect indentation for parent-child node pairs. Indentation is not added if
164161
// * parent and child nodes start on the same line, or
165162
// * parent is an IfStatement and child starts on the same line as an 'else clause'.
@@ -178,7 +175,7 @@ namespace ts.formatting {
178175
}
179176
}
180177

181-
containingListOrParentStart = getContainingListOrParentStart(parent, current, sourceFile);
178+
const containingListOrParentStart = getContainingListOrParentStart(parent, current, sourceFile);
182179
const parentAndChildShareLine =
183180
containingListOrParentStart.line === currentStart.line ||
184181
childStartsOnTheSameLineWithElseInIfStatement(parent, current, currentStart.line, sourceFile);
@@ -196,7 +193,7 @@ namespace ts.formatting {
196193
}
197194

198195
// increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line
199-
if (shouldIndentChildNode(parent, current) && !parentAndChildShareLine) {
196+
if (shouldIndentChildNode(parent, current, isNextChild) && !parentAndChildShareLine) {
200197
indentationDelta += options.indentSize;
201198
}
202199

@@ -214,7 +211,7 @@ namespace ts.formatting {
214211

215212
current = parent;
216213
parent = current.parent;
217-
currentStart = useTrueStart ? sourceFile.getLineAndCharacterOfPosition(current.getStart()) : containingListOrParentStart;
214+
currentStart = useTrueStart ? sourceFile.getLineAndCharacterOfPosition(current.getStart(sourceFile)) : containingListOrParentStart;
218215
}
219216

220217
return indentationDelta + getBaseIndentation(options);
@@ -533,8 +530,7 @@ namespace ts.formatting {
533530
return false;
534531
}
535532

536-
/* @internal */
537-
export function nodeWillIndentChild(parent: TextRangeWithKind, child: TextRangeWithKind, indentByDefault: boolean) {
533+
export function nodeWillIndentChild(parent: TextRangeWithKind, child: TextRangeWithKind | undefined, indentByDefault: boolean): boolean {
538534
const childKind = child ? child.kind : SyntaxKind.Unknown;
539535
switch (parent.kind) {
540536
case SyntaxKind.DoStatement:
@@ -555,19 +551,52 @@ namespace ts.formatting {
555551
return childKind !== SyntaxKind.NamedExports;
556552
case SyntaxKind.ImportDeclaration:
557553
return childKind !== SyntaxKind.ImportClause ||
558-
((<ImportClause>child).namedBindings && (<ImportClause>child).namedBindings.kind !== SyntaxKind.NamedImports);
554+
(!!(<ImportClause>child).namedBindings && (<ImportClause>child).namedBindings.kind !== SyntaxKind.NamedImports);
559555
case SyntaxKind.JsxElement:
560556
return childKind !== SyntaxKind.JsxClosingElement;
561557
}
562558
// No explicit rule for given nodes so the result will follow the default value argument
563559
return indentByDefault;
564560
}
565561

566-
/*
567-
Function returns true when the parent node should indent the given child by an explicit rule
568-
*/
569-
export function shouldIndentChildNode(parent: TextRangeWithKind, child?: TextRangeWithKind): boolean {
570-
return nodeContentIsAlwaysIndented(parent.kind) || nodeWillIndentChild(parent, child, /*indentByDefault*/ false);
562+
function isControlFlowEndingStatement(kind: SyntaxKind, parent: TextRangeWithKind): boolean {
563+
switch (kind) {
564+
case SyntaxKind.ReturnStatement:
565+
case SyntaxKind.ThrowStatement:
566+
switch (parent.kind) {
567+
case SyntaxKind.Block:
568+
const grandParent = (parent as Node).parent;
569+
switch (grandParent && grandParent.kind) {
570+
case SyntaxKind.FunctionDeclaration:
571+
case SyntaxKind.FunctionExpression:
572+
// We may want to write inner functions after this.
573+
return false;
574+
default:
575+
return true;
576+
}
577+
case SyntaxKind.CaseClause:
578+
case SyntaxKind.DefaultClause:
579+
case SyntaxKind.SourceFile:
580+
case SyntaxKind.ModuleBlock:
581+
return true;
582+
default:
583+
throw Debug.fail();
584+
}
585+
case SyntaxKind.ContinueStatement:
586+
case SyntaxKind.BreakStatement:
587+
return true;
588+
default:
589+
return false;
590+
}
591+
}
592+
593+
/**
594+
* True when the parent node should indent the given child by an explicit rule.
595+
* @param isNextChild If true, we are judging indent of a hypothetical child *after* this one, not the current child.
596+
*/
597+
export function shouldIndentChildNode(parent: TextRangeWithKind, child?: TextRangeWithKind, isNextChild = false): boolean {
598+
return (nodeContentIsAlwaysIndented(parent.kind) || nodeWillIndentChild(parent, child, /*indentByDefault*/ false))
599+
&& !(isNextChild && child && isControlFlowEndingStatement(child.kind, parent));
571600
}
572601
}
573602
}

‎src/services/utilities.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,7 @@ namespace ts {
441441
* Assumes `candidate.start <= position` holds.
442442
*/
443443
export function positionBelongsToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean {
444+
Debug.assert(candidate.pos <= position);
444445
return position < candidate.end || !isCompletedNode(candidate, sourceFile);
445446
}
446447

‎tests/cases/fourslash/smartIndentStatementSwitch.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//// case 1:
1212
//// {| "indentation": 12 |}
1313
//// break;
14-
//// {| "indentation": 12 |} // content of case clauses is always indented relatively to case clause
14+
//// {| "indentation": 8 |} // since we just saw "break"
1515
//// }
1616
//// {| "indentation": 4 |}
1717
////}

0 commit comments

Comments
 (0)