From 24c40bb4bca66a12749473186874f4cc08b5c77b Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Wed, 10 Sep 2014 23:13:41 -0700 Subject: [PATCH 01/16] added optional 'sourceFile' argument to methods of Node to avoid unnecesary tree walk --- src/compiler/parser.ts | 4 +-- src/services/services.ts | 62 ++++++++++++++++++++-------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index cfe27e7e7c..7e5ac13b8d 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -50,8 +50,8 @@ module ts { return node.pos; } - export function getTokenPosOfNode(node: Node): number { - return skipTrivia(getSourceFileOfNode(node).text, node.pos); + export function getTokenPosOfNode(node: Node, sourceFile?: SourceFile): number { + return skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos); } export function getSourceTextOfNodeFromSourceText(sourceText: string, node: Node): string { diff --git a/src/services/services.ts b/src/services/services.ts index 0069f53922..740bfb899b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -27,18 +27,18 @@ module ts { export interface Node { getSourceFile(): SourceFile; - getChildCount(): number; - getChildAt(index: number): Node; - getChildren(): Node[]; - getStart(): number; + getChildCount(sourceFile?: SourceFile): number; + getChildAt(index: number, sourceFile?: SourceFile): Node; + getChildren(sourceFile?: SourceFile): Node[]; + getStart(sourceFile?: SourceFile): number; getFullStart(): number; getEnd(): number; - getWidth(): number; + getWidth(sourceFile?: SourceFile): number; getFullWidth(): number; - getLeadingTriviaWidth(): number; - getFullText(): string; - getFirstToken(): Node; - getLastToken(): Node; + getLeadingTriviaWidth(sourceFile?: SourceFile): number; + getFullText(sourceFile?: SourceFile): string; + getFirstToken(sourceFile?: SourceFile): Node; + getLastToken(sourceFile?: SourceFile): Node; } export interface Symbol { @@ -100,8 +100,8 @@ module ts { return node; } - public getStart(): number { - return getTokenPosOfNode(this); + public getStart(sourceFile?: SourceFile): number { + return getTokenPosOfNode(this, sourceFile); } public getFullStart(): number { @@ -112,20 +112,20 @@ module ts { return this.end; } - public getWidth(): number { - return this.getEnd() - this.getStart(); + public getWidth(sourceFile?: SourceFile): number { + return this.getEnd() - this.getStart(sourceFile); } public getFullWidth(): number { return this.end - this.getFullStart(); } - public getLeadingTriviaWidth(): number { - return this.getStart() - this.pos; + public getLeadingTriviaWidth(sourceFile?: SourceFile): number { + return this.getStart(sourceFile) - this.pos; } - public getFullText(): string { - return this.getSourceFile().text.substring(this.pos, this.end); + public getFullText(sourceFile?: SourceFile): string { + return (sourceFile || this.getSourceFile()).text.substring(this.pos, this.end); } private addSyntheticNodes(nodes: Node[], pos: number, end: number): number { @@ -157,9 +157,9 @@ module ts { return list; } - private createChildren() { + private createChildren(sourceFile?: SourceFile) { if (this.kind > SyntaxKind.Missing) { - scanner.setText(this.getSourceFile().text); + scanner.setText((sourceFile || this.getSourceFile()).text); var children: Node[] = []; var pos = this.pos; var processNode = (node: Node) => { @@ -185,36 +185,36 @@ module ts { this._children = children || emptyArray; } - public getChildCount(): number { - if (!this._children) this.createChildren(); + public getChildCount(sourceFile?: SourceFile): number { + if (!this._children) this.createChildren(sourceFile); return this._children.length; } - public getChildAt(index: number): Node { - if (!this._children) this.createChildren(); + public getChildAt(index: number, sourceFile?: SourceFile): Node { + if (!this._children) this.createChildren(sourceFile); return this._children[index]; } - public getChildren(): Node[] { - if (!this._children) this.createChildren(); + public getChildren(sourceFile?: SourceFile): Node[] { + if (!this._children) this.createChildren(sourceFile); return this._children; } - public getFirstToken(): Node { - var children = this.getChildren(); + public getFirstToken(sourceFile?: SourceFile): Node { + var children = this.getChildren(sourceFile); for (var i = 0; i < children.length; i++) { var child = children[i]; if (child.kind < SyntaxKind.Missing) return child; - if (child.kind > SyntaxKind.Missing) return child.getFirstToken(); + if (child.kind > SyntaxKind.Missing) return child.getFirstToken(sourceFile); } } - public getLastToken(): Node { - var children = this.getChildren(); + public getLastToken(sourceFile?: SourceFile): Node { + var children = this.getChildren(sourceFile); for (var i = children.length - 1; i >= 0; i--) { var child = children[i]; if (child.kind < SyntaxKind.Missing) return child; - if (child.kind > SyntaxKind.Missing) return child.getLastToken(); + if (child.kind > SyntaxKind.Missing) return child.getLastToken(sourceFile); } } } From 641e659ffd61a15c4538c9e8bec0f2e0e69e36b7 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 11 Sep 2014 00:35:03 -0700 Subject: [PATCH 02/16] initial revision of SmartIndenter (TODO: indentation in list items) --- Jakefile | 5 +- src/services/formatting/smartIndenter.ts | 383 +++++++++++++++++++++++ src/services/services.ts | 11 +- 3 files changed, 390 insertions(+), 9 deletions(-) create mode 100644 src/services/formatting/smartIndenter.ts diff --git a/Jakefile b/Jakefile index 79907e8a63..97b1bc826a 100644 --- a/Jakefile +++ b/Jakefile @@ -54,6 +54,7 @@ var servicesSources = [ }).concat([ "services.ts", "shims.ts", + "formatting\\smartIndenter.ts" ].map(function (f) { return path.join(servicesDirectory, f); })); @@ -318,7 +319,7 @@ function exec(cmd, completeHandler) { complete(); }) try{ - ex.run(); + ex.run(); } catch(e) { console.log('Exception: ' + e) } @@ -385,7 +386,7 @@ desc("Generates code coverage data via instanbul") task("generate-code-coverage", ["tests", builtLocalDirectory], function () { var cmd = 'istanbul cover node_modules/mocha/bin/_mocha -- -R min -t ' + testTimeout + ' ' + run; console.log(cmd); - exec(cmd); + exec(cmd); }, { async: true }); // Browser tests diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts new file mode 100644 index 0000000000..82bbaad81b --- /dev/null +++ b/src/services/formatting/smartIndenter.ts @@ -0,0 +1,383 @@ +/// + +module ts.formatting { + export module SmartIndenter { + export function getIndentation(position: number, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + if (position > sourceFile.text.length) { + return 0; // past EOF + } + + var precedingToken = findPrecedingToken(position, sourceFile); + if (!precedingToken) { + return 0; + } + + // no indentation in string \regex literals + if ((precedingToken.kind === SyntaxKind.StringLiteral || precedingToken.kind === SyntaxKind.RegularExpressionLiteral) && + precedingToken.getStart(sourceFile) <= position && + precedingToken.end > position) { + return 0; + } + + // try to find the node that will include 'position' starting from 'precedingToken' + // if such node is found - compute initial indentation for 'position' inside this node + var previous: Node; + var current = precedingToken; + var indentation: number; + while (current) { + if (!isToken(current) && isPositionBelongToNode(current, position, sourceFile)) { + indentation = getInitialIndentationInNode(position, current, previous, sourceFile, options); + break; + } + + previous = current; + current = current.parent; + } + + if (!current) { + return 0; + } + + var currentStartLine: number = sourceFile.getLineAndCharacterFromPosition(current.getStart(sourceFile)).line; + var parent: Node = current.parent; + var parentStartLine: number; + + // walk upwards and collect indentations for pairs of parent-child nodes + // 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' + while (parent) { + parentStartLine = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)).line; + if (isNodeContentIndented(parent, current) && parentStartLine !== currentStartLine && !isChildOnTheSameLineWithElseInIfStatement(parent, current, sourceFile)) { + indentation += options.indentSpaces; + } + + current = parent; + currentStartLine = parentStartLine; + parent = current.parent; + } + + return indentation; + } + + function getInitialIndentationInNode(position: number, parent: Node, previous: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + if (parent.kind === SyntaxKind.IfStatement) { + Debug.assert(previous); + + // IfStatement will be parent when: + // - previous token is the immediate child of IfStatement - some token + // - Then\Else parts are completed and position is outside Then\Else statements + if ((parent).thenStatement === previous || (parent).elseStatement === previous) { + if (previous.getStart(sourceFile) > position || previous.end < position) { + return 0; + } + else { + return indentIfPositionOnDifferentLineWithNodeStart(position, previous, sourceFile, options); + } + } + } + else if (parent.kind === SyntaxKind.TryStatement) { + Debug.assert(previous); + // this is possible only if position is next to completed Try\Catch blocks - no indentation + if (previous.kind === SyntaxKind.TryBlock || previous.kind === SyntaxKind.CatchBlock) { + return 0; + } + } + + return indentIfPositionOnDifferentLineWithNodeStart(position, parent, sourceFile, options); + } + + function indentIfPositionOnDifferentLineWithNodeStart(position: number, node: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + return isPositionOnTheSameLineWithNodeStart(position, node, sourceFile) ? 0 : options.indentSpaces; + } + + function isPositionOnTheSameLineWithNodeStart(position: number, node: Node, sourceFile: SourceFile): boolean { + var lineAtPosition = sourceFile.getLineAndCharacterFromPosition(position).line; + var startLine = sourceFile.getLineAndCharacterFromPosition(node.getStart(sourceFile)).line; + return lineAtPosition === startLine; + } + + function isPositionBelongToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean { + return candidate.end > position || !isCompletedNode(candidate, sourceFile); + } + + function isPositionOnTheSameLineWithSomeBrace(token: Node, position: number, sourceFile: SourceFile): boolean { + switch (token.kind) { + case SyntaxKind.OpenBraceToken: + case SyntaxKind.OpenBracketToken: + case SyntaxKind.OpenParenToken: + case SyntaxKind.CloseBraceToken: + case SyntaxKind.CloseBracketToken: + case SyntaxKind.CloseParenToken: + return isPositionOnTheSameLineWithNodeStart(position, token, sourceFile); + default: + return false; + } + } + + function isChildOnTheSameLineWithElseInIfStatement(parent: Node, child: Node, sourceFile: SourceFile): boolean { + if (parent.kind === SyntaxKind.IfStatement && (parent).elseStatement === child) { + var elseKeyword = findTokenOfKind(parent, SyntaxKind.ElseKeyword); + Debug.assert(elseKeyword); + + return isPositionOnTheSameLineWithNodeStart(child.getStart(sourceFile), elseKeyword, sourceFile); + } + } + + function findTokenOfKind(parent: Node, kind: SyntaxKind) { + return forEach(parent.getChildren(), c => c.kind === kind && c); + } + + // preserve indentation for list items + // - first list item is either on the same line with the parent: foo(a... . (in this case it is not indented) or on the different line (then it is indented with base level + delta) + // - subsequent list items inherit indentation for its sibling on the left when these siblings are also on the new line. + // 1. foo(a, b + // $ - indentation = base level + delta + // 2. foo (a, + // b, c, d, + // $ - same indentation with first child node on the previous line + // NOTE: indentation for list items spans from the beginning of the line to the first non-whitespace character + // /*test*/ x, + // $ <-- indentation for a new item will be here + function getCustomIndentationForListItem(leftSibling: Node, sourceFile: SourceFile): number { + if (leftSibling.parent) { + switch (leftSibling.parent.kind) { + case SyntaxKind.ObjectLiteral: + return getCustomIndentationFromList((leftSibling.parent).properties); + case SyntaxKind.TypeLiteral: + return getCustomIndentationFromList((leftSibling.parent).members); + case SyntaxKind.ArrayLiteral: + return getCustomIndentationFromList((leftSibling.parent).elements); + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.FunctionExpression: + case SyntaxKind.ArrowFunction: + case SyntaxKind.Method: + case SyntaxKind.CallSignature: + case SyntaxKind.ConstructSignature: + if ((leftSibling.parent).typeParameters && leftSibling.end < (leftSibling.parent).typeParameters.end) { + return getCustomIndentationFromList((leftSibling.parent).typeParameters); + } + else { + return getCustomIndentationFromList((leftSibling.parent).parameters); + } + case SyntaxKind.NewExpression: + case SyntaxKind.CallExpression: + if ((leftSibling.parent).typeArguments && leftSibling.end < (leftSibling.parent).typeArguments.end) { + return getCustomIndentationFromList((leftSibling.parent).typeArguments); + } + else { + return getCustomIndentationFromList((leftSibling.parent).arguments); + } + + break; + } + } + + return -1; + + function getCustomIndentationFromList(list: Node[]): number { + var index = indexOf(list, leftSibling); + if (index !== -1) { + var lineAndCol = sourceFile.getLineAndCharacterFromPosition(leftSibling.getStart(sourceFile)); + for (var i = index - 1; i >= 0; --i) { + var prevLineAndCol = sourceFile.getLineAndCharacterFromPosition(list[i].getStart(sourceFile)); + if (lineAndCol.line !== prevLineAndCol.line) { + // find the line start position + var lineStart = sourceFile.getPositionFromLineAndCharacter(lineAndCol.line, 1); + for (var i = 0; i <= lineAndCol.character; ++i) { + if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStart + i))) { + return i; + } + } + // code is unreachable because the rance that we check above includes at least one non-whitespace character at the very end + Debug.fail("Unreachable code") + + } + lineAndCol = prevLineAndCol; + } + } + return -1; + } + } + + function findPrecedingToken(position: number, sourceFile: SourceFile): Node { + return find(sourceFile, /*diveIntoLastChild*/ false); + + function find(n: Node, diveIntoLastChild: boolean): Node { + if (isToken(n)) { + return n; + } + + var children = n.getChildren(); + if (diveIntoLastChild) { + var candidate = findLastChildNodeCandidate(children, children.length); + return candidate && find(candidate, diveIntoLastChild); + } + + for (var i = 0, len = children.length; i < len; ++i) { + var child = children[i]; + if (isCandidateNode(child)) { + if (position < child.end) { + if (child.getStart(sourceFile) >= position) { + // actual start of the node is past the position - previous token should be at the end of previous child + var candidate = findLastChildNodeCandidate(children, i); + return candidate && find(candidate, /*diveIntoLastChild*/ true) + } + else { + // candidate should be in this node + return find(child, diveIntoLastChild); + } + } + } + } + + // here we know that none of child token nodes embrace the position + // try to find the closest token on the left + if (children.length) { + var candidate = findLastChildNodeCandidate(children, children.length); + return candidate && find(candidate, /*diveIntoLastChild*/ true); + } + } + + // filters out EOF tokens, Missing\Omitted expressions, empty SyntaxLists and expression statements that wrap any of listed nodes. + function isCandidateNode(n: Node): boolean { + if (n.kind === SyntaxKind.ExpressionStatement) { + return isCandidateNode((n).expression); + } + + if (n.kind === SyntaxKind.EndOfFileToken || n.kind === SyntaxKind.OmittedExpression || n.kind === SyntaxKind.Missing) { + return false; + } + + // SyntaxList is already realized so getChildCount should be fast and non-expensive + return n.kind !== SyntaxKind.SyntaxList || n.getChildCount() !== 0; + } + + // finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition' + function findLastChildNodeCandidate(children: Node[], exclusiveStartPosition: number): Node { + for (var i = exclusiveStartPosition - 1; i >= 0; --i) { + if (isCandidateNode(children[i])) { + return children[i]; + } + } + } + } + + function isToken(n: Node): boolean { + return n.kind < SyntaxKind.Missing; + } + + function isNodeContentIndented(parent: Node, child: Node): boolean { + switch (parent.kind) { + case SyntaxKind.ClassDeclaration: + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.EnumDeclaration: + return true; + case SyntaxKind.ModuleDeclaration: + // ModuleBlock should take care of indentation + return false; + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.Method: + case SyntaxKind.FunctionExpression: + // FunctionBlock should take care of indentation + return false; + case SyntaxKind.DoStatement: + case SyntaxKind.WhileStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.ForStatement: + return child && child.kind !== SyntaxKind.Block; + case SyntaxKind.IfStatement: + return child && child.kind !== SyntaxKind.Block; + case SyntaxKind.TryStatement: + // TryBlock\CatchBlock\FinallyBlock should take care of indentation + return false; + case SyntaxKind.ArrayLiteral: + case SyntaxKind.Block: + case SyntaxKind.FunctionBlock: + case SyntaxKind.TryBlock: + case SyntaxKind.CatchBlock: + case SyntaxKind.FinallyBlock: + case SyntaxKind.ModuleBlock: + case SyntaxKind.ObjectLiteral: + case SyntaxKind.TypeLiteral: + case SyntaxKind.SwitchStatement: + case SyntaxKind.DefaultClause: + case SyntaxKind.CaseClause: + case SyntaxKind.ParenExpression: + case SyntaxKind.BinaryExpression: + case SyntaxKind.CallExpression: + case SyntaxKind.NewExpression: + return true; + default: + return false; + } + } + + function isNodeEndWith(n: Node, expectedLastToken: SyntaxKind, sourceFile: SourceFile): boolean { + var children = n.getChildren(sourceFile); + if (children.length) { + var last = children[children.length - 1]; + if (last.kind === expectedLastToken) { + return true; + } + else if (last.kind === SyntaxKind.SemicolonToken && children.length !== 1) { + return children[children.length - 2].kind === expectedLastToken; + } + } + return false; + } + + function isCompletedNode(n: Node, sourceFile: SourceFile): boolean { + switch (n.kind) { + case SyntaxKind.ClassDeclaration: + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.EnumDeclaration: + case SyntaxKind.ObjectLiteral: + case SyntaxKind.Block: + case SyntaxKind.CatchBlock: + case SyntaxKind.FinallyBlock: + case SyntaxKind.FunctionBlock: + case SyntaxKind.ModuleBlock: + case SyntaxKind.SwitchStatement: + return isNodeEndWith(n, SyntaxKind.CloseBraceToken, sourceFile); + case SyntaxKind.ParenExpression: + case SyntaxKind.CallSignature: + case SyntaxKind.CallExpression: + return isNodeEndWith(n, SyntaxKind.CloseParenToken, sourceFile); + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.FunctionExpression: + case SyntaxKind.Method: + case SyntaxKind.ArrowFunction: + return !(n).body || isCompletedNode((n).body, sourceFile); + case SyntaxKind.ModuleDeclaration: + return (n).body && isCompletedNode((n).body, sourceFile); + case SyntaxKind.IfStatement: + if ((n).elseStatement) { + return isCompletedNode((n).elseStatement, sourceFile); + } + return isCompletedNode((n).thenStatement, sourceFile); + case SyntaxKind.ExpressionStatement: + return isCompletedNode((n).expression, sourceFile); + case SyntaxKind.ArrayLiteral: + return isNodeEndWith(n, SyntaxKind.CloseBracketToken, sourceFile); + case SyntaxKind.Missing: + return false; + case SyntaxKind.CaseClause: + case SyntaxKind.DefaultClause: + // there is no such thing as terminator token for CaseClause\DefaultClause so for simplicitly always consider them non-completed + return false; + case SyntaxKind.VariableStatement: + // variable statement is considered completed if it either doesn'not have variable declarations or last variable declaration is completed + var variableDeclarations = (n).declarations; + return variableDeclarations.length === 0 || isCompletedNode(variableDeclarations[variableDeclarations.length - 1], sourceFile); + case SyntaxKind.VariableDeclaration: + // variable declaration is completed if it either doesn't have initializer or initializer is completed + return !(n).initializer || isCompletedNode((n).initializer, sourceFile); + case SyntaxKind.WhileStatement: + return isCompletedNode((n).statement, sourceFile); + case SyntaxKind.DoStatement: + return isCompletedNode((n).statement, sourceFile); + default: + return true; + } + } + } +} \ No newline at end of file diff --git a/src/services/services.ts b/src/services/services.ts index 740bfb899b..d7edb4349b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -11,6 +11,7 @@ /// /// /// +/// /// /// @@ -2932,15 +2933,11 @@ module ts { function getIndentationAtPosition(filename: string, position: number, editorOptions: EditorOptions) { filename = TypeScript.switchToForwardSlashes(filename); - - var syntaxTree = getSyntaxTree(filename); - - var scriptSnapshot = syntaxTreeCache.getCurrentScriptSnapshot(filename); - var scriptText = TypeScript.SimpleText.fromScriptSnapshot(scriptSnapshot); - var textSnapshot = new TypeScript.Services.Formatting.TextSnapshot(scriptText); + + var sourceFile = getCurrentSourceFile(filename); var options = new TypeScript.FormattingOptions(!editorOptions.ConvertTabsToSpaces, editorOptions.TabSize, editorOptions.IndentSize, editorOptions.NewLineCharacter) - return TypeScript.Services.Formatting.SingleTokenIndenter.getIndentationAmount(position, syntaxTree.sourceUnit(), textSnapshot, options); + return formatting.SmartIndenter.getIndentation(position, sourceFile, options); } function getFormattingManager(filename: string, options: FormatCodeOptions) { From 563b92cdce7887eb12b3826b8b1907d44bd9f54a Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 11 Sep 2014 10:54:30 -0700 Subject: [PATCH 03/16] update test baselines --- .../fourslash/noSmartIndentInsideMultilineString.ts | 3 +-- .../fourslash/smartIndentInsideMultilineString.ts | 10 +++------- tests/cases/fourslash/smartIndentStatementSwitch.ts | 2 +- tests/cases/fourslash/switchIndenting.ts | 4 +--- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/tests/cases/fourslash/noSmartIndentInsideMultilineString.ts b/tests/cases/fourslash/noSmartIndentInsideMultilineString.ts index 9018a54b33..692dfe7075 100644 --- a/tests/cases/fourslash/noSmartIndentInsideMultilineString.ts +++ b/tests/cases/fourslash/noSmartIndentInsideMultilineString.ts @@ -8,5 +8,4 @@ goTo.marker("1"); edit.insert("\r\n"); -// Won't-fixed: Disable SmartIndent inside multiline string in TS. Should be 0 -verify.indentationIs(8); \ No newline at end of file +verify.indentationIs(0); \ No newline at end of file diff --git a/tests/cases/fourslash/smartIndentInsideMultilineString.ts b/tests/cases/fourslash/smartIndentInsideMultilineString.ts index 8ad4430455..24a20844a9 100644 --- a/tests/cases/fourslash/smartIndentInsideMultilineString.ts +++ b/tests/cases/fourslash/smartIndentInsideMultilineString.ts @@ -21,15 +21,11 @@ //// } ////} -// Behavior below is not ideal, ideal is in comments goTo.marker("1"); -//verify.indentationIs(0); -verify.indentationIs(8); +verify.indentationIs(0); goTo.marker("2"); -//verify.indentationIs(0); -verify.indentationIs(4); +verify.indentationIs(0); goTo.marker("3"); -//verify.indentationIs(0); -verify.indentationIs(12); \ No newline at end of file +verify.indentationIs(0); \ No newline at end of file diff --git a/tests/cases/fourslash/smartIndentStatementSwitch.ts b/tests/cases/fourslash/smartIndentStatementSwitch.ts index 902abe4e3e..e8d2a02948 100644 --- a/tests/cases/fourslash/smartIndentStatementSwitch.ts +++ b/tests/cases/fourslash/smartIndentStatementSwitch.ts @@ -11,7 +11,7 @@ //// case 1: //// {| "indentation": 12 |} //// break; -//// {| "indentation": 8 |} +//// {| "indentation": 12 |} // content of case clauses is always indented relatively to case clause //// } //// {| "indentation": 4 |} ////} diff --git a/tests/cases/fourslash/switchIndenting.ts b/tests/cases/fourslash/switchIndenting.ts index 3fee7ba2e4..8ea6f26408 100644 --- a/tests/cases/fourslash/switchIndenting.ts +++ b/tests/cases/fourslash/switchIndenting.ts @@ -8,6 +8,4 @@ goTo.marker(); edit.insert('case 1:\n'); -// ideally would be 8 -//verify.indentationIs(8); -verify.indentationIs(4); +verify.indentationIs(8); \ No newline at end of file From 9a3462a1616a434067482d54939715a165978b76 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 11 Sep 2014 16:17:31 -0700 Subject: [PATCH 04/16] added handling for smart indentation in the beginning of list items, updated test baselines. TODO: smart indentation in the middle of list items (2 failing tests) --- src/services/formatting/smartIndenter.ts | 211 ++++++++++++------ tests/cases/fourslash/indentation.ts | 2 +- ...smartIndentAfterAlignedFunctionArgument.ts | 5 +- .../cases/fourslash/smartIndentCloseBrace.ts | 11 + .../cases/fourslash/smartIndentDoStatement.ts | 17 ++ .../cases/fourslash/smartIndentIfStatement.ts | 20 ++ .../smartIndentOnFunctionParameters.ts | 2 +- 7 files changed, 193 insertions(+), 75 deletions(-) create mode 100644 tests/cases/fourslash/smartIndentCloseBrace.ts create mode 100644 tests/cases/fourslash/smartIndentDoStatement.ts create mode 100644 tests/cases/fourslash/smartIndentIfStatement.ts diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 82bbaad81b..f6958ebbc5 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -19,26 +19,66 @@ module ts.formatting { return 0; } + var lineAtPosition = sourceFile.getLineAndCharacterFromPosition(position).line; + + if (precedingToken.kind === SyntaxKind.CommaToken && precedingToken.parent.kind !== SyntaxKind.BinaryExpression) { + + // previous token is comma that separates items in list - find the previous item and try to derive indentation from it + var precedingListItem = findPrecedingListItem(precedingToken); + var precedingListItemStartLineAndChar = sourceFile.getLineAndCharacterFromPosition(precedingListItem.getStart(sourceFile)); + var listStartLine = getStartLineForNode(precedingListItem.parent, sourceFile); + + if (precedingListItemStartLineAndChar.line !== listStartLine) { + + // previous list item starts on the different line with list, find first non-whitespace character in this line and use its position as indentation + var lineStartPosition = sourceFile.getPositionFromLineAndCharacter(precedingListItemStartLineAndChar.line, 1); + for (var i = 0; i < precedingListItemStartLineAndChar.character; ++i) { + if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStartPosition + i))) { + return i; + } + } + + // seems that this is the first non-whitespace character on the line - return it + return precedingListItemStartLineAndChar.character; + } + } + // try to find the node that will include 'position' starting from 'precedingToken' // if such node is found - compute initial indentation for 'position' inside this node var previous: Node; var current = precedingToken; + var currentStartLine: number; var indentation: number; + while (current) { if (!isToken(current) && isPositionBelongToNode(current, position, sourceFile)) { - indentation = getInitialIndentationInNode(position, current, previous, sourceFile, options); + + currentStartLine = getStartLineForNode(current, sourceFile); + + if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) { + indentation = 0; + } + else { + indentation = + isNodeContentIndented(current, previous) && + lineAtPosition !== currentStartLine + ? options.indentSpaces + : 0; + } + break; } previous = current; current = current.parent; } - + if (!current) { + // no parent was found - return 0 to be indented on the level of SourceFile return 0; } - var currentStartLine: number = sourceFile.getLineAndCharacterFromPosition(current.getStart(sourceFile)).line; + var parent: Node = current.parent; var parentStartLine: number; @@ -46,7 +86,12 @@ module ts.formatting { // 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' while (parent) { parentStartLine = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)).line; - if (isNodeContentIndented(parent, current) && parentStartLine !== currentStartLine && !isChildOnTheSameLineWithElseInIfStatement(parent, current, sourceFile)) { + var increaseIndentation = + isNodeContentIndented(parent, current) && + parentStartLine !== currentStartLine && + !isChildStartsOnTheSameLineWithElseInIfStatement(parent, current, currentStartLine, sourceFile); + + if (increaseIndentation) { indentation += options.indentSpaces; } @@ -58,74 +103,69 @@ module ts.formatting { return indentation; } - function getInitialIndentationInNode(position: number, parent: Node, previous: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { - if (parent.kind === SyntaxKind.IfStatement) { - Debug.assert(previous); - - // IfStatement will be parent when: - // - previous token is the immediate child of IfStatement - some token - // - Then\Else parts are completed and position is outside Then\Else statements - if ((parent).thenStatement === previous || (parent).elseStatement === previous) { - if (previous.getStart(sourceFile) > position || previous.end < position) { - return 0; - } - else { - return indentIfPositionOnDifferentLineWithNodeStart(position, previous, sourceFile, options); - } - } + function discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken: Node, current: Node, lineAtPosition: number, sourceFile: SourceFile): boolean { + var nextToken = findNextToken(precedingToken, current); + if (!nextToken) { + return false; } - else if (parent.kind === SyntaxKind.TryStatement) { - Debug.assert(previous); - // this is possible only if position is next to completed Try\Catch blocks - no indentation - if (previous.kind === SyntaxKind.TryBlock || previous.kind === SyntaxKind.CatchBlock) { - return 0; - } + + if (nextToken.kind === SyntaxKind.OpenBraceToken) { + // open braces are always indented at the parent level + return true; + } + else if (nextToken.kind === SyntaxKind.CloseBraceToken) { + // close braces are indented at the parent level if they are located on the same line with cursor + // this means that if new line will be added at $ position, this case will be indented + // class A { + // $ + // } + /// and this one - not + // class A { + // $} + + var nextTokenStartLine = getStartLineForNode(nextToken, sourceFile); + return lineAtPosition === nextTokenStartLine; } - return indentIfPositionOnDifferentLineWithNodeStart(position, parent, sourceFile, options); + return false; } - function indentIfPositionOnDifferentLineWithNodeStart(position: number, node: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { - return isPositionOnTheSameLineWithNodeStart(position, node, sourceFile) ? 0 : options.indentSpaces; + function getStartLineForNode(n: Node, sourceFile: SourceFile): number { + return sourceFile.getLineAndCharacterFromPosition(n.getStart(sourceFile)).line; } - function isPositionOnTheSameLineWithNodeStart(position: number, node: Node, sourceFile: SourceFile): boolean { - var lineAtPosition = sourceFile.getLineAndCharacterFromPosition(position).line; - var startLine = sourceFile.getLineAndCharacterFromPosition(node.getStart(sourceFile)).line; - return lineAtPosition === startLine; + function findPrecedingListItem(commaToken: Node): Node { + // 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. + // In order to find the preceding list item we first need to locate SyntaxList itself and then search for the position of CommaToken + var syntaxList = forEach(commaToken.parent.getChildren(), c => { + // find syntax list that covers the span of CommaToken + if (c.kind == SyntaxKind.SyntaxList && c.pos <= commaToken.end && c.end >= commaToken.end) { + return c; + } + }); + Debug.assert(syntaxList); + + var children = syntaxList.getChildren(); + var commaIndex = indexOf(children, commaToken); + Debug.assert(commaIndex !== -1 && commaIndex !== 0); + + return children[commaIndex - 1]; } function isPositionBelongToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean { return candidate.end > position || !isCompletedNode(candidate, sourceFile); } - function isPositionOnTheSameLineWithSomeBrace(token: Node, position: number, sourceFile: SourceFile): boolean { - switch (token.kind) { - case SyntaxKind.OpenBraceToken: - case SyntaxKind.OpenBracketToken: - case SyntaxKind.OpenParenToken: - case SyntaxKind.CloseBraceToken: - case SyntaxKind.CloseBracketToken: - case SyntaxKind.CloseParenToken: - return isPositionOnTheSameLineWithNodeStart(position, token, sourceFile); - default: - return false; - } - } - - function isChildOnTheSameLineWithElseInIfStatement(parent: Node, child: Node, sourceFile: SourceFile): boolean { + function isChildStartsOnTheSameLineWithElseInIfStatement(parent: Node, child: Node, childStartLine: number, sourceFile: SourceFile): boolean { if (parent.kind === SyntaxKind.IfStatement && (parent).elseStatement === child) { - var elseKeyword = findTokenOfKind(parent, SyntaxKind.ElseKeyword); + var elseKeyword = forEach(parent.getChildren(), c => c.kind === SyntaxKind.ElseKeyword && c); Debug.assert(elseKeyword); - return isPositionOnTheSameLineWithNodeStart(child.getStart(sourceFile), elseKeyword, sourceFile); + var elseKeywordStartLine = getStartLineForNode(elseKeyword, sourceFile); + return elseKeywordStartLine === childStartLine; } } - function findTokenOfKind(parent: Node, kind: SyntaxKind) { - return forEach(parent.getChildren(), c => c.kind === kind && c); - } - // preserve indentation for list items // - first list item is either on the same line with the parent: foo(a... . (in this case it is not indented) or on the different line (then it is indented with base level + delta) // - subsequent list items inherit indentation for its sibling on the left when these siblings are also on the new line. @@ -198,6 +238,31 @@ module ts.formatting { } } + function findNextToken(previousToken: Node, parent: Node): Node { + return find(parent); + + function find(n: Node): Node { + if (isToken(n) && n.pos === previousToken.end) { + // this is token that starts at the end of previous token - return it + return n; + } + + var children = n.getChildren(); + for (var i = 0, len = children.length; i < len; ++i) { + var child = children[i]; + var shouldDiveInChildNode = + // previous token is enclosed somewhere in the child + (child.pos <= previousToken.pos && child.end > previousToken.end) || + // previous token end exactly at the beginning of child + (child.pos === previousToken.end); + + if (shouldDiveInChildNode && isCandidateNode(child)) { + return find(child); + } + } + } + } + function findPrecedingToken(position: number, sourceFile: SourceFile): Node { return find(sourceFile, /*diveIntoLastChild*/ false); @@ -208,7 +273,7 @@ module ts.formatting { var children = n.getChildren(); if (diveIntoLastChild) { - var candidate = findLastChildNodeCandidate(children, children.length); + var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ children.length); return candidate && find(candidate, diveIntoLastChild); } @@ -218,7 +283,7 @@ module ts.formatting { if (position < child.end) { if (child.getStart(sourceFile) >= position) { // actual start of the node is past the position - previous token should be at the end of previous child - var candidate = findLastChildNodeCandidate(children, i); + var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ i); return candidate && find(candidate, /*diveIntoLastChild*/ true) } else { @@ -232,26 +297,12 @@ module ts.formatting { // here we know that none of child token nodes embrace the position // try to find the closest token on the left if (children.length) { - var candidate = findLastChildNodeCandidate(children, children.length); + var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ children.length); return candidate && find(candidate, /*diveIntoLastChild*/ true); } } - // filters out EOF tokens, Missing\Omitted expressions, empty SyntaxLists and expression statements that wrap any of listed nodes. - function isCandidateNode(n: Node): boolean { - if (n.kind === SyntaxKind.ExpressionStatement) { - return isCandidateNode((n).expression); - } - - if (n.kind === SyntaxKind.EndOfFileToken || n.kind === SyntaxKind.OmittedExpression || n.kind === SyntaxKind.Missing) { - return false; - } - - // SyntaxList is already realized so getChildCount should be fast and non-expensive - return n.kind !== SyntaxKind.SyntaxList || n.getChildCount() !== 0; - } - - // finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition' + /// finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition' function findLastChildNodeCandidate(children: Node[], exclusiveStartPosition: number): Node { for (var i = exclusiveStartPosition - 1; i >= 0; --i) { if (isCandidateNode(children[i])) { @@ -261,6 +312,20 @@ module ts.formatting { } } + /// 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. + function isCandidateNode(n: Node): boolean { + if (n.kind === SyntaxKind.ExpressionStatement) { + return isCandidateNode((n).expression); + } + + if (n.kind === SyntaxKind.EndOfFileToken || n.kind === SyntaxKind.OmittedExpression || n.kind === SyntaxKind.Missing) { + return false; + } + + // SyntaxList is already realized so getChildCount should be fast and non-expensive + return n.kind !== SyntaxKind.SyntaxList || n.getChildCount() !== 0; + } + function isToken(n: Node): boolean { return n.kind < SyntaxKind.Missing; } @@ -305,12 +370,16 @@ module ts.formatting { case SyntaxKind.BinaryExpression: case SyntaxKind.CallExpression: case SyntaxKind.NewExpression: + case SyntaxKind.VariableStatement: + case SyntaxKind.VariableDeclaration: return true; default: return false; } } + /// checks if node ends with 'expectedLastToken'. + /// If child at position 'length - 1' is 'SemicolonToken' it is skipped and 'expectedLastToken' is compared with child at position 'length - 2'. function isNodeEndWith(n: Node, expectedLastToken: SyntaxKind, sourceFile: SourceFile): boolean { var children = n.getChildren(sourceFile); if (children.length) { diff --git a/tests/cases/fourslash/indentation.ts b/tests/cases/fourslash/indentation.ts index 3ff6dac45a..32319ae282 100644 --- a/tests/cases/fourslash/indentation.ts +++ b/tests/cases/fourslash/indentation.ts @@ -154,7 +154,7 @@ //// ////function argumentsListIndentation(bar, //// blah, -////{| "indent": 0 |} +//// {| "indent": 13 |} ////); //// //// diff --git a/tests/cases/fourslash/smartIndentAfterAlignedFunctionArgument.ts b/tests/cases/fourslash/smartIndentAfterAlignedFunctionArgument.ts index 8efd5a9c9b..d5aa2bc431 100644 --- a/tests/cases/fourslash/smartIndentAfterAlignedFunctionArgument.ts +++ b/tests/cases/fourslash/smartIndentAfterAlignedFunctionArgument.ts @@ -1,9 +1,10 @@ /// ////function foo(bar, -//// blah, +//// blah, baz, //// /**/ ////) { }; goTo.marker(); -verify.indentationIs(0); +// keep indentation of 'blah' +verify.indentationIs(13); diff --git a/tests/cases/fourslash/smartIndentCloseBrace.ts b/tests/cases/fourslash/smartIndentCloseBrace.ts new file mode 100644 index 0000000000..da2094d76c --- /dev/null +++ b/tests/cases/fourslash/smartIndentCloseBrace.ts @@ -0,0 +1,11 @@ +/// + +////class A { +//// {| "indentation": 0|} } +////class B { +//// var x = 1; +//// {| "indentation": 0|} } + +test.markers().forEach((marker) => { + verify.indentationAtPositionIs(marker.fileName, marker.position, marker.data.indentation); +}); diff --git a/tests/cases/fourslash/smartIndentDoStatement.ts b/tests/cases/fourslash/smartIndentDoStatement.ts new file mode 100644 index 0000000000..2dff10f927 --- /dev/null +++ b/tests/cases/fourslash/smartIndentDoStatement.ts @@ -0,0 +1,17 @@ +/// +//// do /*1*/ { +//// } while (true) +//// +//// do { /*2*/ +//// } /*3*/while (true)/*4*/ + +function verifyIndentationAfterNewLine(marker: string, indentation: number): void { + goTo.marker(marker); + edit.insert("\r\n"); + verify.indentationIs(indentation); +} + +verifyIndentationAfterNewLine("1", 0); +verifyIndentationAfterNewLine("2", 4); +verifyIndentationAfterNewLine("3", 0); +verifyIndentationAfterNewLine("4", 0); diff --git a/tests/cases/fourslash/smartIndentIfStatement.ts b/tests/cases/fourslash/smartIndentIfStatement.ts new file mode 100644 index 0000000000..fb377ec9c9 --- /dev/null +++ b/tests/cases/fourslash/smartIndentIfStatement.ts @@ -0,0 +1,20 @@ +/// + +//// if /*1*/(true) { } +//// +//// if (true) /*2*/ { /*3*/ +//// } /*4*/ +//// +//// if (1 === /*5*/ 2) { } + +function verifyIndentationAfterNewLine(marker: string, indentation: number): void { + goTo.marker(marker); + edit.insert("\r\n"); + verify.indentationIs(indentation); +} + +verifyIndentationAfterNewLine("1", 4); +verifyIndentationAfterNewLine("2", 0); +verifyIndentationAfterNewLine("3", 4); +verifyIndentationAfterNewLine("4", 0); +verifyIndentationAfterNewLine("5", 4); \ No newline at end of file diff --git a/tests/cases/fourslash/smartIndentOnFunctionParameters.ts b/tests/cases/fourslash/smartIndentOnFunctionParameters.ts index c357ab4805..012c4d93b4 100644 --- a/tests/cases/fourslash/smartIndentOnFunctionParameters.ts +++ b/tests/cases/fourslash/smartIndentOnFunctionParameters.ts @@ -22,7 +22,7 @@ goTo.marker("4"); verify.currentLineContentIs(" c"); goTo.marker("1"); edit.insert("\r\n"); -verify.indentationIs(0); +verify.indentationIs(4); goTo.marker("5"); verify.currentLineContentIs(" //comment"); goTo.marker("6"); From e9227d656d4b0358f2b894b1ad09731bba4c1e98 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 11 Sep 2014 17:26:05 -0700 Subject: [PATCH 05/16] added support for smart indentation in the middle of list items, updated test baselines --- src/services/formatting/smartIndenter.ts | 52 +++++++++---------- .../chainedFunctionFunctionArgIndent.ts | 4 +- 2 files changed, 27 insertions(+), 29 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index f6958ebbc5..4eb3aaaa55 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -51,7 +51,7 @@ module ts.formatting { var indentation: number; while (current) { - if (!isToken(current) && isPositionBelongToNode(current, position, sourceFile)) { + if (isPositionBelongToNode(current, position, sourceFile)) { currentStartLine = getStartLineForNode(current, sourceFile); @@ -68,6 +68,10 @@ module ts.formatting { break; } + var customIndentation = getCustomIndentationForListItem(current, sourceFile); + if (customIndentation !== -1) { + return customIndentation; + } previous = current; current = current.parent; @@ -85,6 +89,11 @@ module ts.formatting { // walk upwards and collect indentations for pairs of parent-child nodes // 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' while (parent) { + var customIndentation = getCustomIndentationForListItem(current, sourceFile); + if (customIndentation !== -1) { + return customIndentation + indentation; + } + parentStartLine = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)).line; var increaseIndentation = isNodeContentIndented(parent, current) && @@ -166,45 +175,34 @@ module ts.formatting { } } - // preserve indentation for list items - // - first list item is either on the same line with the parent: foo(a... . (in this case it is not indented) or on the different line (then it is indented with base level + delta) - // - subsequent list items inherit indentation for its sibling on the left when these siblings are also on the new line. - // 1. foo(a, b - // $ - indentation = base level + delta - // 2. foo (a, - // b, c, d, - // $ - same indentation with first child node on the previous line - // NOTE: indentation for list items spans from the beginning of the line to the first non-whitespace character - // /*test*/ x, - // $ <-- indentation for a new item will be here - function getCustomIndentationForListItem(leftSibling: Node, sourceFile: SourceFile): number { - if (leftSibling.parent) { - switch (leftSibling.parent.kind) { + function getCustomIndentationForListItem(node: Node, sourceFile: SourceFile): number { + if (node.parent) { + switch (node.parent.kind) { case SyntaxKind.ObjectLiteral: - return getCustomIndentationFromList((leftSibling.parent).properties); + return getCustomIndentationFromList((node.parent).properties); case SyntaxKind.TypeLiteral: - return getCustomIndentationFromList((leftSibling.parent).members); + return getCustomIndentationFromList((node.parent).members); case SyntaxKind.ArrayLiteral: - return getCustomIndentationFromList((leftSibling.parent).elements); + return getCustomIndentationFromList((node.parent).elements); case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: case SyntaxKind.ArrowFunction: case SyntaxKind.Method: case SyntaxKind.CallSignature: case SyntaxKind.ConstructSignature: - if ((leftSibling.parent).typeParameters && leftSibling.end < (leftSibling.parent).typeParameters.end) { - return getCustomIndentationFromList((leftSibling.parent).typeParameters); + if ((node.parent).typeParameters && node.end < (node.parent).typeParameters.end) { + return getCustomIndentationFromList((node.parent).typeParameters); } else { - return getCustomIndentationFromList((leftSibling.parent).parameters); + return getCustomIndentationFromList((node.parent).parameters); } case SyntaxKind.NewExpression: case SyntaxKind.CallExpression: - if ((leftSibling.parent).typeArguments && leftSibling.end < (leftSibling.parent).typeArguments.end) { - return getCustomIndentationFromList((leftSibling.parent).typeArguments); + if ((node.parent).typeArguments && node.end < (node.parent).typeArguments.end) { + return getCustomIndentationFromList((node.parent).typeArguments); } else { - return getCustomIndentationFromList((leftSibling.parent).arguments); + return getCustomIndentationFromList((node.parent).arguments); } break; @@ -214,9 +212,9 @@ module ts.formatting { return -1; function getCustomIndentationFromList(list: Node[]): number { - var index = indexOf(list, leftSibling); + var index = indexOf(list, node); if (index !== -1) { - var lineAndCol = sourceFile.getLineAndCharacterFromPosition(leftSibling.getStart(sourceFile)); + var lineAndCol = sourceFile.getLineAndCharacterFromPosition(node.getStart(sourceFile)); for (var i = index - 1; i >= 0; --i) { var prevLineAndCol = sourceFile.getLineAndCharacterFromPosition(list[i].getStart(sourceFile)); if (lineAndCol.line !== prevLineAndCol.line) { @@ -227,7 +225,7 @@ module ts.formatting { return i; } } - // code is unreachable because the rance that we check above includes at least one non-whitespace character at the very end + // code is unreachable because the range that we check above includes at least one non-whitespace character at the very end Debug.fail("Unreachable code") } diff --git a/tests/cases/fourslash/chainedFunctionFunctionArgIndent.ts b/tests/cases/fourslash/chainedFunctionFunctionArgIndent.ts index 8b9a54de52..ac5b74e83c 100644 --- a/tests/cases/fourslash/chainedFunctionFunctionArgIndent.ts +++ b/tests/cases/fourslash/chainedFunctionFunctionArgIndent.ts @@ -2,10 +2,10 @@ //// declare var $: any; //// $(".contentDiv").each(function (index, element) {/**/ //// // <-- ensure cursor is here after return on above -//// }); // Ensure indent is 0 after LBrace +//// }); goTo.marker(); edit.insert("\n"); verify.indentationIs(4); edit.insert("}"); -verify.indentationIs(0); +verify.indentationIs(4); // keep arguments indented From d119de8227653732aa47db66ca3d3e9bb2ee4332 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 11 Sep 2014 17:26:05 -0700 Subject: [PATCH 06/16] added support for smart indentation in the middle of list items, updated test baselines --- src/services/formatting/smartIndenter.ts | 40 ++++++++++++++---------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 4eb3aaaa55..6e04833922 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -29,7 +29,7 @@ module ts.formatting { var listStartLine = getStartLineForNode(precedingListItem.parent, sourceFile); if (precedingListItemStartLineAndChar.line !== listStartLine) { - + return findFirstNonWhitespaceCharacterInLine(precedingListItemStartLineAndChar.line, precedingListItemStartLineAndChar.character, sourceFile); // previous list item starts on the different line with list, find first non-whitespace character in this line and use its position as indentation var lineStartPosition = sourceFile.getPositionFromLineAndCharacter(precedingListItemStartLineAndChar.line, 1); for (var i = 0; i < precedingListItemStartLineAndChar.character; ++i) { @@ -52,18 +52,13 @@ module ts.formatting { while (current) { if (isPositionBelongToNode(current, position, sourceFile)) { - currentStartLine = getStartLineForNode(current, sourceFile); if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) { indentation = 0; } else { - indentation = - isNodeContentIndented(current, previous) && - lineAtPosition !== currentStartLine - ? options.indentSpaces - : 0; + indentation = isNodeContentIndented(current, previous) && lineAtPosition !== currentStartLine ? options.indentSpaces : 0; } break; @@ -73,6 +68,12 @@ module ts.formatting { return customIndentation; } + // check if current node is a list item - if yes, take indentation from it + var customIndentation = getCustomIndentationForListItem(current, sourceFile); + if (customIndentation !== -1) { + return customIndentation; + } + previous = current; current = current.parent; } @@ -89,12 +90,15 @@ module ts.formatting { // walk upwards and collect indentations for pairs of parent-child nodes // 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' while (parent) { + + // check if current node is a list item - if yes, take indentation from it var customIndentation = getCustomIndentationForListItem(current, sourceFile); if (customIndentation !== -1) { return customIndentation + indentation; } parentStartLine = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)).line; + // increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line var increaseIndentation = isNodeContentIndented(parent, current) && parentStartLine !== currentStartLine && @@ -218,16 +222,7 @@ module ts.formatting { for (var i = index - 1; i >= 0; --i) { var prevLineAndCol = sourceFile.getLineAndCharacterFromPosition(list[i].getStart(sourceFile)); if (lineAndCol.line !== prevLineAndCol.line) { - // find the line start position - var lineStart = sourceFile.getPositionFromLineAndCharacter(lineAndCol.line, 1); - for (var i = 0; i <= lineAndCol.character; ++i) { - if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStart + i))) { - return i; - } - } - // code is unreachable because the range that we check above includes at least one non-whitespace character at the very end - Debug.fail("Unreachable code") - + return findFirstNonWhitespaceCharacterInLine(lineAndCol.line, lineAndCol.character, sourceFile); } lineAndCol = prevLineAndCol; } @@ -236,6 +231,17 @@ module ts.formatting { } } + function findFirstNonWhitespaceCharacterInLine(line: number, maxCharacter: number, sourceFile: SourceFile): number { + var lineStart = sourceFile.getPositionFromLineAndCharacter(line, 1); + for (var i = 0; i < maxCharacter; ++i) { + if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStart + i))) { + return i; + } + } + + return maxCharacter; + } + function findNextToken(previousToken: Node, parent: Node): Node { return find(parent); From ddbe031a8b446d8573246d9ac63ef26509bb64b6 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Sep 2014 00:18:29 -0700 Subject: [PATCH 07/16] use actual indentation if possible --- src/services/formatting/smartIndenter.ts | 207 ++++++++++++------ .../fourslash/smartIndentActualIndentation.ts | 17 ++ 2 files changed, 161 insertions(+), 63 deletions(-) create mode 100644 tests/cases/fourslash/smartIndentActualIndentation.ts diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 6e04833922..19240d537b 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -2,6 +2,12 @@ module ts.formatting { export module SmartIndenter { + + interface LineAndCharacter { + line: number; + character: number; + } + export function getIndentation(position: number, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { if (position > sourceFile.text.length) { return 0; // past EOF @@ -22,24 +28,10 @@ module ts.formatting { var lineAtPosition = sourceFile.getLineAndCharacterFromPosition(position).line; if (precedingToken.kind === SyntaxKind.CommaToken && precedingToken.parent.kind !== SyntaxKind.BinaryExpression) { - // previous token is comma that separates items in list - find the previous item and try to derive indentation from it - var precedingListItem = findPrecedingListItem(precedingToken); - var precedingListItemStartLineAndChar = sourceFile.getLineAndCharacterFromPosition(precedingListItem.getStart(sourceFile)); - var listStartLine = getStartLineForNode(precedingListItem.parent, sourceFile); - - if (precedingListItemStartLineAndChar.line !== listStartLine) { - return findFirstNonWhitespaceCharacterInLine(precedingListItemStartLineAndChar.line, precedingListItemStartLineAndChar.character, sourceFile); - // previous list item starts on the different line with list, find first non-whitespace character in this line and use its position as indentation - var lineStartPosition = sourceFile.getPositionFromLineAndCharacter(precedingListItemStartLineAndChar.line, 1); - for (var i = 0; i < precedingListItemStartLineAndChar.character; ++i) { - if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStartPosition + i))) { - return i; - } - } - - // seems that this is the first non-whitespace character on the line - return it - return precedingListItemStartLineAndChar.character; + var actualIndentation = getActualIndentationForListItemBeforeComma(precedingToken, sourceFile, options); + if (actualIndentation !== -1) { + return actualIndentation; } } @@ -47,31 +39,27 @@ module ts.formatting { // if such node is found - compute initial indentation for 'position' inside this node var previous: Node; var current = precedingToken; - var currentStartLine: number; + var currentStart: LineAndCharacter; var indentation: number; while (current) { if (isPositionBelongToNode(current, position, sourceFile)) { - currentStartLine = getStartLineForNode(current, sourceFile); + currentStart = getStartLineAndCharacterForNode(current, sourceFile); if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) { indentation = 0; } else { - indentation = isNodeContentIndented(current, previous) && lineAtPosition !== currentStartLine ? options.indentSpaces : 0; + indentation = isNodeContentIndented(current, previous) && lineAtPosition !== currentStart.line ? options.indentSpaces : 0; } break; } - var customIndentation = getCustomIndentationForListItem(current, sourceFile); - if (customIndentation !== -1) { - return customIndentation; - } // check if current node is a list item - if yes, take indentation from it - var customIndentation = getCustomIndentationForListItem(current, sourceFile); - if (customIndentation !== -1) { - return customIndentation; + var actualIndentation = getActualIndentationForListItem(current, sourceFile, options); + if (actualIndentation !== -1) { + return actualIndentation; } previous = current; @@ -85,37 +73,121 @@ module ts.formatting { var parent: Node = current.parent; - var parentStartLine: number; + var parentStart: LineAndCharacter; // walk upwards and collect indentations for pairs of parent-child nodes // 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' while (parent) { // check if current node is a list item - if yes, take indentation from it - var customIndentation = getCustomIndentationForListItem(current, sourceFile); - if (customIndentation !== -1) { - return customIndentation + indentation; + var actualIndentation = getActualIndentationForListItem(current, sourceFile, options); + if (actualIndentation !== -1) { + return actualIndentation + indentation; + } + + parentStart = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)); + var parentAndChildShareLine = + parentStart.line === currentStart.line || + isChildStartsOnTheSameLineWithElseInIfStatement(parent, current, currentStart.line, sourceFile); + + // try to fetch actual indentation for current node from source text + var actualIndentation = getActualIndentationForNode(current, parent, currentStart, parentAndChildShareLine, sourceFile, options); + if (actualIndentation !== -1) { + return actualIndentation + indentation; } - parentStartLine = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)).line; // increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line - var increaseIndentation = - isNodeContentIndented(parent, current) && - parentStartLine !== currentStartLine && - !isChildStartsOnTheSameLineWithElseInIfStatement(parent, current, currentStartLine, sourceFile); + var increaseIndentation = isNodeContentIndented(parent, current) && !parentAndChildShareLine; if (increaseIndentation) { indentation += options.indentSpaces; } current = parent; - currentStartLine = parentStartLine; + currentStart = parentStart; parent = current.parent; } return indentation; } + function isDeclaration(n: Node): boolean { + switch(n.kind) { + case SyntaxKind.ClassDeclaration: + case SyntaxKind.EnumDeclaration: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ImportDeclaration: + case SyntaxKind.Method: + case SyntaxKind.Property: + case SyntaxKind.ModuleDeclaration: + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.VariableDeclaration: + return true; + default: + return false; + } + } + + function isStatement(n: Node): boolean { + switch(n.kind) { + case SyntaxKind.BreakStatement: + case SyntaxKind.ContinueStatement: + case SyntaxKind.DebuggerStatement: + case SyntaxKind.DoStatement: + case SyntaxKind.ExpressionStatement: + case SyntaxKind.EmptyStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.ForStatement: + case SyntaxKind.IfStatement: + case SyntaxKind.LabelledStatement: + case SyntaxKind.ReturnStatement: + case SyntaxKind.SwitchStatement: + case SyntaxKind.ThrowKeyword: + case SyntaxKind.TryStatement: + case SyntaxKind.VariableStatement: + case SyntaxKind.WhileStatement: + case SyntaxKind.WithStatement: + return true; + default: + return false; + } + } + + function getActualIndentationForListItemBeforeComma(commaToken: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + // previous token is comma that separates items in list - find the previous item and try to derive indentation from it + var precedingListItem = findPrecedingListItem(commaToken); + var precedingListItemStartLineAndChar = sourceFile.getLineAndCharacterFromPosition(precedingListItem.getStart(sourceFile)); + var listStart = getStartLineAndCharacterForNode(precedingListItem.parent, sourceFile); + + if (precedingListItemStartLineAndChar.line !== listStart.line) { + return findColumnForFirstNonWhitespaceCharacterInLine(precedingListItemStartLineAndChar, sourceFile, options); + // previous list item starts on the different line with list, find first non-whitespace character in this line and use its position as indentation + var lineStartPosition = sourceFile.getPositionFromLineAndCharacter(precedingListItemStartLineAndChar.line, 1); + for (var i = 0; i < precedingListItemStartLineAndChar.character; ++i) { + if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStartPosition + i))) { + return i; + } + } + + // seems that this is the first non-whitespace character on the line - return it + return precedingListItemStartLineAndChar.character; + } + + return -1; + } + + function getActualIndentationForNode(current: Node, parent: Node, currentLineAndChar: LineAndCharacter, parentAndChildShareLine: boolean, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + var useActualIndentation = + (isDeclaration(current) || isStatement(current)) && + (parent.kind === SyntaxKind.SourceFile || !parentAndChildShareLine); + + if (!useActualIndentation) { + return -1; + } + + return findColumnForFirstNonWhitespaceCharacterInLine(currentLineAndChar, sourceFile, options); + } + function discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken: Node, current: Node, lineAtPosition: number, sourceFile: SourceFile): boolean { var nextToken = findNextToken(precedingToken, current); if (!nextToken) { @@ -136,15 +208,15 @@ module ts.formatting { // class A { // $} - var nextTokenStartLine = getStartLineForNode(nextToken, sourceFile); + var nextTokenStartLine = getStartLineAndCharacterForNode(nextToken, sourceFile).line; return lineAtPosition === nextTokenStartLine; } return false; } - function getStartLineForNode(n: Node, sourceFile: SourceFile): number { - return sourceFile.getLineAndCharacterFromPosition(n.getStart(sourceFile)).line; + function getStartLineAndCharacterForNode(n: Node, sourceFile: SourceFile): LineAndCharacter { + return sourceFile.getLineAndCharacterFromPosition(n.getStart(sourceFile)); } function findPrecedingListItem(commaToken: Node): Node { @@ -174,20 +246,20 @@ module ts.formatting { var elseKeyword = forEach(parent.getChildren(), c => c.kind === SyntaxKind.ElseKeyword && c); Debug.assert(elseKeyword); - var elseKeywordStartLine = getStartLineForNode(elseKeyword, sourceFile); + var elseKeywordStartLine = getStartLineAndCharacterForNode(elseKeyword, sourceFile).line; return elseKeywordStartLine === childStartLine; } } - function getCustomIndentationForListItem(node: Node, sourceFile: SourceFile): number { + function getActualIndentationForListItem(node: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { if (node.parent) { switch (node.parent.kind) { case SyntaxKind.ObjectLiteral: - return getCustomIndentationFromList((node.parent).properties); + return getActualIndentationFromList((node.parent).properties); case SyntaxKind.TypeLiteral: - return getCustomIndentationFromList((node.parent).members); + return getActualIndentationFromList((node.parent).members); case SyntaxKind.ArrayLiteral: - return getCustomIndentationFromList((node.parent).elements); + return getActualIndentationFromList((node.parent).elements); case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: case SyntaxKind.ArrowFunction: @@ -195,18 +267,18 @@ module ts.formatting { case SyntaxKind.CallSignature: case SyntaxKind.ConstructSignature: if ((node.parent).typeParameters && node.end < (node.parent).typeParameters.end) { - return getCustomIndentationFromList((node.parent).typeParameters); + return getActualIndentationFromList((node.parent).typeParameters); } else { - return getCustomIndentationFromList((node.parent).parameters); + return getActualIndentationFromList((node.parent).parameters); } case SyntaxKind.NewExpression: case SyntaxKind.CallExpression: if ((node.parent).typeArguments && node.end < (node.parent).typeArguments.end) { - return getCustomIndentationFromList((node.parent).typeArguments); + return getActualIndentationFromList((node.parent).typeArguments); } else { - return getCustomIndentationFromList((node.parent).arguments); + return getActualIndentationFromList((node.parent).arguments); } break; @@ -215,31 +287,40 @@ module ts.formatting { return -1; - function getCustomIndentationFromList(list: Node[]): number { + function getActualIndentationFromList(list: Node[]): number { var index = indexOf(list, node); if (index !== -1) { - var lineAndCol = sourceFile.getLineAndCharacterFromPosition(node.getStart(sourceFile)); + var lineAndCharacter = getStartLineAndCharacterForNode(node, sourceFile);; for (var i = index - 1; i >= 0; --i) { - var prevLineAndCol = sourceFile.getLineAndCharacterFromPosition(list[i].getStart(sourceFile)); - if (lineAndCol.line !== prevLineAndCol.line) { - return findFirstNonWhitespaceCharacterInLine(lineAndCol.line, lineAndCol.character, sourceFile); + var prevLineAndCharacter = getStartLineAndCharacterForNode(list[i], sourceFile); + if (lineAndCharacter.line !== prevLineAndCharacter.line) { + return findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter, sourceFile, options); } - lineAndCol = prevLineAndCol; + lineAndCharacter = prevLineAndCharacter; } } return -1; } } - function findFirstNonWhitespaceCharacterInLine(line: number, maxCharacter: number, sourceFile: SourceFile): number { - var lineStart = sourceFile.getPositionFromLineAndCharacter(line, 1); - for (var i = 0; i < maxCharacter; ++i) { - if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStart + i))) { - return i; + function findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter: LineAndCharacter, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + var lineStart = sourceFile.getPositionFromLineAndCharacter(lineAndCharacter.line, 1); + var column = 0; + for (var i = 0; i < lineAndCharacter.character; ++i) { + var charCode = sourceFile.text.charCodeAt(lineStart + i); + if (!isWhiteSpace(charCode)) { + return column; + } + + if (charCode === CharacterCodes.tab) { + column += options.spacesPerTab; + } + else { + column++; } } - return maxCharacter; + return column; } function findNextToken(previousToken: Node, parent: Node): Node { @@ -257,10 +338,10 @@ module ts.formatting { var shouldDiveInChildNode = // previous token is enclosed somewhere in the child (child.pos <= previousToken.pos && child.end > previousToken.end) || - // previous token end exactly at the beginning of child + // previous token ends exactly at the beginning of child (child.pos === previousToken.end); - if (shouldDiveInChildNode && isCandidateNode(child)) { + if (shouldDiveInChildNode && isCandidateNode(child)) { return find(child); } } diff --git a/tests/cases/fourslash/smartIndentActualIndentation.ts b/tests/cases/fourslash/smartIndentActualIndentation.ts new file mode 100644 index 0000000000..b2874d3c1d --- /dev/null +++ b/tests/cases/fourslash/smartIndentActualIndentation.ts @@ -0,0 +1,17 @@ +/// + +//// class A { +//// /*1*/ +//// } + +////module M { +//// class C { +//// /*2*/ +//// } +////} + +goTo.marker("1"); +verify.indentationIs(12); + +goTo.marker("2"); +verify.indentationIs(16); From 221f805d09c5091afed30a273462ac08f895f26f Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Sep 2014 13:28:39 -0700 Subject: [PATCH 08/16] remove singleTokenIndenter.ts --- src/services/formatting/formatting.ts | 1 - .../formatting/singleTokenIndenter.ts | 46 ------------------- 2 files changed, 47 deletions(-) delete mode 100644 src/services/formatting/singleTokenIndenter.ts diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index c334e1864a..7570ef936c 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -36,5 +36,4 @@ /// /// /// -/// /// \ No newline at end of file diff --git a/src/services/formatting/singleTokenIndenter.ts b/src/services/formatting/singleTokenIndenter.ts deleted file mode 100644 index 896c2e8ac0..0000000000 --- a/src/services/formatting/singleTokenIndenter.ts +++ /dev/null @@ -1,46 +0,0 @@ -// -// Copyright (c) Microsoft Corporation. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -/// - -module TypeScript.Services.Formatting { - export class SingleTokenIndenter extends IndentationTrackingWalker { - private indentationAmount: number = null; - private indentationPosition: number; - - constructor(indentationPosition: number, sourceUnit: SourceUnitSyntax, snapshot: ITextSnapshot, indentFirstToken: boolean, options: FormattingOptions) { - super(new TextSpan(indentationPosition, 1), sourceUnit, snapshot, indentFirstToken, options); - - this.indentationPosition = indentationPosition; - } - - public static getIndentationAmount(position: number, sourceUnit: SourceUnitSyntax, snapshot: ITextSnapshot, options: FormattingOptions): number { - var walker = new SingleTokenIndenter(position, sourceUnit, snapshot, true, options); - visitNodeOrToken(walker, sourceUnit); - return walker.indentationAmount; - } - - public indentToken(token: ISyntaxToken, indentationAmount: number, commentIndentationAmount: number): void { - // Compute an indentation string for this token - if (token.fullWidth() === 0 || (this.indentationPosition - this.position() < token.leadingTriviaWidth())) { - // The position is in the leading trivia, use comment indentation - this.indentationAmount = commentIndentationAmount; - } - else { - this.indentationAmount = indentationAmount; - } - } - } -} \ No newline at end of file From cd663f07f31b0af0b01c44cfdef3bd9289bd73df Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Sep 2014 15:41:46 -0700 Subject: [PATCH 09/16] minor code cleanup --- src/services/formatting/smartIndenter.ts | 29 ++++++++---------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 19240d537b..86e0720632 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -161,16 +161,6 @@ module ts.formatting { if (precedingListItemStartLineAndChar.line !== listStart.line) { return findColumnForFirstNonWhitespaceCharacterInLine(precedingListItemStartLineAndChar, sourceFile, options); - // previous list item starts on the different line with list, find first non-whitespace character in this line and use its position as indentation - var lineStartPosition = sourceFile.getPositionFromLineAndCharacter(precedingListItemStartLineAndChar.line, 1); - for (var i = 0; i < precedingListItemStartLineAndChar.character; ++i) { - if (!isWhiteSpace(sourceFile.text.charCodeAt(lineStartPosition + i))) { - return i; - } - } - - // seems that this is the first non-whitespace character on the line - return it - return precedingListItemStartLineAndChar.character; } return -1; @@ -290,6 +280,8 @@ module ts.formatting { function getActualIndentationFromList(list: Node[]): number { var index = indexOf(list, node); if (index !== -1) { + // walk toward the start of the list starting from current node and check if if line is the same for all items. + // 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] var lineAndCharacter = getStartLineAndCharacterForNode(node, sourceFile);; for (var i = index - 1; i >= 0; --i) { var prevLineAndCharacter = getStartLineAndCharacterForNode(list[i], sourceFile); @@ -312,7 +304,7 @@ module ts.formatting { return column; } - if (charCode === CharacterCodes.tab) { + if (charCode === CharacterCodes.tab && !options.useTabs) { column += options.spacesPerTab; } else { @@ -479,6 +471,7 @@ module ts.formatting { return false; } + // this function is alwasy called when position of the cursor is located after the node function isCompletedNode(n: Node, sourceFile: SourceFile): boolean { switch (n.kind) { case SyntaxKind.ClassDeclaration: @@ -518,17 +511,15 @@ module ts.formatting { case SyntaxKind.DefaultClause: // there is no such thing as terminator token for CaseClause\DefaultClause so for simplicitly always consider them non-completed return false; - case SyntaxKind.VariableStatement: - // variable statement is considered completed if it either doesn'not have variable declarations or last variable declaration is completed - var variableDeclarations = (n).declarations; - return variableDeclarations.length === 0 || isCompletedNode(variableDeclarations[variableDeclarations.length - 1], sourceFile); - case SyntaxKind.VariableDeclaration: - // variable declaration is completed if it either doesn't have initializer or initializer is completed - return !(n).initializer || isCompletedNode((n).initializer, sourceFile); case SyntaxKind.WhileStatement: return isCompletedNode((n).statement, sourceFile); case SyntaxKind.DoStatement: - return isCompletedNode((n).statement, sourceFile); + // rough approximation: if DoStatement has While keyword - then if node is completed is checking the presence of ')'; + var hasWhileKeyword = forEach(n.getChildren(), c => c.kind === SyntaxKind.WhileKeyword && c); + if(hasWhileKeyword) { + return isNodeEndWith(n, SyntaxKind.CloseParenToken, sourceFile); + } + return isCompletedNode((n).statement, sourceFile); default: return true; } From 54b8c73a7834337013463eab41e1257cc5a2e672 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Sep 2014 16:18:06 -0700 Subject: [PATCH 10/16] when looking for a start node consider only nodes that can contribute to indentation --- src/services/formatting/smartIndenter.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 86e0720632..f4a492f405 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -35,7 +35,7 @@ module ts.formatting { } } - // try to find the node that will include 'position' starting from 'precedingToken' + // try to find node that can contribute to indentation and includes 'position' starting from 'precedingToken' // if such node is found - compute initial indentation for 'position' inside this node var previous: Node; var current = precedingToken; @@ -43,14 +43,14 @@ module ts.formatting { var indentation: number; while (current) { - if (isPositionBelongToNode(current, position, sourceFile)) { + if (isPositionBelongToNode(current, position, sourceFile) && isNodeContentIndented(current, previous)) { currentStart = getStartLineAndCharacterForNode(current, sourceFile); if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) { indentation = 0; } else { - indentation = isNodeContentIndented(current, previous) && lineAtPosition !== currentStart.line ? options.indentSpaces : 0; + indentation = lineAtPosition !== currentStart.line ? options.indentSpaces : 0; } break; @@ -444,7 +444,6 @@ module ts.formatting { case SyntaxKind.DefaultClause: case SyntaxKind.CaseClause: case SyntaxKind.ParenExpression: - case SyntaxKind.BinaryExpression: case SyntaxKind.CallExpression: case SyntaxKind.NewExpression: case SyntaxKind.VariableStatement: From c21b4abb5072a2713e95dee78b23c318a46a20f1 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Sep 2014 16:29:39 -0700 Subject: [PATCH 11/16] update JakeFile --- Jakefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Jakefile b/Jakefile index 97b1bc826a..1307601fb8 100644 --- a/Jakefile +++ b/Jakefile @@ -54,7 +54,6 @@ var servicesSources = [ }).concat([ "services.ts", "shims.ts", - "formatting\\smartIndenter.ts" ].map(function (f) { return path.join(servicesDirectory, f); })); From aa7ffdba4c8add4a4cdce40c81ec8dd69f2c9ed6 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Fri, 12 Sep 2014 22:45:52 -0700 Subject: [PATCH 12/16] address CR feedback --- src/compiler/types.ts | 4 +++- src/services/formatting/smartIndenter.ts | 30 +++++++++++++----------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index c13b47a066..3285d24a5b 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -221,7 +221,9 @@ module ts { FirstTypeNode = TypeReference, LastTypeNode = ArrayType, FirstPunctuation= OpenBraceToken, - LastPunctuation = CaretEqualsToken + LastPunctuation = CaretEqualsToken, + FirstToken = EndOfFileToken, + LastToken = StringKeyword } export enum NodeFlags { diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index f4a492f405..20cf087631 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -43,7 +43,7 @@ module ts.formatting { var indentation: number; while (current) { - if (isPositionBelongToNode(current, position, sourceFile) && isNodeContentIndented(current, previous)) { + if (positionBelongsToNode(current, position, sourceFile) && nodeContentIsIndented(current, previous)) { currentStart = getStartLineAndCharacterForNode(current, sourceFile); if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) { @@ -88,7 +88,7 @@ module ts.formatting { parentStart = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)); var parentAndChildShareLine = parentStart.line === currentStart.line || - isChildStartsOnTheSameLineWithElseInIfStatement(parent, current, currentStart.line, sourceFile); + childStartsOnTheSameLineWithElseInIfStatement(parent, current, currentStart.line, sourceFile); // try to fetch actual indentation for current node from source text var actualIndentation = getActualIndentationForNode(current, parent, currentStart, parentAndChildShareLine, sourceFile, options); @@ -97,7 +97,7 @@ module ts.formatting { } // increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line - var increaseIndentation = isNodeContentIndented(parent, current) && !parentAndChildShareLine; + var increaseIndentation = nodeContentIsIndented(parent, current) && !parentAndChildShareLine; if (increaseIndentation) { indentation += options.indentSpaces; @@ -153,6 +153,7 @@ module ts.formatting { } } + // function returns -1 if indentation cannot be determined function getActualIndentationForListItemBeforeComma(commaToken: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { // previous token is comma that separates items in list - find the previous item and try to derive indentation from it var precedingListItem = findPrecedingListItem(commaToken); @@ -166,6 +167,7 @@ module ts.formatting { return -1; } + // function returns -1 if actual indentation for node should not be used (i.e because node is nested expression) function getActualIndentationForNode(current: Node, parent: Node, currentLineAndChar: LineAndCharacter, parentAndChildShareLine: boolean, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { var useActualIndentation = (isDeclaration(current) || isStatement(current)) && @@ -227,11 +229,11 @@ module ts.formatting { return children[commaIndex - 1]; } - function isPositionBelongToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean { + function positionBelongsToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean { return candidate.end > position || !isCompletedNode(candidate, sourceFile); } - function isChildStartsOnTheSameLineWithElseInIfStatement(parent: Node, child: Node, childStartLine: number, sourceFile: SourceFile): boolean { + function childStartsOnTheSameLineWithElseInIfStatement(parent: Node, child: Node, childStartLine: number, sourceFile: SourceFile): boolean { if (parent.kind === SyntaxKind.IfStatement && (parent).elseStatement === child) { var elseKeyword = forEach(parent.getChildren(), c => c.kind === SyntaxKind.ElseKeyword && c); Debug.assert(elseKeyword); @@ -404,10 +406,10 @@ module ts.formatting { } function isToken(n: Node): boolean { - return n.kind < SyntaxKind.Missing; + return n.kind >= SyntaxKind.FirstToken && n.kind <= SyntaxKind.LastToken; } - function isNodeContentIndented(parent: Node, child: Node): boolean { + function nodeContentIsIndented(parent: Node, child: Node): boolean { switch (parent.kind) { case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: @@ -456,7 +458,7 @@ module ts.formatting { /// checks if node ends with 'expectedLastToken'. /// If child at position 'length - 1' is 'SemicolonToken' it is skipped and 'expectedLastToken' is compared with child at position 'length - 2'. - function isNodeEndWith(n: Node, expectedLastToken: SyntaxKind, sourceFile: SourceFile): boolean { + function nodeEndsWith(n: Node, expectedLastToken: SyntaxKind, sourceFile: SourceFile): boolean { var children = n.getChildren(sourceFile); if (children.length) { var last = children[children.length - 1]; @@ -470,7 +472,7 @@ module ts.formatting { return false; } - // this function is alwasy called when position of the cursor is located after the node + // this function is always called when position of the cursor is located after the node function isCompletedNode(n: Node, sourceFile: SourceFile): boolean { switch (n.kind) { case SyntaxKind.ClassDeclaration: @@ -483,11 +485,11 @@ module ts.formatting { case SyntaxKind.FunctionBlock: case SyntaxKind.ModuleBlock: case SyntaxKind.SwitchStatement: - return isNodeEndWith(n, SyntaxKind.CloseBraceToken, sourceFile); + return nodeEndsWith(n, SyntaxKind.CloseBraceToken, sourceFile); case SyntaxKind.ParenExpression: case SyntaxKind.CallSignature: case SyntaxKind.CallExpression: - return isNodeEndWith(n, SyntaxKind.CloseParenToken, sourceFile); + return nodeEndsWith(n, SyntaxKind.CloseParenToken, sourceFile); case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: case SyntaxKind.Method: @@ -503,7 +505,7 @@ module ts.formatting { case SyntaxKind.ExpressionStatement: return isCompletedNode((n).expression, sourceFile); case SyntaxKind.ArrayLiteral: - return isNodeEndWith(n, SyntaxKind.CloseBracketToken, sourceFile); + return nodeEndsWith(n, SyntaxKind.CloseBracketToken, sourceFile); case SyntaxKind.Missing: return false; case SyntaxKind.CaseClause: @@ -516,8 +518,8 @@ module ts.formatting { // rough approximation: if DoStatement has While keyword - then if node is completed is checking the presence of ')'; var hasWhileKeyword = forEach(n.getChildren(), c => c.kind === SyntaxKind.WhileKeyword && c); if(hasWhileKeyword) { - return isNodeEndWith(n, SyntaxKind.CloseParenToken, sourceFile); - } + return nodeEndsWith(n, SyntaxKind.CloseParenToken, sourceFile); + } return isCompletedNode((n).statement, sourceFile); default: return true; From 685d131f4a2eb1924e2eadb602b79658120e94ad Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 15 Sep 2014 12:18:09 -0700 Subject: [PATCH 13/16] addressed CR feedback: use isDeclaration from parser, drop 'useTabs' check --- src/compiler/types.ts | 9 +++++ src/services/formatting/smartIndenter.ts | 46 +++++++++--------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 3285d24a5b..b460b1eb0c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -979,6 +979,15 @@ module ts { AMD, } + export interface LineAndCharacter { + line: number; + /* + * This value denotes the character position in line and is different from the 'column' because of tab characters. + */ + character: number; + } + + export enum ScriptTarget { ES3, ES5, diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 20cf087631..30ae865e00 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -3,11 +3,6 @@ module ts.formatting { export module SmartIndenter { - interface LineAndCharacter { - line: number; - character: number; - } - export function getIndentation(position: number, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { if (position > sourceFile.text.length) { return 0; // past EOF @@ -111,23 +106,6 @@ module ts.formatting { return indentation; } - function isDeclaration(n: Node): boolean { - switch(n.kind) { - case SyntaxKind.ClassDeclaration: - case SyntaxKind.EnumDeclaration: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.ImportDeclaration: - case SyntaxKind.Method: - case SyntaxKind.Property: - case SyntaxKind.ModuleDeclaration: - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.VariableDeclaration: - return true; - default: - return false; - } - } - function isStatement(n: Node): boolean { switch(n.kind) { case SyntaxKind.BreakStatement: @@ -153,7 +131,9 @@ module ts.formatting { } } - // function returns -1 if indentation cannot be determined + /* + * Function returns -1 if indentation cannot be determined + */ function getActualIndentationForListItemBeforeComma(commaToken: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { // previous token is comma that separates items in list - find the previous item and try to derive indentation from it var precedingListItem = findPrecedingListItem(commaToken); @@ -167,7 +147,9 @@ module ts.formatting { return -1; } - // function returns -1 if actual indentation for node should not be used (i.e because node is nested expression) + /* + * Function returns -1 if actual indentation for node should not be used (i.e because node is nested expression) + */ function getActualIndentationForNode(current: Node, parent: Node, currentLineAndChar: LineAndCharacter, parentAndChildShareLine: boolean, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { var useActualIndentation = (isDeclaration(current) || isStatement(current)) && @@ -306,7 +288,7 @@ module ts.formatting { return column; } - if (charCode === CharacterCodes.tab && !options.useTabs) { + if (charCode === CharacterCodes.tab) { column += options.spacesPerTab; } else { @@ -391,7 +373,9 @@ module ts.formatting { } } - /// 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. + /* + * 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. + */ function isCandidateNode(n: Node): boolean { if (n.kind === SyntaxKind.ExpressionStatement) { return isCandidateNode((n).expression); @@ -456,8 +440,10 @@ module ts.formatting { } } - /// checks if node ends with 'expectedLastToken'. - /// If child at position 'length - 1' is 'SemicolonToken' it is skipped and 'expectedLastToken' is compared with child at position 'length - 2'. + /* + * Checks if node ends with 'expectedLastToken'. + * If child at position 'length - 1' is 'SemicolonToken' it is skipped and 'expectedLastToken' is compared with child at position 'length - 2'. + */ function nodeEndsWith(n: Node, expectedLastToken: SyntaxKind, sourceFile: SourceFile): boolean { var children = n.getChildren(sourceFile); if (children.length) { @@ -472,7 +458,9 @@ module ts.formatting { return false; } - // this function is always called when position of the cursor is located after the node + /* + * This function is always called when position of the cursor is located after the node + */ function isCompletedNode(n: Node, sourceFile: SourceFile): boolean { switch (n.kind) { case SyntaxKind.ClassDeclaration: From c4e6ad8dd903c76fd17917323749e5df91f10d53 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 15 Sep 2014 16:08:33 -0700 Subject: [PATCH 14/16] address CR feedback: renames, handle smart indentation in type argument lists in type references --- src/compiler/parser.ts | 26 +++ src/services/formatting/smartIndenter.ts | 150 +++++++++--------- tests/cases/fourslash/smartIndentListItem.ts | 14 ++ .../fourslash/smartIndentTypeArgumentList.ts | 8 + 4 files changed, 120 insertions(+), 78 deletions(-) create mode 100644 tests/cases/fourslash/smartIndentListItem.ts create mode 100644 tests/cases/fourslash/smartIndentTypeArgumentList.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 7e5ac13b8d..a79b90f5bb 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -384,6 +384,32 @@ module ts { return false; } + export function isStatement(n: Node): boolean { + switch(n.kind) { + case SyntaxKind.BreakStatement: + case SyntaxKind.ContinueStatement: + case SyntaxKind.DebuggerStatement: + case SyntaxKind.DoStatement: + case SyntaxKind.ExpressionStatement: + case SyntaxKind.EmptyStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.ForStatement: + case SyntaxKind.IfStatement: + case SyntaxKind.LabelledStatement: + case SyntaxKind.ReturnStatement: + case SyntaxKind.SwitchStatement: + case SyntaxKind.ThrowKeyword: + case SyntaxKind.TryStatement: + case SyntaxKind.VariableStatement: + case SyntaxKind.WhileStatement: + case SyntaxKind.WithStatement: + case SyntaxKind.ExportAssignment: + return true; + default: + return false; + } + } + // True if the given identifier, string literal, or number literal is the name of a declaration node export function isDeclarationOrFunctionExpressionOrCatchVariableName(name: Node): boolean { if (name.kind !== SyntaxKind.Identifier && name.kind !== SyntaxKind.StringLiteral && name.kind !== SyntaxKind.NumericLiteral) { diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 30ae865e00..09294f7998 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -35,17 +35,17 @@ module ts.formatting { var previous: Node; var current = precedingToken; var currentStart: LineAndCharacter; - var indentation: number; + var indentationDelta: number; while (current) { if (positionBelongsToNode(current, position, sourceFile) && nodeContentIsIndented(current, previous)) { currentStart = getStartLineAndCharacterForNode(current, sourceFile); - if (discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken, current, lineAtPosition, sourceFile)) { - indentation = 0; + if (nextTokenIsCurlyBraceOnSameLineAsCursor(precedingToken, current, lineAtPosition, sourceFile)) { + indentationDelta = 0; } else { - indentation = lineAtPosition !== currentStart.line ? options.indentSpaces : 0; + indentationDelta = lineAtPosition !== currentStart.line ? options.indentSpaces : 0; } break; @@ -73,11 +73,10 @@ module ts.formatting { // walk upwards and collect indentations for pairs of parent-child nodes // 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' while (parent) { - // check if current node is a list item - if yes, take indentation from it var actualIndentation = getActualIndentationForListItem(current, sourceFile, options); if (actualIndentation !== -1) { - return actualIndentation + indentation; + return actualIndentation + indentationDelta; } parentStart = sourceFile.getLineAndCharacterFromPosition(parent.getStart(sourceFile)); @@ -88,14 +87,12 @@ module ts.formatting { // try to fetch actual indentation for current node from source text var actualIndentation = getActualIndentationForNode(current, parent, currentStart, parentAndChildShareLine, sourceFile, options); if (actualIndentation !== -1) { - return actualIndentation + indentation; + return actualIndentation + indentationDelta; } // increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line - var increaseIndentation = nodeContentIsIndented(parent, current) && !parentAndChildShareLine; - - if (increaseIndentation) { - indentation += options.indentSpaces; + if (nodeContentIsIndented(parent, current) && !parentAndChildShareLine) { + indentationDelta += options.indentSpaces; } current = parent; @@ -103,32 +100,7 @@ module ts.formatting { parent = current.parent; } - return indentation; - } - - function isStatement(n: Node): boolean { - switch(n.kind) { - case SyntaxKind.BreakStatement: - case SyntaxKind.ContinueStatement: - case SyntaxKind.DebuggerStatement: - case SyntaxKind.DoStatement: - case SyntaxKind.ExpressionStatement: - case SyntaxKind.EmptyStatement: - case SyntaxKind.ForInStatement: - case SyntaxKind.ForStatement: - case SyntaxKind.IfStatement: - case SyntaxKind.LabelledStatement: - case SyntaxKind.ReturnStatement: - case SyntaxKind.SwitchStatement: - case SyntaxKind.ThrowKeyword: - case SyntaxKind.TryStatement: - case SyntaxKind.VariableStatement: - case SyntaxKind.WhileStatement: - case SyntaxKind.WithStatement: - return true; - default: - return false; - } + return indentationDelta; } /* @@ -136,21 +108,23 @@ module ts.formatting { */ function getActualIndentationForListItemBeforeComma(commaToken: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { // previous token is comma that separates items in list - find the previous item and try to derive indentation from it - var precedingListItem = findPrecedingListItem(commaToken); - var precedingListItemStartLineAndChar = sourceFile.getLineAndCharacterFromPosition(precedingListItem.getStart(sourceFile)); - var listStart = getStartLineAndCharacterForNode(precedingListItem.parent, sourceFile); - - if (precedingListItemStartLineAndChar.line !== listStart.line) { - return findColumnForFirstNonWhitespaceCharacterInLine(precedingListItemStartLineAndChar, sourceFile, options); - } - - return -1; + var itemInfo = findPrecedingListItem(commaToken); + return deriveActualIndentationFromList(itemInfo.list.getChildren(), itemInfo.listItemIndex, sourceFile, options); } /* * Function returns -1 if actual indentation for node should not be used (i.e because node is nested expression) */ - function getActualIndentationForNode(current: Node, parent: Node, currentLineAndChar: LineAndCharacter, parentAndChildShareLine: boolean, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + function getActualIndentationForNode(current: Node, + parent: Node, + currentLineAndChar: LineAndCharacter, + parentAndChildShareLine: boolean, + sourceFile: SourceFile, + options: TypeScript.FormattingOptions): number { + + // actual indentation is used for statements\declarations if one of cases below is true: + // - parent is SourceFile - by default immediate children of SourceFile are not indented except when user indents them manually + // - parent and child are not on the same line var useActualIndentation = (isDeclaration(current) || isStatement(current)) && (parent.kind === SyntaxKind.SourceFile || !parentAndChildShareLine); @@ -162,7 +136,7 @@ module ts.formatting { return findColumnForFirstNonWhitespaceCharacterInLine(currentLineAndChar, sourceFile, options); } - function discardInitialIndentationIfNextTokenIsOpenOrCloseBrace(precedingToken: Node, current: Node, lineAtPosition: number, sourceFile: SourceFile): boolean { + function nextTokenIsCurlyBraceOnSameLineAsCursor(precedingToken: Node, current: Node, lineAtPosition: number, sourceFile: SourceFile): boolean { var nextToken = findNextToken(precedingToken, current); if (!nextToken) { return false; @@ -193,7 +167,7 @@ module ts.formatting { return sourceFile.getLineAndCharacterFromPosition(n.getStart(sourceFile)); } - function findPrecedingListItem(commaToken: Node): Node { + function findPrecedingListItem(commaToken: Node): { listItemIndex: number; list: Node } { // 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. // In order to find the preceding list item we first need to locate SyntaxList itself and then search for the position of CommaToken var syntaxList = forEach(commaToken.parent.getChildren(), c => { @@ -208,7 +182,10 @@ module ts.formatting { var commaIndex = indexOf(children, commaToken); Debug.assert(commaIndex !== -1 && commaIndex !== 0); - return children[commaIndex - 1]; + return { + listItemIndex: commaIndex - 1, + list: syntaxList + }; } function positionBelongsToNode(candidate: Node, position: number, sourceFile: SourceFile): boolean { @@ -228,6 +205,10 @@ module ts.formatting { function getActualIndentationForListItem(node: Node, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { if (node.parent) { switch (node.parent.kind) { + case SyntaxKind.TypeReference: + if ((node.parent).typeArguments) { + return getActualIndentationFromList((node.parent).typeArguments); + } case SyntaxKind.ObjectLiteral: return getActualIndentationFromList((node.parent).properties); case SyntaxKind.TypeLiteral: @@ -243,19 +224,15 @@ module ts.formatting { if ((node.parent).typeParameters && node.end < (node.parent).typeParameters.end) { return getActualIndentationFromList((node.parent).typeParameters); } - else { - return getActualIndentationFromList((node.parent).parameters); - } + + return getActualIndentationFromList((node.parent).parameters); case SyntaxKind.NewExpression: case SyntaxKind.CallExpression: if ((node.parent).typeArguments && node.end < (node.parent).typeArguments.end) { return getActualIndentationFromList((node.parent).typeArguments); } - else { - return getActualIndentationFromList((node.parent).arguments); - } - - break; + + return getActualIndentationFromList((node.parent).arguments); } } @@ -263,22 +240,33 @@ module ts.formatting { function getActualIndentationFromList(list: Node[]): number { var index = indexOf(list, node); - if (index !== -1) { - // walk toward the start of the list starting from current node and check if if line is the same for all items. - // 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] - var lineAndCharacter = getStartLineAndCharacterForNode(node, sourceFile);; - for (var i = index - 1; i >= 0; --i) { - var prevLineAndCharacter = getStartLineAndCharacterForNode(list[i], sourceFile); - if (lineAndCharacter.line !== prevLineAndCharacter.line) { - return findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter, sourceFile, options); - } - lineAndCharacter = prevLineAndCharacter; - } - } - return -1; + return index !== -1 ? deriveActualIndentationFromList(list, index, sourceFile, options) : -1; } } + + function deriveActualIndentationFromList(list: Node[], index: number, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { + Debug.assert(index >= 0 && index < list.length); + var node = list[index]; + + // walk toward the start of the list starting from current node and check if the line is the same for all items. + // 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] + var lineAndCharacter = getStartLineAndCharacterForNode(node, sourceFile); + for (var i = index - 1; i >= 0; --i) { + if (list[i].kind === SyntaxKind.CommaToken) { + continue; + } + // skip list items that ends on the same line with the current list element + var prevEndLine = sourceFile.getLineAndCharacterFromPosition(list[i].end).line; + if (prevEndLine !== lineAndCharacter.line) { + return findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter, sourceFile, options); + } + + lineAndCharacter = getStartLineAndCharacterForNode(list[i], sourceFile); + } + return -1; + } + function findColumnForFirstNonWhitespaceCharacterInLine(lineAndCharacter: LineAndCharacter, sourceFile: SourceFile, options: TypeScript.FormattingOptions): number { var lineStart = sourceFile.getPositionFromLineAndCharacter(lineAndCharacter.line, 1); var column = 0; @@ -317,10 +305,12 @@ module ts.formatting { // previous token ends exactly at the beginning of child (child.pos === previousToken.end); - if (shouldDiveInChildNode && isCandidateNode(child)) { + if (shouldDiveInChildNode && nodeHasTokens(child)) { return find(child); } } + + return undefined; } } @@ -335,12 +325,12 @@ module ts.formatting { var children = n.getChildren(); if (diveIntoLastChild) { var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ children.length); - return candidate && find(candidate, diveIntoLastChild); + return candidate && find(candidate, /*diveIntoLastChild*/ true); } for (var i = 0, len = children.length; i < len; ++i) { var child = children[i]; - if (isCandidateNode(child)) { + if (nodeHasTokens(child)) { if (position < child.end) { if (child.getStart(sourceFile) >= position) { // 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 { /// finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition' function findLastChildNodeCandidate(children: Node[], exclusiveStartPosition: number): Node { for (var i = exclusiveStartPosition - 1; i >= 0; --i) { - if (isCandidateNode(children[i])) { + if (nodeHasTokens(children[i])) { return children[i]; } } @@ -376,9 +366,9 @@ module ts.formatting { /* * 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. */ - function isCandidateNode(n: Node): boolean { + function nodeHasTokens(n: Node): boolean { if (n.kind === SyntaxKind.ExpressionStatement) { - return isCandidateNode((n).expression); + return nodeHasTokens((n).expression); } if (n.kind === SyntaxKind.EndOfFileToken || n.kind === SyntaxKind.OmittedExpression || n.kind === SyntaxKind.Missing) { @@ -404,7 +394,10 @@ module ts.formatting { return false; case SyntaxKind.FunctionDeclaration: case SyntaxKind.Method: - case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionExpression: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + case SyntaxKind.Constructor: // FunctionBlock should take care of indentation return false; case SyntaxKind.DoStatement: @@ -477,6 +470,7 @@ module ts.formatting { case SyntaxKind.ParenExpression: case SyntaxKind.CallSignature: case SyntaxKind.CallExpression: + case SyntaxKind.ConstructSignature: return nodeEndsWith(n, SyntaxKind.CloseParenToken, sourceFile); case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: diff --git a/tests/cases/fourslash/smartIndentListItem.ts b/tests/cases/fourslash/smartIndentListItem.ts new file mode 100644 index 0000000000..f71db9c865 --- /dev/null +++ b/tests/cases/fourslash/smartIndentListItem.ts @@ -0,0 +1,14 @@ +/// +////[1, +//// 2 +//// + 3, 4, +//// /*1*/ +////[1, +//// 2 +//// + 3, 4 +//// /*2*/ + +goTo.marker("1"); +verify.indentationIs(4) +goTo.marker("2"); +verify.indentationIs(4) \ No newline at end of file diff --git a/tests/cases/fourslash/smartIndentTypeArgumentList.ts b/tests/cases/fourslash/smartIndentTypeArgumentList.ts new file mode 100644 index 0000000000..24136596ed --- /dev/null +++ b/tests/cases/fourslash/smartIndentTypeArgumentList.ts @@ -0,0 +1,8 @@ +/// + +////interface T1 extends T + +goTo.marker(); +verify.indentationIs(32); \ No newline at end of file From e369e369ccee686ac2f8c1f0ec44c0820f588c5a Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 15 Sep 2014 18:21:45 -0700 Subject: [PATCH 15/16] split 'find' function into 'find' and 'findRightmostChildNodeWithTokens' --- src/services/formatting/smartIndenter.ts | 36 +++++++++++++++--------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 09294f7998..adbc3d2a0c 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -315,46 +315,56 @@ module ts.formatting { } function findPrecedingToken(position: number, sourceFile: SourceFile): Node { - return find(sourceFile, /*diveIntoLastChild*/ false); + return find(sourceFile); - function find(n: Node, diveIntoLastChild: boolean): Node { + function findRightmostToken(n: Node): Node { if (isToken(n)) { return n; } var children = n.getChildren(); - if (diveIntoLastChild) { - var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ children.length); - return candidate && find(candidate, /*diveIntoLastChild*/ true); + var candidate = findRightmostChildNodeWithTokens(children, /*exclusiveStartPosition*/ children.length); + return candidate && findRightmostToken(candidate); + + } + + function find(n: Node): Node { + if (isToken(n)) { + return n; } + var children = n.getChildren(); for (var i = 0, len = children.length; i < len; ++i) { var child = children[i]; if (nodeHasTokens(child)) { if (position < child.end) { if (child.getStart(sourceFile) >= position) { // actual start of the node is past the position - previous token should be at the end of previous child - var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ i); - return candidate && find(candidate, /*diveIntoLastChild*/ true) + var candidate = findRightmostChildNodeWithTokens(children, /*exclusiveStartPosition*/ i); + return candidate && findRightmostToken(candidate) } else { // candidate should be in this node - return find(child, diveIntoLastChild); + return find(child); } } } } - // here we know that none of child token nodes embrace the position - // try to find the closest token on the left + Debug.assert(n.kind === SyntaxKind.SourceFile); + + // Here we know that none of child token nodes embrace the position, + // the only known case is when position is at the end of the file. + // Try to find the rightmost token in the file without filtering. + // Namely we are skipping the check: 'position < node.end' if (children.length) { - var candidate = findLastChildNodeCandidate(children, /*exclusiveStartPosition*/ children.length); - return candidate && find(candidate, /*diveIntoLastChild*/ true); + var candidate = findRightmostChildNodeWithTokens(children, /*exclusiveStartPosition*/ children.length); + return candidate && findRightmostToken(candidate); } } /// finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition' - function findLastChildNodeCandidate(children: Node[], exclusiveStartPosition: number): Node { + function findRightmostChildNodeWithTokens(children: Node[], exclusiveStartPosition: number): Node { for (var i = exclusiveStartPosition - 1; i >= 0; --i) { if (nodeHasTokens(children[i])) { return children[i]; From 08f446ddf7e7514cda3a2aa4133e55e73b4473a7 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Tue, 16 Sep 2014 11:27:58 -0700 Subject: [PATCH 16/16] resolve merge conflicts --- src/compiler/parser.ts | 2 +- src/services/formatting/smartIndenter.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 5aa333f4c8..40d9722af0 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -498,7 +498,7 @@ module ts { case SyntaxKind.ForInStatement: case SyntaxKind.ForStatement: case SyntaxKind.IfStatement: - case SyntaxKind.LabelledStatement: + case SyntaxKind.LabeledStatement: case SyntaxKind.ReturnStatement: case SyntaxKind.SwitchStatement: case SyntaxKind.ThrowKeyword: diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index adbc3d2a0c..ad07f884cc 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -209,6 +209,7 @@ module ts.formatting { if ((node.parent).typeArguments) { return getActualIndentationFromList((node.parent).typeArguments); } + break; case SyntaxKind.ObjectLiteral: return getActualIndentationFromList((node.parent).properties); case SyntaxKind.TypeLiteral: