Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,14 @@ module ts {
return skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos);
}

export function getNonDecoratorTokenPosOfNode(node: Node, sourceFile?: SourceFile): number {
if (nodeIsMissing(node) || !node.decorators) {
return getTokenPosOfNode(node, sourceFile);
}

return skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.decorators.end);
}

export function getSourceTextOfNodeFromSourceFile(sourceFile: SourceFile, node: Node): string {
if (nodeIsMissing(node)) {
return "";
Expand Down
26 changes: 19 additions & 7 deletions src/services/formatting/formatting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,13 @@ module ts.formatting {

if (formattingScanner.isOnToken()) {
let startLine = sourceFile.getLineAndCharacterOfPosition(enclosingNode.getStart(sourceFile)).line;
let undecoratedStartLine = startLine;
if (enclosingNode.decorators) {
undecoratedStartLine = sourceFile.getLineAndCharacterOfPosition(getNonDecoratorTokenPosOfNode(enclosingNode, sourceFile)).line;
}

let delta = getOwnOrInheritedDelta(enclosingNode, options, sourceFile);
processNode(enclosingNode, enclosingNode, startLine, initialIndentation, delta);
processNode(enclosingNode, enclosingNode, startLine, undecoratedStartLine, initialIndentation, delta);
}

formattingScanner.close();
Expand Down Expand Up @@ -500,7 +505,7 @@ module ts.formatting {
}
}

function processNode(node: Node, contextNode: Node, nodeStartLine: number, indentation: number, delta: number) {
function processNode(node: Node, contextNode: Node, nodeStartLine: number, undecoratedNodeStartLine: number, indentation: number, delta: number) {
if (!rangeOverlapsWithStartEnd(originalRange, node.getStart(sourceFile), node.getEnd())) {
return;
}
Expand All @@ -526,7 +531,7 @@ module ts.formatting {
forEachChild(
node,
child => {
processChildNode(child, /*inheritedIndentation*/ Constants.Unknown, node, nodeDynamicIndentation, nodeStartLine, /*isListElement*/ false)
processChildNode(child, /*inheritedIndentation*/ Constants.Unknown, node, nodeDynamicIndentation, nodeStartLine, undecoratedNodeStartLine, /*isListElement*/ false)
},
(nodes: NodeArray<Node>) => {
processChildNodes(nodes, node, nodeStartLine, nodeDynamicIndentation);
Expand All @@ -547,11 +552,17 @@ module ts.formatting {
parent: Node,
parentDynamicIndentation: DynamicIndentation,
parentStartLine: number,
undecoratedParentStartLine: number,
isListItem: boolean): number {

let childStartPos = child.getStart(sourceFile);

let childStart = sourceFile.getLineAndCharacterOfPosition(childStartPos);
let childStartLine = sourceFile.getLineAndCharacterOfPosition(childStartPos).line;

let undecoratedChildStartLine = childStartLine;
if (child.decorators) {
undecoratedChildStartLine = sourceFile.getLineAndCharacterOfPosition(getNonDecoratorTokenPosOfNode(child, sourceFile)).line;
}

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

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

processNode(child, childContextNode, childStart.line, childIndentation.indentation, childIndentation.delta);
processNode(child, childContextNode, childStartLine, undecoratedChildStartLine, childIndentation.indentation, childIndentation.delta);

childContextNode = node;

Expand Down Expand Up @@ -640,7 +652,7 @@ module ts.formatting {

let inheritedIndentation = Constants.Unknown;
for (let child of nodes) {
inheritedIndentation = processChildNode(child, inheritedIndentation, node, listDynamicIndentation, startLine, /*isListElement*/ true)
inheritedIndentation = processChildNode(child, inheritedIndentation, node, listDynamicIndentation, startLine, startLine, /*isListElement*/ true)
}

if (listEndToken !== SyntaxKind.Unknown) {
Expand Down
29 changes: 28 additions & 1 deletion src/services/formatting/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,11 @@ module ts.formatting {
public SpaceAfterAnonymousFunctionKeyword: Rule;
public NoSpaceAfterAnonymousFunctionKeyword: Rule;

// Insert space after @ in decorator
public SpaceBeforeAt: Rule;
public NoSpaceAfterAt: Rule;
public SpaceAfterDecorator: Rule;

constructor() {
///
/// Common Rules
Expand Down Expand Up @@ -344,6 +349,11 @@ module ts.formatting {
// Remove spaces in empty interface literals. e.g.: x: {}
this.NoSpaceBetweenEmptyInterfaceBraceBrackets = new Rule(RuleDescriptor.create1(SyntaxKind.OpenBraceToken, SyntaxKind.CloseBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsSameLineTokenContext, Rules.IsObjectTypeContext), RuleAction.Delete));

// decorators
this.SpaceBeforeAt = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.AtToken), RuleOperation.create2(new RuleOperationContext(Rules.IsSameLineTokenContext), RuleAction.Space));
this.NoSpaceAfterAt = new Rule(RuleDescriptor.create3(SyntaxKind.AtToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsSameLineTokenContext), RuleAction.Delete));
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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for space after decorator should not be added

@x[""]
class c {
}

or

@decorator(arg) 
class  {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would add "function" as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhegazy, can you clarify what you mean by this:

i would add "function" as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhegazy, I'm not sure what you're trying illustrate with this:

Can you add a test for space after decorator should not be added

@x[""]
class c {
}

or

@decorator(arg) 
class  {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i meant functionKeyword. @dec function func() {} thought we can add that when we support it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i meant the sapceAfterDecorator rule would not apply to decorators expressions.


// These rules are higher in priority than user-configurable rules.
this.HighPriorityCommonRules =
[
Expand Down Expand Up @@ -381,7 +391,10 @@ module ts.formatting {
this.NoSpaceBetweenCloseParenAndAngularBracket,
this.NoSpaceAfterOpenAngularBracket,
this.NoSpaceBeforeCloseAngularBracket,
this.NoSpaceAfterCloseAngularBracket
this.NoSpaceAfterCloseAngularBracket,
this.SpaceBeforeAt,
this.NoSpaceAfterAt,
this.SpaceAfterDecorator,
];

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

static IsEndOfDecoratorContextOnSameLine(context: FormattingContext): boolean {
return context.TokensAreOnSameLine() &&
context.contextNode.decorators &&
Rules.NodeIsInDecoratorContext(context.currentTokenParent) &&
!Rules.NodeIsInDecoratorContext(context.nextTokenParent);
}

static NodeIsInDecoratorContext(node: Node): boolean {
while (isExpression(node)) {
node = node.parent;
}
return node.kind === SyntaxKind.Decorator;
}

static IsStartOfVariableDeclarationList(context: FormattingContext): boolean {
return context.currentTokenParent.kind === SyntaxKind.VariableDeclarationList &&
context.currentTokenParent.getStart(context.sourceFile) === context.currentTokenSpan.pos;
Expand Down
106 changes: 106 additions & 0 deletions tests/cases/fourslash/formattingDecorators.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/// <reference path='fourslash.ts' />

/////*1*/ @ decorator1
/////*2*/ @ decorator2
/////*3*/ @decorator3
/////*4*/ @ decorator4 @ decorator5
/////*5*/class C {
/////*6*/ @ decorator6
/////*7*/ @ decorator7
/////*8*/ @decorator8
/////*9*/ method1() { }
////
/////*10*/ @ decorator9 @ decorator10 @decorator11 method2() { }
////
//// method3(
/////*11*/ @ decorator12
/////*12*/ @ decorator13
/////*13*/ @decorator14
/////*14*/ x) { }
////
//// method4(
/////*15*/ @ decorator15 @ decorator16 @decorator17 x) { }
////
/////*16*/ @ decorator18
/////*17*/ @ decorator19
/////*18*/ @decorator20
/////*19*/ ["computed1"]() { }
////
/////*20*/ @ decorator21 @ decorator22 @decorator23 ["computed2"]() { }
////
/////*21*/ @ decorator24
/////*22*/ @ decorator25
/////*23*/ @decorator26
/////*24*/ get accessor1() { }
////
/////*25*/ @ decorator27 @ decorator28 @decorator29 get accessor2() { }
////
/////*26*/ @ decorator30
/////*27*/ @ decorator31
/////*28*/ @decorator32
/////*29*/ property1;
////
/////*30*/ @ decorator33 @ decorator34 @decorator35 property2;
////}

format.document();
goTo.marker("1");
verify.currentLineContentIs("@decorator1");
goTo.marker("2");
verify.currentLineContentIs("@decorator2");
goTo.marker("3");
verify.currentLineContentIs("@decorator3");
goTo.marker("4");
verify.currentLineContentIs("@decorator4 @decorator5");
goTo.marker("5");
verify.currentLineContentIs("class C {");
goTo.marker("6");
verify.currentLineContentIs(" @decorator6");
goTo.marker("7");
verify.currentLineContentIs(" @decorator7");
goTo.marker("8");
verify.currentLineContentIs(" @decorator8");
goTo.marker("9");
verify.currentLineContentIs(" method1() { }");
goTo.marker("10");
verify.currentLineContentIs(" @decorator9 @decorator10 @decorator11 method2() { }");
goTo.marker("11");
verify.currentLineContentIs(" @decorator12");
goTo.marker("12");
verify.currentLineContentIs(" @decorator13");
goTo.marker("13");
verify.currentLineContentIs(" @decorator14");
goTo.marker("14");
verify.currentLineContentIs(" x) { }");
goTo.marker("15");
verify.currentLineContentIs(" @decorator15 @decorator16 @decorator17 x) { }");
goTo.marker("16");
verify.currentLineContentIs(" @decorator18");
goTo.marker("17");
verify.currentLineContentIs(" @decorator19");
goTo.marker("18");
verify.currentLineContentIs(" @decorator20");
goTo.marker("19");
verify.currentLineContentIs(" [\"computed1\"]() { }");
goTo.marker("20");
verify.currentLineContentIs(" @decorator21 @decorator22 @decorator23 [\"computed2\"]() { }");
goTo.marker("21");
verify.currentLineContentIs(" @decorator24");
goTo.marker("22");
verify.currentLineContentIs(" @decorator25");
goTo.marker("23");
verify.currentLineContentIs(" @decorator26");
goTo.marker("24");
verify.currentLineContentIs(" get accessor1() { }");
goTo.marker("25");
verify.currentLineContentIs(" @decorator27 @decorator28 @decorator29 get accessor2() { }");
goTo.marker("26");
verify.currentLineContentIs(" @decorator30");
goTo.marker("27");
verify.currentLineContentIs(" @decorator31");
goTo.marker("28");
verify.currentLineContentIs(" @decorator32");
goTo.marker("29");
verify.currentLineContentIs(" property1;");
goTo.marker("30");
verify.currentLineContentIs(" @decorator33 @decorator34 @decorator35 property2;");