Skip to content

Commit 1227210

Browse files
DanielRosenwasserDanielRosenwasser
committed
Merge pull request #2385 from Microsoft/completeIsCompleteNode
More thorough node completed-ness checking
2 parents a301ee3 + 857d1e0 commit 1227210

27 files changed

+375
-17
lines changed

‎src/harness/fourslash.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,17 +1621,19 @@ module FourSlash {
16211621
this.taoInvalidReason = 'verifyIndentationAtCurrentPosition NYI';
16221622

16231623
var actual = this.getIndentation(this.activeFile.fileName, this.currentCaretPosition);
1624-
if (actual != numberOfSpaces) {
1625-
this.raiseError('verifyIndentationAtCurrentPosition failed - expected: ' + numberOfSpaces + ', actual: ' + actual);
1624+
var lineCol = this.getLineColStringAtPosition(this.currentCaretPosition);
1625+
if (actual !== numberOfSpaces) {
1626+
this.raiseError('verifyIndentationAtCurrentPosition failed at ' + lineCol + ' - expected: ' + numberOfSpaces + ', actual: ' + actual);
16261627
}
16271628
}
16281629

16291630
public verifyIndentationAtPosition(fileName: string, position: number, numberOfSpaces: number) {
16301631
this.taoInvalidReason = 'verifyIndentationAtPosition NYI';
16311632

16321633
var actual = this.getIndentation(fileName, position);
1634+
var lineCol = this.getLineColStringAtPosition(position);
16331635
if (actual !== numberOfSpaces) {
1634-
this.raiseError('verifyIndentationAtPosition failed - expected: ' + numberOfSpaces + ', actual: ' + actual);
1636+
this.raiseError('verifyIndentationAtPosition failed at ' + lineCol + ' - expected: ' + numberOfSpaces + ', actual: ' + actual);
16351637
}
16361638
}
16371639

‎src/services/formatting/smartIndenter.ts

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ module ts.formatting {
359359
case SyntaxKind.ModuleBlock:
360360
case SyntaxKind.ObjectLiteralExpression:
361361
case SyntaxKind.TypeLiteral:
362+
case SyntaxKind.TupleType:
362363
case SyntaxKind.CaseBlock:
363364
case SyntaxKind.DefaultClause:
364365
case SyntaxKind.CaseClause:
@@ -370,6 +371,8 @@ module ts.formatting {
370371
case SyntaxKind.ExportAssignment:
371372
case SyntaxKind.ReturnStatement:
372373
case SyntaxKind.ConditionalExpression:
374+
case SyntaxKind.ArrayBindingPattern:
375+
case SyntaxKind.ObjectBindingPattern:
373376
return true;
374377
}
375378
return false;
@@ -390,6 +393,7 @@ module ts.formatting {
390393
case SyntaxKind.FunctionExpression:
391394
case SyntaxKind.MethodDeclaration:
392395
case SyntaxKind.MethodSignature:
396+
case SyntaxKind.CallSignature:
393397
case SyntaxKind.ArrowFunction:
394398
case SyntaxKind.Constructor:
395399
case SyntaxKind.GetAccessor:
@@ -431,53 +435,93 @@ module ts.formatting {
431435
case SyntaxKind.InterfaceDeclaration:
432436
case SyntaxKind.EnumDeclaration:
433437
case SyntaxKind.ObjectLiteralExpression:
438+
case SyntaxKind.ObjectBindingPattern:
439+
case SyntaxKind.TypeLiteral:
434440
case SyntaxKind.Block:
435441
case SyntaxKind.ModuleBlock:
436442
case SyntaxKind.CaseBlock:
437443
return nodeEndsWith(n, SyntaxKind.CloseBraceToken, sourceFile);
438444
case SyntaxKind.CatchClause:
439445
return isCompletedNode((<CatchClause>n).block, sourceFile);
440-
case SyntaxKind.ParenthesizedExpression:
441-
case SyntaxKind.CallSignature:
446+
case SyntaxKind.NewExpression:
447+
if (!(<NewExpression>n).arguments) {
448+
return true;
449+
}
450+
// fall through
442451
case SyntaxKind.CallExpression:
443-
case SyntaxKind.ConstructSignature:
452+
case SyntaxKind.ParenthesizedExpression:
453+
case SyntaxKind.ParenthesizedType:
444454
return nodeEndsWith(n, SyntaxKind.CloseParenToken, sourceFile);
455+
456+
case SyntaxKind.FunctionType:
457+
case SyntaxKind.ConstructorType:
458+
return isCompletedNode((<SignatureDeclaration>n).type, sourceFile);
459+
460+
case SyntaxKind.Constructor:
461+
case SyntaxKind.GetAccessor:
462+
case SyntaxKind.SetAccessor:
445463
case SyntaxKind.FunctionDeclaration:
446464
case SyntaxKind.FunctionExpression:
447465
case SyntaxKind.MethodDeclaration:
448466
case SyntaxKind.MethodSignature:
467+
case SyntaxKind.ConstructSignature:
468+
case SyntaxKind.CallSignature:
449469
case SyntaxKind.ArrowFunction:
450-
return !(<FunctionLikeDeclaration>n).body || isCompletedNode((<FunctionLikeDeclaration>n).body, sourceFile);
470+
if ((<FunctionLikeDeclaration>n).body) {
471+
return isCompletedNode((<FunctionLikeDeclaration>n).body, sourceFile);
472+
}
473+
474+
if ((<FunctionLikeDeclaration>n).type) {
475+
return isCompletedNode((<FunctionLikeDeclaration>n).type, sourceFile);
476+
}
477+
478+
// Even though type parameters can be unclosed, we can get away with
479+
// having at least a closing paren.
480+
return hasChildOfKind(n, SyntaxKind.CloseParenToken, sourceFile);
481+
451482
case SyntaxKind.ModuleDeclaration:
452483
return (<ModuleDeclaration>n).body && isCompletedNode((<ModuleDeclaration>n).body, sourceFile);
484+
453485
case SyntaxKind.IfStatement:
454486
if ((<IfStatement>n).elseStatement) {
455487
return isCompletedNode((<IfStatement>n).elseStatement, sourceFile);
456488
}
457489
return isCompletedNode((<IfStatement>n).thenStatement, sourceFile);
490+
458491
case SyntaxKind.ExpressionStatement:
459492
return isCompletedNode((<ExpressionStatement>n).expression, sourceFile);
493+
460494
case SyntaxKind.ArrayLiteralExpression:
495+
case SyntaxKind.ArrayBindingPattern:
496+
case SyntaxKind.ComputedPropertyName:
497+
case SyntaxKind.TupleType:
461498
return nodeEndsWith(n, SyntaxKind.CloseBracketToken, sourceFile);
499+
500+
case SyntaxKind.IndexSignature:
501+
if ((<IndexSignatureDeclaration>n).type) {
502+
return isCompletedNode((<IndexSignatureDeclaration>n).type, sourceFile);
503+
}
504+
505+
return hasChildOfKind(n, SyntaxKind.CloseBracketToken, sourceFile);
506+
462507
case SyntaxKind.CaseClause:
463508
case SyntaxKind.DefaultClause:
464-
// there is no such thing as terminator token for CaseClause\DefaultClause so for simplicitly always consider them non-completed
509+
// there is no such thing as terminator token for CaseClause/DefaultClause so for simplicitly always consider them non-completed
465510
return false;
511+
466512
case SyntaxKind.ForStatement:
467-
return isCompletedNode((<ForStatement>n).statement, sourceFile);
468513
case SyntaxKind.ForInStatement:
469-
return isCompletedNode((<ForInStatement>n).statement, sourceFile);
470514
case SyntaxKind.ForOfStatement:
471-
return isCompletedNode((<ForOfStatement>n).statement, sourceFile);
472515
case SyntaxKind.WhileStatement:
473-
return isCompletedNode((<WhileStatement>n).statement, sourceFile);
516+
return isCompletedNode((<IterationStatement>n).statement, sourceFile);
474517
case SyntaxKind.DoStatement:
475518
// rough approximation: if DoStatement has While keyword - then if node is completed is checking the presence of ')';
476519
let hasWhileKeyword = findChildOfKind(n, SyntaxKind.WhileKeyword, sourceFile);
477520
if (hasWhileKeyword) {
478521
return nodeEndsWith(n, SyntaxKind.CloseParenToken, sourceFile);
479522
}
480523
return isCompletedNode((<DoStatement>n).statement, sourceFile);
524+
481525
default:
482526
return true;
483527
}

‎src/services/utilities.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ module ts {
7979
};
8080
}
8181

82+
export function hasChildOfKind(n: Node, kind: SyntaxKind, sourceFile?: SourceFile): boolean {
83+
return !!findChildOfKind(n, kind, sourceFile);
84+
}
85+
8286
export function findChildOfKind(n: Node, kind: SyntaxKind, sourceFile?: SourceFile): Node {
8387
return forEach(n.getChildren(sourceFile), c => c.kind === kind && c);
8488
}

‎tests/cases/fourslash/indentation.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@
176176
////// the purpose of this test is to verity smart indent
177177
////// works for unterminated function arguments at the end of a file.
178178
////function unterminatedListIndentation(a,
179-
////{| "indent": 0 |}
179+
////{| "indent": 4 |}
180180

181-
test.markers().forEach((marker) => {
182-
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
183-
});
181+
test.markers().forEach(marker => {
182+
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
183+
});

‎tests/cases/fourslash/smartIndentAfterNewExpression.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////
4+
////new Array
5+
////{| "indent": 0 |}
6+
////new Array;
7+
////{| "indent": 0 |}
8+
////new Array(0);
9+
////{| "indent": 0 |}
10+
////new Array(;
11+
////{| "indent": 0 |}
12+
////new Array(
13+
////{| "indent": 4 |}
14+
15+
test.markers().forEach(marker => {
16+
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
17+
});

‎tests/cases/fourslash/smartIndentArrayBindingPattern01.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+
3+
////var /*1*/[/*2*/a,/*3*/b,/*4*/
4+
5+
function verifyIndentationAfterNewLine(marker: string, indentation: number): void {
6+
goTo.marker(marker);
7+
edit.insert("\r\n");
8+
verify.indentationIs(indentation);
9+
}
10+
11+
verifyIndentationAfterNewLine("1", 4);
12+
verifyIndentationAfterNewLine("2", 8);
13+
verifyIndentationAfterNewLine("3", 8);
14+
verifyIndentationAfterNewLine("4", 8);

‎tests/cases/fourslash/smartIndentArrayBindingPattern02.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////var /*1*/[/*2*/a,/*3*/b/*4*/]/*5*/
4+
5+
function verifyIndentationAfterNewLine(marker: string, indentation: number): void {
6+
goTo.marker(marker);
7+
edit.insert("\r\n");
8+
verify.indentationIs(indentation);
9+
}
10+
11+
verifyIndentationAfterNewLine("1", 4);
12+
verifyIndentationAfterNewLine("2", 8);
13+
verifyIndentationAfterNewLine("3", 8);
14+
verifyIndentationAfterNewLine("4", 8);
15+
verifyIndentationAfterNewLine("5", 0);

‎tests/cases/fourslash/smartIndentInParenthesizedExpression01.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////var x = (/*1*/1/*2*/)/*3*/
4+
5+
function verifyIndentationAfterNewLine(marker: string, indentation: number): void {
6+
goTo.marker(marker);
7+
edit.insert("\r\n");
8+
verify.indentationIs(indentation);
9+
}
10+
11+
verifyIndentationAfterNewLine("1", 4);
12+
verifyIndentationAfterNewLine("2", 4);
13+
verifyIndentationAfterNewLine("3", 0);

‎tests/cases/fourslash/smartIndentInParenthesizedExpression02.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+
////var y = (
4+
////{| "indent": 4 |}
5+
6+
test.markers().forEach(marker => {
7+
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
8+
});

‎tests/cases/fourslash/smartIndentNonterminatedArgumentListAtEOF.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
/////**/
55

66
goTo.marker();
7-
verify.indentationIs(0);
7+
verify.indentationIs(4);

‎tests/cases/fourslash/smartIndentObjectBindingPattern01.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////var /*1*/{/*2*/a,/*3*/b:/*4*/k,/*5*/
4+
5+
function verifyIndentationAfterNewLine(marker: string, indentation: number): void {
6+
goTo.marker(marker);
7+
edit.insert("\r\n");
8+
verify.indentationIs(indentation);
9+
}
10+
11+
verifyIndentationAfterNewLine("1", 4);
12+
verifyIndentationAfterNewLine("2", 8);
13+
verifyIndentationAfterNewLine("3", 8);
14+
verifyIndentationAfterNewLine("4", 8);
15+
verifyIndentationAfterNewLine("5", 8);

‎tests/cases/fourslash/smartIndentObjectBindingPattern02.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////var /*1*/{/*2*/a,/*3*/b:/*4*/k,/*5*/}/*6*/
4+
5+
function verifyIndentationAfterNewLine(marker: string, indentation: number): void {
6+
goTo.marker(marker);
7+
edit.insert("\r\n");
8+
verify.indentationIs(indentation);
9+
}
10+
11+
verifyIndentationAfterNewLine("1", 4);
12+
verifyIndentationAfterNewLine("2", 8);
13+
verifyIndentationAfterNewLine("3", 8);
14+
verifyIndentationAfterNewLine("4", 8);
15+
verifyIndentationAfterNewLine("5", 8);
16+
verifyIndentationAfterNewLine("6", 0);

‎tests/cases/fourslash/smartIndentOnAccessors.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////class Foo {
4+
//// get foo(a,
5+
//// /*1*/b,/*0*/
6+
//// //comment/*2*/
7+
//// /*3*/c
8+
//// ) {
9+
//// }
10+
//// set foo(a,
11+
//// /*5*/b,/*4*/
12+
//// //comment/*6*/
13+
//// /*7*/c
14+
//// ) {
15+
//// }
16+
////}
17+
18+
19+
goTo.marker("0");
20+
edit.insert("\r\n");
21+
verify.indentationIs(8);
22+
goTo.marker("1");
23+
verify.currentLineContentIs(" b,");
24+
goTo.marker("2");
25+
verify.currentLineContentIs(" //comment");
26+
goTo.marker("3");
27+
verify.currentLineContentIs(" c");
28+
goTo.marker("4");
29+
edit.insert("\r\n");
30+
verify.indentationIs(8);
31+
goTo.marker("5");
32+
verify.currentLineContentIs(" b,");
33+
goTo.marker("6");
34+
verify.currentLineContentIs(" //comment");
35+
goTo.marker("7");
36+
verify.currentLineContentIs(" c");

‎tests/cases/fourslash/smartIndentOnAccessors01.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////class Foo {
4+
//// get foo(a,
5+
//// /*1*/b,/*0*/
6+
//// //comment/*2*/
7+
//// /*3*/c
8+
//// ) {
9+
//// }
10+
//// set foo(a,
11+
//// /*5*/b,/*4*/
12+
//// //comment/*6*/
13+
//// /*7*/c
14+
//// ) {
15+
//// }
16+
////}
17+
18+
19+
goTo.marker("0");
20+
edit.insert("\r\n");
21+
verify.indentationIs(8);
22+
goTo.marker("1");
23+
verify.currentLineContentIs(" b,");
24+
goTo.marker("2");
25+
verify.currentLineContentIs(" //comment");
26+
goTo.marker("3");
27+
verify.currentLineContentIs(" c");
28+
goTo.marker("4");
29+
edit.insert("\r\n");
30+
verify.indentationIs(8);
31+
goTo.marker("5");
32+
verify.currentLineContentIs(" b,");
33+
goTo.marker("6");
34+
verify.currentLineContentIs(" //comment");
35+
goTo.marker("7");
36+
verify.currentLineContentIs(" c");

‎tests/cases/fourslash/smartIndentOnAccessors02.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////class Foo {
4+
//// get foo() {
5+
////{| "indent": 8 |}
6+
7+
test.markers().forEach(marker => {
8+
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
9+
});

‎tests/cases/fourslash/smartIndentOnUnclosedArrowType01.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+
////var x: () => {
4+
////{| "indent": 4 |}
5+
6+
test.markers().forEach(marker => {
7+
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
8+
});

‎tests/cases/fourslash/smartIndentOnUnclosedComputedProperty01.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////var x = {
4+
//// [1123123123132
5+
////{| "indent": 4 |}
6+
////}
7+
8+
// Note that we currently do NOT indent further in a computed property.
9+
test.markers().forEach(marker => {
10+
verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indent);
11+
});

0 commit comments

Comments
 (0)