Skip to content

Commit f318515

Browse files
rbucktonrbuckton
committed
Merge pull request #2664 from Microsoft/fixDecoratorFormatting
Fixes some formatting for decorators
2 parents 01d945b + 609036a commit f318515

File tree

4 files changed

+161
-8
lines changed

4 files changed

+161
-8
lines changed

‎src/compiler/utilities.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,14 @@ module ts {
160160
return skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos);
161161
}
162162

163+
export function getNonDecoratorTokenPosOfNode(node: Node, sourceFile?: SourceFile): number {
164+
if (nodeIsMissing(node) || !node.decorators) {
165+
return getTokenPosOfNode(node, sourceFile);
166+
}
167+
168+
return skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.decorators.end);
169+
}
170+
163171
export function getSourceTextOfNodeFromSourceFile(sourceFile: SourceFile, node: Node): string {
164172
if (nodeIsMissing(node)) {
165173
return "";

‎src/services/formatting/formatting.ts‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,13 @@ module ts.formatting {
328328

329329
if (formattingScanner.isOnToken()) {
330330
let startLine = sourceFile.getLineAndCharacterOfPosition(enclosingNode.getStart(sourceFile)).line;
331+
let undecoratedStartLine = startLine;
332+
if (enclosingNode.decorators) {
333+
undecoratedStartLine = sourceFile.getLineAndCharacterOfPosition(getNonDecoratorTokenPosOfNode(enclosingNode, sourceFile)).line;
334+
}
335+
331336
let delta = getOwnOrInheritedDelta(enclosingNode, options, sourceFile);
332-
processNode(enclosingNode, enclosingNode, startLine, initialIndentation, delta);
337+
processNode(enclosingNode, enclosingNode, startLine, undecoratedStartLine, initialIndentation, delta);
333338
}
334339

335340
formattingScanner.close();
@@ -500,7 +505,7 @@ module ts.formatting {
500505
}
501506
}
502507

503-
function processNode(node: Node, contextNode: Node, nodeStartLine: number, indentation: number, delta: number) {
508+
function processNode(node: Node, contextNode: Node, nodeStartLine: number, undecoratedNodeStartLine: number, indentation: number, delta: number) {
504509
if (!rangeOverlapsWithStartEnd(originalRange, node.getStart(sourceFile), node.getEnd())) {
505510
return;
506511
}
@@ -526,7 +531,7 @@ module ts.formatting {
526531
forEachChild(
527532
node,
528533
child => {
529-
processChildNode(child, /*inheritedIndentation*/ Constants.Unknown, node, nodeDynamicIndentation, nodeStartLine, /*isListElement*/ false)
534+
processChildNode(child, /*inheritedIndentation*/ Constants.Unknown, node, nodeDynamicIndentation, nodeStartLine, undecoratedNodeStartLine, /*isListElement*/ false)
530535
},
531536
(nodes: NodeArray<Node>) => {
532537
processChildNodes(nodes, node, nodeStartLine, nodeDynamicIndentation);
@@ -547,11 +552,17 @@ module ts.formatting {
547552
parent: Node,
548553
parentDynamicIndentation: DynamicIndentation,
549554
parentStartLine: number,
555+
undecoratedParentStartLine: number,
550556
isListItem: boolean): number {
551557

552558
let childStartPos = child.getStart(sourceFile);
553559

554-
let childStart = sourceFile.getLineAndCharacterOfPosition(childStartPos);
560+
let childStartLine = sourceFile.getLineAndCharacterOfPosition(childStartPos).line;
561+
562+
let undecoratedChildStartLine = childStartLine;
563+
if (child.decorators) {
564+
undecoratedChildStartLine = sourceFile.getLineAndCharacterOfPosition(getNonDecoratorTokenPosOfNode(child, sourceFile)).line;
565+
}
555566

556567
// if child is a list item - try to get its indentation
557568
let childIndentationAmount = Constants.Unknown;
@@ -594,9 +605,10 @@ module ts.formatting {
594605
return inheritedIndentation;
595606
}
596607

597-
let childIndentation = computeIndentation(child, childStart.line, childIndentationAmount, node, parentDynamicIndentation, parentStartLine);
608+
let effectiveParentStartLine = child.kind === SyntaxKind.Decorator ? childStartLine : undecoratedParentStartLine;
609+
let childIndentation = computeIndentation(child, childStartLine, childIndentationAmount, node, parentDynamicIndentation, effectiveParentStartLine);
598610

599-
processNode(child, childContextNode, childStart.line, childIndentation.indentation, childIndentation.delta);
611+
processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta);
600612

601613
childContextNode = node;
602614

@@ -640,7 +652,7 @@ module ts.formatting {
640652

641653
let inheritedIndentation = Constants.Unknown;
642654
for (let child of nodes) {
643-
inheritedIndentation = processChildNode(child, inheritedIndentation, node, listDynamicIndentation, startLine, /*isListElement*/ true)
655+
inheritedIndentation = processChildNode(child, inheritedIndentation, node, listDynamicIndentation, startLine, startLine, /*isListElement*/ true)
644656
}
645657

646658
if (listEndToken !== SyntaxKind.Unknown) {

‎src/services/formatting/rules.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,11 @@ module ts.formatting {
208208
public SpaceAfterAnonymousFunctionKeyword: Rule;
209209
public NoSpaceAfterAnonymousFunctionKeyword: Rule;
210210

211+
// Insert space after @ in decorator
212+
public SpaceBeforeAt: Rule;
213+
public NoSpaceAfterAt: Rule;
214+
public SpaceAfterDecorator: Rule;
215+
211216
constructor() {
212217
///
213218
/// Common Rules
@@ -344,6 +349,11 @@ module ts.formatting {
344349
// Remove spaces in empty interface literals. e.g.: x: {}
345350
this.NoSpaceBetweenEmptyInterfaceBraceBrackets = new Rule(RuleDescriptor.create1(SyntaxKind.OpenBraceToken, SyntaxKind.CloseBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsSameLineTokenContext, Rules.IsObjectTypeContext), RuleAction.Delete));
346351

352+
// decorators
353+
this.SpaceBeforeAt = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.AtToken), RuleOperation.create2(new RuleOperationContext(Rules.IsSameLineTokenContext), RuleAction.Space));
354+
this.NoSpaceAfterAt = new Rule(RuleDescriptor.create3(SyntaxKind.AtToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsSameLineTokenContext), RuleAction.Delete));
355+
this.SpaceAfterDecorator = new Rule(RuleDescriptor.create4(Shared.TokenRange.Any, Shared.TokenRange.FromTokens([SyntaxKind.Identifier, SyntaxKind.ExportKeyword, SyntaxKind.DefaultKeyword, SyntaxKind.ClassKeyword, SyntaxKind.StaticKeyword, SyntaxKind.PublicKeyword, SyntaxKind.PrivateKeyword, SyntaxKind.ProtectedKeyword, SyntaxKind.GetKeyword, SyntaxKind.SetKeyword, SyntaxKind.OpenBracketToken, SyntaxKind.AsteriskToken])), RuleOperation.create2(new RuleOperationContext(Rules.IsEndOfDecoratorContextOnSameLine), RuleAction.Space));
356+
347357
// These rules are higher in priority than user-configurable rules.
348358
this.HighPriorityCommonRules =
349359
[
@@ -381,7 +391,10 @@ module ts.formatting {
381391
this.NoSpaceBetweenCloseParenAndAngularBracket,
382392
this.NoSpaceAfterOpenAngularBracket,
383393
this.NoSpaceBeforeCloseAngularBracket,
384-
this.NoSpaceAfterCloseAngularBracket
394+
this.NoSpaceAfterCloseAngularBracket,
395+
this.SpaceBeforeAt,
396+
this.NoSpaceAfterAt,
397+
this.SpaceAfterDecorator,
385398
];
386399

387400
// These rules are lower in priority than user-configurable rules.
@@ -649,6 +662,20 @@ module ts.formatting {
649662
return context.TokensAreOnSameLine();
650663
}
651664

665+
static IsEndOfDecoratorContextOnSameLine(context: FormattingContext): boolean {
666+
return context.TokensAreOnSameLine() &&
667+
context.contextNode.decorators &&
668+
Rules.NodeIsInDecoratorContext(context.currentTokenParent) &&
669+
!Rules.NodeIsInDecoratorContext(context.nextTokenParent);
670+
}
671+
672+
static NodeIsInDecoratorContext(node: Node): boolean {
673+
while (isExpression(node)) {
674+
node = node.parent;
675+
}
676+
return node.kind === SyntaxKind.Decorator;
677+
}
678+
652679
static IsStartOfVariableDeclarationList(context: FormattingContext): boolean {
653680
return context.currentTokenParent.kind === SyntaxKind.VariableDeclarationList &&
654681
context.currentTokenParent.getStart(context.sourceFile) === context.currentTokenSpan.pos;

‎tests/cases/fourslash/formattingDecorators.ts‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
/////*1*/ @ decorator1
4+
/////*2*/ @ decorator2
5+
/////*3*/ @decorator3
6+
/////*4*/ @ decorator4 @ decorator5
7+
/////*5*/class C {
8+
/////*6*/ @ decorator6
9+
/////*7*/ @ decorator7
10+
/////*8*/ @decorator8
11+
/////*9*/ method1() { }
12+
////
13+
/////*10*/ @ decorator9 @ decorator10 @decorator11 method2() { }
14+
////
15+
//// method3(
16+
/////*11*/ @ decorator12
17+
/////*12*/ @ decorator13
18+
/////*13*/ @decorator14
19+
/////*14*/ x) { }
20+
////
21+
//// method4(
22+
/////*15*/ @ decorator15 @ decorator16 @decorator17 x) { }
23+
////
24+
/////*16*/ @ decorator18
25+
/////*17*/ @ decorator19
26+
/////*18*/ @decorator20
27+
/////*19*/ ["computed1"]() { }
28+
////
29+
/////*20*/ @ decorator21 @ decorator22 @decorator23 ["computed2"]() { }
30+
////
31+
/////*21*/ @ decorator24
32+
/////*22*/ @ decorator25
33+
/////*23*/ @decorator26
34+
/////*24*/ get accessor1() { }
35+
////
36+
/////*25*/ @ decorator27 @ decorator28 @decorator29 get accessor2() { }
37+
////
38+
/////*26*/ @ decorator30
39+
/////*27*/ @ decorator31
40+
/////*28*/ @decorator32
41+
/////*29*/ property1;
42+
////
43+
/////*30*/ @ decorator33 @ decorator34 @decorator35 property2;
44+
////}
45+
46+
format.document();
47+
goTo.marker("1");
48+
verify.currentLineContentIs("@decorator1");
49+
goTo.marker("2");
50+
verify.currentLineContentIs("@decorator2");
51+
goTo.marker("3");
52+
verify.currentLineContentIs("@decorator3");
53+
goTo.marker("4");
54+
verify.currentLineContentIs("@decorator4 @decorator5");
55+
goTo.marker("5");
56+
verify.currentLineContentIs("class C {");
57+
goTo.marker("6");
58+
verify.currentLineContentIs(" @decorator6");
59+
goTo.marker("7");
60+
verify.currentLineContentIs(" @decorator7");
61+
goTo.marker("8");
62+
verify.currentLineContentIs(" @decorator8");
63+
goTo.marker("9");
64+
verify.currentLineContentIs(" method1() { }");
65+
goTo.marker("10");
66+
verify.currentLineContentIs(" @decorator9 @decorator10 @decorator11 method2() { }");
67+
goTo.marker("11");
68+
verify.currentLineContentIs(" @decorator12");
69+
goTo.marker("12");
70+
verify.currentLineContentIs(" @decorator13");
71+
goTo.marker("13");
72+
verify.currentLineContentIs(" @decorator14");
73+
goTo.marker("14");
74+
verify.currentLineContentIs(" x) { }");
75+
goTo.marker("15");
76+
verify.currentLineContentIs(" @decorator15 @decorator16 @decorator17 x) { }");
77+
goTo.marker("16");
78+
verify.currentLineContentIs(" @decorator18");
79+
goTo.marker("17");
80+
verify.currentLineContentIs(" @decorator19");
81+
goTo.marker("18");
82+
verify.currentLineContentIs(" @decorator20");
83+
goTo.marker("19");
84+
verify.currentLineContentIs(" [\"computed1\"]() { }");
85+
goTo.marker("20");
86+
verify.currentLineContentIs(" @decorator21 @decorator22 @decorator23 [\"computed2\"]() { }");
87+
goTo.marker("21");
88+
verify.currentLineContentIs(" @decorator24");
89+
goTo.marker("22");
90+
verify.currentLineContentIs(" @decorator25");
91+
goTo.marker("23");
92+
verify.currentLineContentIs(" @decorator26");
93+
goTo.marker("24");
94+
verify.currentLineContentIs(" get accessor1() { }");
95+
goTo.marker("25");
96+
verify.currentLineContentIs(" @decorator27 @decorator28 @decorator29 get accessor2() { }");
97+
goTo.marker("26");
98+
verify.currentLineContentIs(" @decorator30");
99+
goTo.marker("27");
100+
verify.currentLineContentIs(" @decorator31");
101+
goTo.marker("28");
102+
verify.currentLineContentIs(" @decorator32");
103+
goTo.marker("29");
104+
verify.currentLineContentIs(" property1;");
105+
goTo.marker("30");
106+
verify.currentLineContentIs(" @decorator33 @decorator34 @decorator35 property2;");

0 commit comments

Comments
 (0)