Skip to content

Commit c4e6ad8

Browse files
vladimavladima
committed
address CR feedback: renames, handle smart indentation in type argument lists in type references
1 parent 685d131 commit c4e6ad8

File tree

4 files changed

+119
-77
lines changed

4 files changed

+119
-77
lines changed

‎src/compiler/parser.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,32 @@ module ts {
384384
return false;
385385
}
386386

387+
export function isStatement(n: Node): boolean {
388+
switch(n.kind) {
389+
case SyntaxKind.BreakStatement:
390+
case SyntaxKind.ContinueStatement:
391+
case SyntaxKind.DebuggerStatement:
392+
case SyntaxKind.DoStatement:
393+
case SyntaxKind.ExpressionStatement:
394+
case SyntaxKind.EmptyStatement:
395+
case SyntaxKind.ForInStatement:
396+
case SyntaxKind.ForStatement:
397+
case SyntaxKind.IfStatement:
398+
case SyntaxKind.LabelledStatement:
399+
case SyntaxKind.ReturnStatement:
400+
case SyntaxKind.SwitchStatement:
401+
case SyntaxKind.ThrowKeyword:
402+
case SyntaxKind.TryStatement:
403+
case SyntaxKind.VariableStatement:
404+
case SyntaxKind.WhileStatement:
405+
case SyntaxKind.WithStatement:
406+
case SyntaxKind.ExportAssignment:
407+
return true;
408+
default:
409+
return false;
410+
}
411+
}
412+
387413
// True if the given identifier, string literal, or number literal is the name of a declaration node
388414
export function isDeclarationOrFunctionExpressionOrCatchVariableName(name: Node): boolean {
389415
if (name.kind !== SyntaxKind.Identifier && name.kind !== SyntaxKind.StringLiteral && name.kind !== SyntaxKind.NumericLiteral) {

‎src/services/formatting/smartIndenter.ts

Lines changed: 71 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,17 @@ module ts.formatting {
3535
var previous: Node;
3636
var current = precedingToken;
3737
var currentStart: LineAndCharacter;
38-
var indentation: number;
38+
var indentationDelta: number;
3939

4040
while (current) {
4141
if (positionBelongsToNode(current, position, sourceFile) && nodeContentIsIndented(current, previous)) {
4242
currentStart = getStartLineAndCharacterForNode(current, sourceFile);
4343

44-
if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) {
45-
indentation = 0;
44+
if (nextTokenIsCurlyBraceOnSameLineAsCursor(precedingToken, current, lineAtPosition, sourceFile)) {
45+
indentationDelta = 0;
4646
}
4747
else {
48-
indentation = lineAtPosition !== currentStart.line ? options.indentSpaces : 0;
48+
indentationDelta = lineAtPosition !== currentStart.line ? options.indentSpaces : 0;
4949
}
5050

5151
break;
@@ -73,11 +73,10 @@ module ts.formatting {
7373
// walk upwards and collect indentations for pairs of parent-child nodes
7474
// indentation is not added if parent and child nodes start on the same line or if parent is IfStatement and child starts on the same line with 'else clause'
7575
while (parent) {
76-
7776
// check if current node is a list item - if yes, take indentation from it
7877
var actualIndentation = getActualIndentationForListItem(current, sourceFile, options);
7978
if (actualIndentation !== -1) {
80-
return actualIndentation + indentation;
79+
return actualIndentation + indentationDelta;
8180
}
8281

8382
parentStart = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile));
@@ -88,69 +87,44 @@ module ts.formatting {
8887
// try to fetch actual indentation for current node from source text
8988
var actualIndentation = getActualIndentationForNode(current, parent, currentStart, parentAndChildShareLine, sourceFile, options);
9089
if (actualIndentation !== -1) {
91-
return actualIndentation + indentation;
90+
return actualIndentation + indentationDelta;
9291
}
9392

9493
// increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line
95-
var increaseIndentation = nodeContentIsIndented(parent, current) && !parentAndChildShareLine;
96-
97-
if (increaseIndentation) {
98-
indentation += options.indentSpaces;
94+
if (nodeContentIsIndented(parent, current) && !parentAndChildShareLine) {
95+
indentationDelta += options.indentSpaces;
9996
}
10097

10198
current = parent;
10299
currentStart = parentStart;
103100
parent = current.parent;
104101
}
105102

106-
return indentation;
107-
}
108-
109-
function isStatement(n: Node): boolean {
110-
switch(n.kind) {
111-
case SyntaxKind.BreakStatement:
112-
case SyntaxKind.ContinueStatement:
113-
case SyntaxKind.DebuggerStatement:
114-
case SyntaxKind.DoStatement:
115-
case SyntaxKind.ExpressionStatement:
116-
case SyntaxKind.EmptyStatement:
117-
case SyntaxKind.ForInStatement:
118-
case SyntaxKind.ForStatement:
119-
case SyntaxKind.IfStatement:
120-
case SyntaxKind.LabelledStatement:
121-
case SyntaxKind.ReturnStatement:
122-
case SyntaxKind.SwitchStatement:
123-
case SyntaxKind.ThrowKeyword:
124-
case SyntaxKind.TryStatement:
125-
case SyntaxKind.VariableStatement:
126-
case SyntaxKind.WhileStatement:
127-
case SyntaxKind.WithStatement:
128-
return true;
129-
default:
130-
return false;
131-
}
103+
return indentationDelta;
132104
}
133105

134106
/*
135107
* Function returns -1 if indentation cannot be determined
136108
*/
137109
function getActualIndentationForListItemBeforeComma(commaToken: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number {
138110
// previous token is comma that separates items in list - find the previous item and try to derive indentation from it
139-
var precedingListItem = findPrecedingListItem(commaToken);
140-
var precedingListItemStartLineAndChar = sourceFile.getLineAndCharacterFromPosition(precedingListItem.getStart(sourceFile));
141-
var listStart = getStartLineAndCharacterForNode(precedingListItem.parent, sourceFile);
142-
143-
if (precedingListItemStartLineAndChar.line !== listStart.line) {
144-
return findColumnForFirstNonWhitespaceCharacterInLine(precedingListItemStartLineAndChar, sourceFile, options);
145-
}
146-
147-
return -1;
111+
var itemInfo = findPrecedingListItem(commaToken);
112+
return deriveActualIndentationFromList(itemInfo.list.getChildren(), itemInfo.listItemIndex, sourceFile, options);
148113
}
149114

150115
/*
151116
* Function returns -1 if actual indentation for node should not be used (i.e because node is nested expression)
152117
*/
153-
function getActualIndentationForNode(current: Node, parent: Node, currentLineAndChar: LineAndCharacter, parentAndChildShareLine: boolean, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number {
118+
function getActualIndentationForNode(current: Node,
119+
parent: Node,
120+
currentLineAndChar: LineAndCharacter,
121+
parentAndChildShareLine: boolean,
122+
sourceFile: SourceFile,
123+
options: TypeScript.FormattingOptions): number {
124+
125+
// actual indentation is used for statements\declarations if one of cases below is true:
126+
// - parent is SourceFile - by default immediate children of SourceFile are not indented except when user indents them manually
127+
// - parent and child are not on the same line
154128
var useActualIndentation =
155129
(isDeclaration(current) || isStatement(current)) &&
156130
(parent.kind === SyntaxKind.SourceFile || !parentAndChildShareLine);
@@ -162,7 +136,7 @@ module ts.formatting {
162136
return findColumnForFirstNonWhitespaceCharacterInLine(currentLineAndChar, sourceFile, options);
163137
}
164138

165-
function discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken: Node, current: Node, lineAtPosition: number, sourceFile: SourceFile): boolean {
139+
function nextTokenIsCurlyBraceOnSameLineAsCursor(precedingToken: Node, current: Node, lineAtPosition: number, sourceFile: SourceFile): boolean {
166140
var nextToken = findNextToken(precedingToken, current);
167141
if (!nextToken) {
168142
return false;
@@ -193,7 +167,7 @@ module ts.formatting {
193167
return sourceFile.getLineAndCharacterFromPosition(n.getStart(sourceFile));
194168
}
195169

196-
function findPrecedingListItem(commaToken: Node): Node {
170+
function findPrecedingListItem(commaToken: Node): { listItemIndex: number; list: Node } {
197171
// CommaToken node is synthetic and thus will be stored in SyntaxList, however parent of the CommaToken points to the container of the SyntaxList skipping the list.
198172
// In order to find the preceding list item we first need to locate SyntaxList itself and then search for the position of CommaToken
199173
var syntaxList = forEach(commaToken.parent.getChildren(), c => {
@@ -208,7 +182,10 @@ module ts.formatting {
208182
var commaIndex = indexOf(children, commaToken);
209183
Debug.assert(commaIndex !== -1 && commaIndex !== 0);
210184

211-
return children[commaIndex - 1];
185+
return {
186+
listItemIndex: commaIndex - 1,
187+
list: syntaxList
188+
};
212189
}
213190

214191
function positionBelongsToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean {
@@ -228,6 +205,10 @@ module ts.formatting {
228205
function getActualIndentationForListItem(node: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number {
229206
if (node.parent) {
230207
switch (node.parent.kind) {
208+
case SyntaxKind.TypeReference:
209+
if ((<TypeReferenceNode>node.parent).typeArguments) {
210+
return getActualIndentationFromList((<TypeReferenceNode>node.parent).typeArguments);
211+
}
231212
case SyntaxKind.ObjectLiteral:
232213
return getActualIndentationFromList((<ObjectLiteral>node.parent).properties);
233214
case SyntaxKind.TypeLiteral:
@@ -243,40 +224,47 @@ module ts.formatting {
243224
if ((<SignatureDeclaration>node.parent).typeParameters && node.end < (<SignatureDeclaration>node.parent).typeParameters.end) {
244225
return getActualIndentationFromList((<SignatureDeclaration>node.parent).typeParameters);
245226
}
246-
else {
247-
return getActualIndentationFromList((<SignatureDeclaration>node.parent).parameters);
248-
}
227+
228+
return getActualIndentationFromList((<SignatureDeclaration>node.parent).parameters);
249229
case SyntaxKind.NewExpression:
250230
case SyntaxKind.CallExpression:
251231
if ((<CallExpression>node.parent).typeArguments && node.end < (<CallExpression>node.parent).typeArguments.end) {
252232
return getActualIndentationFromList((<CallExpression>node.parent).typeArguments);
253233
}
254-
else {
255-
return getActualIndentationFromList((<CallExpression>node.parent).arguments);
256-
}
257-
258-
break;
234+
235+
return getActualIndentationFromList((<CallExpression>node.parent).arguments);
259236
}
260237
}
261238

262239
return -1;
263240

264241
function getActualIndentationFromList(list: Node[]): number {
265242
var index = indexOf(list, node);
266-
if (index !== -1) {
267-
// walk toward the start of the list starting from current node and check if if line is the same for all items.
268-
// if line for item [i - 1] differs from the line for item [i] - find column of the first non-whitespace character on the line of item [i]
269-
var lineAndCharacter = getStartLineAndCharacterForNode(node, sourceFile);;
270-
for (var i = index - 1; i >= 0; --i) {
271-
var prevLineAndCharacter = getStartLineAndCharacterForNode(list[i], sourceFile);
272-
if (lineAndCharacter.line !== prevLineAndCharacter.line) {
273-
return findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter, sourceFile, options);
274-
}
275-
lineAndCharacter = prevLineAndCharacter;
276-
}
243+
return index !== -1 ? deriveActualIndentationFromList(list, index, sourceFile, options) : -1;
244+
}
245+
}
246+
247+
248+
function deriveActualIndentationFromList(list: Node[], index: number, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number {
249+
Debug.assert(index >= 0 && index < list.length);
250+
var node = list[index];
251+
252+
// walk toward the start of the list starting from current node and check if the line is the same for all items.
253+
// if end line for item [i - 1] differs from the start line for item [i] - find column of the first non-whitespace character on the line of item [i]
254+
var lineAndCharacter = getStartLineAndCharacterForNode(node, sourceFile);
255+
for (var i = index - 1; i >= 0; --i) {
256+
if (list[i].kind === SyntaxKind.CommaToken) {
257+
continue;
277258
}
278-
return -1;
259+
// skip list items that ends on the same line with the current list element
260+
var prevEndLine = sourceFile.getLineAndCharacterFromPosition(list[i].end).line;
261+
if (prevEndLine !== lineAndCharacter.line) {
262+
return findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter, sourceFile, options);
263+
}
264+
265+
lineAndCharacter = getStartLineAndCharacterForNode(list[i], sourceFile);
279266
}
267+
return -1;
280268
}
281269

282270
function findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter: LineAndCharacter, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number {
@@ -317,10 +305,12 @@ module ts.formatting {
317305
// previous token ends exactly at the beginning of child
318306
(child.pos === previousToken.end);
319307

320-
if (shouldDiveInChildNode && isCandidateNode(child)) {
308+
if (shouldDiveInChildNode && nodeHasTokens(child)) {
321309
return find(child);
322310
}
323311
}
312+
313+
return undefined;
324314
}
325315
}
326316

@@ -335,12 +325,12 @@ module ts.formatting {
335325
var children = n.getChildren();
336326
if (diveIntoLastChild) {
337327
var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ children.length);
338-
return candidate && find(candidate, diveIntoLastChild);
328+
return candidate && find(candidate, /*diveIntoLastChild*/ true);
339329
}
340330

341331
for (var i = 0, len = children.length; i < len; ++i) {
342332
var child = children[i];
343-
if (isCandidateNode(child)) {
333+
if (nodeHasTokens(child)) {
344334
if (position < child.end) {
345335
if (child.getStart(sourceFile) >= position) {
346336
// actual start of the node is past the position - previous token should be at the end of previous child
@@ -366,7 +356,7 @@ module ts.formatting {
366356
/// finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition'
367357
function findLastChildNodeCandidate(children: Node[], exclusiveStartPosition: number): Node {
368358
for (var i = exclusiveStartPosition - 1; i >= 0; --i) {
369-
if (isCandidateNode(children[i])) {
359+
if (nodeHasTokens(children[i])) {
370360
return children[i];
371361
}
372362
}
@@ -376,9 +366,9 @@ module ts.formatting {
376366
/*
377367
* Checks if node is something that can contain tokens (except EOF) - filters out EOF tokens, Missing\Omitted expressions, empty SyntaxLists and expression statements that wrap any of listed nodes.
378368
*/
379-
function isCandidateNode(n: Node): boolean {
369+
function nodeHasTokens(n: Node): boolean {
380370
if (n.kind === SyntaxKind.ExpressionStatement) {
381-
return isCandidateNode((<ExpressionStatement>n).expression);
371+
return nodeHasTokens((<ExpressionStatement>n).expression);
382372
}
383373

384374
if (n.kind === SyntaxKind.EndOfFileToken || n.kind === SyntaxKind.OmittedExpression || n.kind === SyntaxKind.Missing) {
@@ -404,7 +394,10 @@ module ts.formatting {
404394
return false;
405395
case SyntaxKind.FunctionDeclaration:
406396
case SyntaxKind.Method:
407-
case SyntaxKind.FunctionExpression:
397+
case SyntaxKind.FunctionExpression:
398+
case SyntaxKind.GetAccessor:
399+
case SyntaxKind.SetAccessor:
400+
case SyntaxKind.Constructor:
408401
// FunctionBlock should take care of indentation
409402
return false;
410403
case SyntaxKind.DoStatement:
@@ -477,6 +470,7 @@ module ts.formatting {
477470
case SyntaxKind.ParenExpression:
478471
case SyntaxKind.CallSignature:
479472
case SyntaxKind.CallExpression:
473+
case SyntaxKind.ConstructSignature:
480474
return nodeEndsWith(n, SyntaxKind.CloseParenToken, sourceFile);
481475
case SyntaxKind.FunctionDeclaration:
482476
case SyntaxKind.FunctionExpression:

‎tests/cases/fourslash/smartIndentListItem.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path='fourslash.ts'/>
2+
////[1,
3+
//// 2
4+
//// + 3, 4,
5+
//// /*1*/
6+
////[1,
7+
//// 2
8+
//// + 3, 4
9+
//// /*2*/
10+
11+
goTo.marker("1");
12+
verify.indentationIs(4)
13+
goTo.marker("2");
14+
verify.indentationIs(4)

‎tests/cases/fourslash/smartIndentTypeArgumentList.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////interface T1 extends T<number,
4+
//// string
5+
//// /**/>
6+
7+
goTo.marker();
8+
verify.indentationIs(32);

0 commit comments

Comments
 (0)