From 35b16ab7a8294abd093e7eed36260b46f1c4942e Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 7 Jun 2017 14:01:42 -0700 Subject: [PATCH 01/50] temp --- src/harness/fourslash.ts | 14 ++++++++++++++ src/harness/harnessLanguageService.ts | 3 +++ src/server/client.ts | 4 ++++ src/server/protocol.ts | 15 +++++++++++++++ src/server/session.ts | 11 +++++++++++ src/services/services.ts | 11 +++++++++++ src/services/shims.ts | 12 ++++++++++++ src/services/types.ts | 1 + tests/cases/fourslash/fourslash.ts | 1 + tests/cases/fourslash/isInMultiLineComment.ts | 6 ++++++ 10 files changed, 78 insertions(+) create mode 100644 tests/cases/fourslash/isInMultiLineComment.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index fc50e71478e..ed29e74f66f 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2427,6 +2427,16 @@ namespace FourSlash { } } + public verifyIsInMultiLineComment(negative: boolean) { + const expected = !negative; + const position = this.currentCaretPosition; + const fileName = this.activeFile.fileName; + const actual = this.languageService.getIsInMultiLineComment(fileName, position); + if (expected !== actual) { + this.raiseError(`verifyIsInDocComment failed: at position '${position}' in '${fileName}', expected '${expected}'.`); + } + } + private clarifyNewlines(str: string) { return str.replace(/\r?\n/g, lineEnding => { const representation = lineEnding === "\r\n" ? "CRLF" : "LF"; @@ -3577,6 +3587,10 @@ namespace FourSlashInterface { this.state.verifyCodeFixAvailable(this.negative); } + public isInMultiLineComment() { + this.state.verifyIsInMultiLineComment(this.negative); + } + public applicableRefactorAvailableAtMarker(markerName: string) { this.state.verifyApplicableRefactorAvailableAtMarker(this.negative, markerName); } diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 7aefb0f3a1f..c67dde5f67b 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -483,6 +483,9 @@ namespace Harness.LanguageService { getDocCommentTemplateAtPosition(fileName: string, position: number): ts.TextInsertion { return unwrapJSONCallResult(this.shim.getDocCommentTemplateAtPosition(fileName, position)); } + getIsInMultiLineComment(fileName: string, position: number): boolean { + return unwrapJSONCallResult(this.shim.getIsInMultiLineComment(fileName, position)); + } isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { return unwrapJSONCallResult(this.shim.isValidBraceCompletionAtPosition(fileName, position, openingBrace)); } diff --git a/src/server/client.ts b/src/server/client.ts index e29261f7244..779d3bce6bf 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -672,6 +672,10 @@ namespace ts.server { return notImplemented(); } + getIsInMultiLineComment(_fileName: string, _position: number): boolean { + return notImplemented(); + } + isValidBraceCompletionAtPosition(_fileName: string, _position: number, _openingBrace: number): boolean { return notImplemented(); } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 8d85ad125d5..2df147a4d37 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -84,6 +84,7 @@ namespace ts.server.protocol { export type TodoComments = "todoComments"; export type Indentation = "indentation"; export type DocCommentTemplate = "docCommentTemplate"; + export type IsInMultiLineComment = "isInMultiLineComment"; /* @internal */ export type CompilerOptionsDiagnosticsFull = "compilerOptionsDiagnostics-full"; /* @internal */ @@ -237,6 +238,20 @@ namespace ts.server.protocol { body?: TodoComment[]; } + /** + * A request to determine if the caret is inside a multi-line comment. + */ + export interface IsInMultiLineCommentRequest extends FileLocationRequest { + command: CommandTypes.IsInMultiLineComment; + } + + /** + * Response for TodoCommentRequest request. + */ + export interface IsInMultiLineCommentResponse extends Response { + body?: { isInMultiLineComment: boolean }; + } + /** * Request to obtain outlining spans in file. */ diff --git a/src/server/session.ts b/src/server/session.ts index bae53dcafe8..a79e572e11b 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -194,6 +194,7 @@ namespace ts.server { export const TodoComments: protocol.CommandTypes.TodoComments = "todoComments"; export const Indentation: protocol.CommandTypes.Indentation = "indentation"; export const DocCommentTemplate: protocol.CommandTypes.DocCommentTemplate = "docCommentTemplate"; + export const IsInMultiLineComment: protocol.CommandTypes.IsInMultiLineComment = "isInMultiLineComment"; /* @internal */ export const CompilerOptionsDiagnosticsFull: protocol.CommandTypes.CompilerOptionsDiagnosticsFull = "compilerOptionsDiagnostics-full"; /* @internal */ @@ -1049,6 +1050,13 @@ namespace ts.server { return project.getLanguageService(/*ensureSynchronized*/ false).getDocCommentTemplateAtPosition(file, position); } + private getIsInMultiLineComment(args: protocol.FileLocationRequestArgs) { + const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); + const scriptInfo = project.getScriptInfoForNormalizedPath(file); + const position = this.getPosition(args, scriptInfo); + return project.getLanguageService(/*ensureSynchronized*/ false).getIsInMultiLineComment(file, position); + } + private getIndentation(args: protocol.IndentationRequestArgs) { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const position = this.getPosition(args, project.getScriptInfoForNormalizedPath(file)); @@ -1782,6 +1790,9 @@ namespace ts.server { [CommandNames.DocCommentTemplate]: (request: protocol.DocCommentTemplateRequest) => { return this.requiredResponse(this.getDocCommentTemplate(request.arguments)); }, + [CommandNames.IsInMultiLineComment]: (request: protocol.IsInMultiLineCommentRequest) => { + return this.requiredResponse(this.getIsInMultiLineComment(request.arguments)); + }, [CommandNames.Format]: (request: protocol.FormatRequest) => { return this.requiredResponse(this.getFormattingEditsForRange(request.arguments)); }, diff --git a/src/services/services.ts b/src/services/services.ts index 1a1ac47847f..83c94e91c58 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1779,6 +1779,16 @@ namespace ts { return JsDoc.getDocCommentTemplateAtPosition(getNewLineOrDefaultFromHost(host), syntaxTreeCache.getCurrentSourceFile(fileName), position); } + function getIsInMultiLineComment(_fileName: string, position: number): boolean { + const sourceFile = syntaxTreeCache.getCurrentSourceFile(_fileName); + const token = getTokenAtPosition(sourceFile, position); + const _triviaWidth = token.getLeadingTriviaWidth(sourceFile); _triviaWidth; + const _text = token.getText(sourceFile); _text; + const _fullText = token.getFullText(sourceFile); _fullText; + // TODO: distinguish multi-line and single line comments... + return token.getFullStart() <= position && position < token.getStart(sourceFile, /*includeJsDocComment*/ false); + } + function isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { // '<' is currently not supported, figuring out if we're in a Generic Type vs. a comparison is too // expensive to do during typing scenarios @@ -2030,6 +2040,7 @@ namespace ts { getFormattingEditsForDocument, getFormattingEditsAfterKeystroke, getDocCommentTemplateAtPosition, + getIsInMultiLineComment, isValidBraceCompletionAtPosition, getCodeFixesAtPosition, getEmitOutput, diff --git a/src/services/shims.ts b/src/services/shims.ts index 498c1834a60..3ce553d552e 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -247,6 +247,11 @@ namespace ts { */ getDocCommentTemplateAtPosition(fileName: string, position: number): string; + /** + * Returns JSON-encoded value of the type TextInsertion. + */ + getIsInMultiLineComment(fileName: string, position: number): string; + /** * Returns JSON-encoded boolean to indicate whether we should support brace location * at the current position. @@ -935,6 +940,13 @@ namespace ts { ); } + public getIsInMultiLineComment(fileName: string, position: number): string { + return this.forwardJSONCall( + `getIsInMultiLineComment('${fileName}', ${position})`, + () => this.languageService.getIsInMultiLineComment(fileName, position) + ); + } + /// NAVIGATE TO /** Return a list of symbols that are interesting to navigate to */ diff --git a/src/services/types.ts b/src/services/types.ts index 6c64ce5b9ba..20505378882 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -257,6 +257,7 @@ namespace ts { getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[]; getDocCommentTemplateAtPosition(fileName: string, position: number): TextInsertion; + getIsInMultiLineComment(fileName: string, position: number): boolean; isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 3fada673f35..3af3464138a 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -150,6 +150,7 @@ declare namespace FourSlashInterface { implementationListIsEmpty(): void; isValidBraceCompletionAtPosition(openingBrace?: string): void; codeFixAvailable(): void; + isInMultiLineComment(): void; applicableRefactorAvailableAtMarker(markerName: string): void; codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; applicableRefactorAvailableForRange(): void; diff --git a/tests/cases/fourslash/isInMultiLineComment.ts b/tests/cases/fourslash/isInMultiLineComment.ts new file mode 100644 index 00000000000..4d6c8d6af1a --- /dev/null +++ b/tests/cases/fourslash/isInMultiLineComment.ts @@ -0,0 +1,6 @@ +/// + +//// /* blach /*m0*/ */ let x = 10; + +goTo.marker("m0"); +verify.isInMultiLineComment(); \ No newline at end of file From a819e4ed172405bffe6fb419483bdccaeb1de16b Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 7 Jun 2017 16:43:13 -0700 Subject: [PATCH 02/50] isInMultiLineComment --- src/services/services.ts | 17 +++++---- tests/cases/fourslash/isInMultiLineComment.ts | 36 +++++++++++++++++-- .../fourslash/isInMultiLineCommentEOF.ts | 12 +++++++ 3 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 tests/cases/fourslash/isInMultiLineCommentEOF.ts diff --git a/src/services/services.ts b/src/services/services.ts index 8da654140e0..1f85ad427f5 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1791,12 +1791,17 @@ namespace ts { function getIsInMultiLineComment(_fileName: string, position: number): boolean { const sourceFile = syntaxTreeCache.getCurrentSourceFile(_fileName); - const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ true); - const _triviaWidth = token.getLeadingTriviaWidth(sourceFile); _triviaWidth; - const _text = token.getText(sourceFile); _text; - const _fullText = token.getFullText(sourceFile); _fullText; - // TODO: distinguish multi-line and single line comments... - return token.getFullStart() <= position && position < token.getStart(sourceFile, /*includeJsDocComment*/ false); + const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); + const leadingCommentRanges = getLeadingCommentRangesOfNode(token, sourceFile); + + for (const range of leadingCommentRanges) { + // We need to extend the range when in an unclosed multi-line comment. + if (range.pos < position && (position < range.end || position === range.end && position === sourceFile.getFullWidth())) { + return range.kind === SyntaxKind.MultiLineCommentTrivia; + } + } + + return false; } function isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { diff --git a/tests/cases/fourslash/isInMultiLineComment.ts b/tests/cases/fourslash/isInMultiLineComment.ts index 4d6c8d6af1a..ebcac7215f9 100644 --- a/tests/cases/fourslash/isInMultiLineComment.ts +++ b/tests/cases/fourslash/isInMultiLineComment.ts @@ -1,6 +1,36 @@ /// -//// /* blach /*m0*/ */ let x = 10; +//// /* x */ +//// /** x */ +//// // x +//// let x = 1; -goTo.marker("m0"); -verify.isInMultiLineComment(); \ No newline at end of file + +for (let i = 1; i < 7; ++i) { + goTo.position(i); + verify.isInMultiLineComment(); +} + +for (let i = 0; i < 2; ++i) { + goTo.position(i * 7); + verify.not.isInMultiLineComment(); +} + +const jsDocStart = 8; + +for (let i = 1; i < 8; ++i) { + goTo.position(jsDocStart + i); + verify.isInMultiLineComment(); +} + +for (let i = 0; i < 2; ++i) { + goTo.position(jsDocStart + i * 8); + verify.not.isInMultiLineComment(); +} + +const singleLineCommentStart = 17; + +for (let i = 0; i < 5; ++i) { + goTo.position(singleLineCommentStart + i); + verify.not.isInMultiLineComment(); +} diff --git a/tests/cases/fourslash/isInMultiLineCommentEOF.ts b/tests/cases/fourslash/isInMultiLineCommentEOF.ts new file mode 100644 index 00000000000..2e34264b1f6 --- /dev/null +++ b/tests/cases/fourslash/isInMultiLineCommentEOF.ts @@ -0,0 +1,12 @@ +/// + +// @Filename: f1.ts +//// /* /*0*/ blah /*1*/ */ + +// @Filename: f2.ts +//// /* /*2*/ blah /*3*/ + +for (let i = 0; i < 4; ++i) { + goTo.marker(`${i}`); + verify.isInMultiLineComment(); +} \ No newline at end of file From 8f28a0264ffcaa75e99617f9888f46e6c8f2ff41 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 8 Jun 2017 17:08:07 -0700 Subject: [PATCH 03/50] indent block comments according to first line --- src/harness/fourslash.ts | 28 +++++++++---------- src/harness/harnessLanguageService.ts | 6 ++-- src/server/client.ts | 4 +-- src/server/protocol.ts | 13 ++------- src/server/session.ts | 8 +++--- src/services/formatting/formatting.ts | 21 ++++++++++++++ src/services/formatting/smartIndenter.ts | 7 ++++- src/services/services.ts | 22 ++++----------- src/services/shims.ts | 25 +++++++++-------- src/services/types.ts | 3 +- tests/cases/fourslash/fourslash.ts | 2 +- tests/cases/fourslash/isInMultiLineComment.ts | 10 +++---- .../fourslash/isInMultiLineCommentEOF.ts | 2 +- 13 files changed, 81 insertions(+), 70 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index aa3097debad..54b5b48da2b 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2427,16 +2427,6 @@ namespace FourSlash { } } - public verifyIsInMultiLineComment(negative: boolean) { - const expected = !negative; - const position = this.currentCaretPosition; - const fileName = this.activeFile.fileName; - const actual = this.languageService.getIsInMultiLineComment(fileName, position); - if (expected !== actual) { - this.raiseError(`verifyIsInDocComment failed: at position '${position}' in '${fileName}', expected '${expected}'.`); - } - } - private clarifyNewlines(str: string) { return str.replace(/\r?\n/g, lineEnding => { const representation = lineEnding === "\r\n" ? "CRLF" : "LF"; @@ -2510,6 +2500,16 @@ namespace FourSlash { } } + public verifyisInMultiLineCommentAtPosition(negative: boolean) { + const expected = !negative; + const position = this.currentCaretPosition; + const fileName = this.activeFile.fileName; + const actual = this.languageService.getisInMultiLineCommentAtPosition(fileName, position); + if (expected !== actual) { + this.raiseError(`verifyIsInDocComment failed: at position '${position}' in '${fileName}', expected '${expected}'.`); + } + } + /* Check number of navigationItems which match both searchValue and matchKind, if a filename is passed in, limit the results to that file. @@ -3583,12 +3583,12 @@ namespace FourSlashInterface { this.state.verifyBraceCompletionAtPosition(this.negative, openingBrace); } - public codeFixAvailable() { - this.state.verifyCodeFixAvailable(this.negative); + public isInMultiLineCommentAtPosition() { + this.state.verifyisInMultiLineCommentAtPosition(this.negative); } - public isInMultiLineComment() { - this.state.verifyIsInMultiLineComment(this.negative); + public codeFixAvailable() { + this.state.verifyCodeFixAvailable(this.negative); } public applicableRefactorAvailableAtMarker(markerName: string) { diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index c67dde5f67b..c3a09b3926f 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -483,12 +483,12 @@ namespace Harness.LanguageService { getDocCommentTemplateAtPosition(fileName: string, position: number): ts.TextInsertion { return unwrapJSONCallResult(this.shim.getDocCommentTemplateAtPosition(fileName, position)); } - getIsInMultiLineComment(fileName: string, position: number): boolean { - return unwrapJSONCallResult(this.shim.getIsInMultiLineComment(fileName, position)); - } isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { return unwrapJSONCallResult(this.shim.isValidBraceCompletionAtPosition(fileName, position, openingBrace)); } + getisInMultiLineCommentAtPosition(fileName: string, position: number): boolean { + return unwrapJSONCallResult(this.shim.getisInMultiLineCommentAtPosition(fileName, position)); + } getCodeFixesAtPosition(): ts.CodeAction[] { throw new Error("Not supported on the shim."); } diff --git a/src/server/client.ts b/src/server/client.ts index 69727a18e60..5a162c1041d 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -672,11 +672,11 @@ namespace ts.server { return notImplemented(); } - getIsInMultiLineComment(_fileName: string, _position: number): boolean { + isValidBraceCompletionAtPosition(_fileName: string, _position: number, _openingBrace: number): boolean { return notImplemented(); } - isValidBraceCompletionAtPosition(_fileName: string, _position: number, _openingBrace: number): boolean { + getisInMultiLineCommentAtPosition(_fileName: string, _position: number): boolean { return notImplemented(); } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 1cfe617ca72..299ba4fceed 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -8,6 +8,7 @@ namespace ts.server.protocol { /* @internal */ BraceFull = "brace-full", BraceCompletion = "braceCompletion", + isInMultiLineComment = "isInMultiLineComment", Change = "change", Close = "close", Completions = "completions", @@ -85,7 +86,6 @@ namespace ts.server.protocol { TodoComments = "todoComments", Indentation = "indentation", DocCommentTemplate = "docCommentTemplate", - IsInMultiLineComment = "isInMultiLineComment", /* @internal */ CompilerOptionsDiagnosticsFull = "compilerOptionsDiagnostics-full", /* @internal */ @@ -242,15 +242,8 @@ namespace ts.server.protocol { /** * A request to determine if the caret is inside a multi-line comment. */ - export interface IsInMultiLineCommentRequest extends FileLocationRequest { - command: CommandTypes.IsInMultiLineComment; - } - - /** - * Response for TodoCommentRequest request. - */ - export interface IsInMultiLineCommentResponse extends Response { - body?: { isInMultiLineComment: boolean }; + export interface IsInMultiLineCommentAtPositionRequest extends FileLocationRequest { + command: CommandTypes.isInMultiLineComment; } /** diff --git a/src/server/session.ts b/src/server/session.ts index abebdb5fae9..e4e900ddce7 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -961,11 +961,11 @@ namespace ts.server { return project.getLanguageService(/*ensureSynchronized*/ false).getDocCommentTemplateAtPosition(file, position); } - private getIsInMultiLineComment(args: protocol.FileLocationRequestArgs) { + private getisInMultiLineCommentAtPosition(args: protocol.FileLocationRequestArgs) { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const position = this.getPosition(args, scriptInfo); - return project.getLanguageService(/*ensureSynchronized*/ false).getIsInMultiLineComment(file, position); + return project.getLanguageService(/*ensureSynchronized*/ false).getisInMultiLineCommentAtPosition(file, position); } private getIndentation(args: protocol.IndentationRequestArgs) { @@ -1701,8 +1701,8 @@ namespace ts.server { [CommandNames.DocCommentTemplate]: (request: protocol.DocCommentTemplateRequest) => { return this.requiredResponse(this.getDocCommentTemplate(request.arguments)); }, - [CommandNames.IsInMultiLineComment]: (request: protocol.IsInMultiLineCommentRequest) => { - return this.requiredResponse(this.getIsInMultiLineComment(request.arguments)); + [CommandNames.isInMultiLineComment]: (request: protocol.IsInMultiLineCommentAtPositionRequest) => { + return this.requiredResponse(this.getisInMultiLineCommentAtPosition(request.arguments)); }, [CommandNames.Format]: (request: protocol.FormatRequest) => { return this.requiredResponse(this.getFormattingEditsForRange(request.arguments)); diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 531b768f6d8..fa49ee852b1 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1117,6 +1117,27 @@ namespace ts.formatting { } } + /** + * @returns -1 iff the position is not in a multi-line comment. + */ + export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number): number { + const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); + const leadingCommentRanges = getLeadingCommentRangesOfNode(token, sourceFile); + if (leadingCommentRanges) { + loop: for (const range of leadingCommentRanges) { + // We need to extend the range when in an unclosed multi-line comment. + if (range.pos < position && (position < range.end || position === range.end && position === sourceFile.getFullWidth())) { + if (range.kind === SyntaxKind.MultiLineCommentTrivia) { + return range.pos - getLineStartPositionForPosition(range.pos, sourceFile); + } + break loop; + } + } + } + return -1; + } + + function getOpenTokenForList(node: Node, list: Node[]) { switch (node.kind) { case SyntaxKind.Constructor: diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 18b0479c85a..8acab0e6603 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -38,12 +38,17 @@ namespace ts.formatting { // no indentation in string \regex\template literals const precedingTokenIsLiteral = isStringOrRegularExpressionOrTemplateLiteral(precedingToken.kind); - if (precedingTokenIsLiteral && precedingToken.getStart(sourceFile) <= position && precedingToken.end > position) { + if (precedingTokenIsLiteral && precedingToken.getStart(sourceFile) <= position && position < precedingToken.end) { return 0; } const lineAtPosition = sourceFile.getLineAndCharacterOfPosition(position).line; + const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position); + if (indentationOfEnclosingMultiLineComment !== -1) { + return indentationOfEnclosingMultiLineComment; + } + // indentation is first non-whitespace character in a previous line // for block indentation, we should look for a line which contains something that's not // whitespace. diff --git a/src/services/services.ts b/src/services/services.ts index 1f85ad427f5..ab398db7c35 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1789,21 +1789,6 @@ namespace ts { return JsDoc.getDocCommentTemplateAtPosition(getNewLineOrDefaultFromHost(host), syntaxTreeCache.getCurrentSourceFile(fileName), position); } - function getIsInMultiLineComment(_fileName: string, position: number): boolean { - const sourceFile = syntaxTreeCache.getCurrentSourceFile(_fileName); - const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); - const leadingCommentRanges = getLeadingCommentRangesOfNode(token, sourceFile); - - for (const range of leadingCommentRanges) { - // We need to extend the range when in an unclosed multi-line comment. - if (range.pos < position && (position < range.end || position === range.end && position === sourceFile.getFullWidth())) { - return range.kind === SyntaxKind.MultiLineCommentTrivia; - } - } - - return false; - } - function isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { // '<' is currently not supported, figuring out if we're in a Generic Type vs. a comparison is too // expensive to do during typing scenarios @@ -1833,6 +1818,11 @@ namespace ts { return true; } + function getisInMultiLineCommentAtPosition(fileName: string, position: number): boolean { + const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); + return ts.formatting.getIndentationOfEnclosingMultiLineComment(sourceFile, position) !== -1; + } + function getTodoComments(fileName: string, descriptors: TodoCommentDescriptor[]): TodoComment[] { // Note: while getting todo comments seems like a syntactic operation, we actually // treat it as a semantic operation here. This is because we expect our host to call @@ -2054,8 +2044,8 @@ namespace ts { getFormattingEditsForDocument, getFormattingEditsAfterKeystroke, getDocCommentTemplateAtPosition, - getIsInMultiLineComment, isValidBraceCompletionAtPosition, + getisInMultiLineCommentAtPosition, getCodeFixesAtPosition, getEmitOutput, getNonBoundSourceFile, diff --git a/src/services/shims.ts b/src/services/shims.ts index 3ce553d552e..75c5e8445c1 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -247,11 +247,6 @@ namespace ts { */ getDocCommentTemplateAtPosition(fileName: string, position: number): string; - /** - * Returns JSON-encoded value of the type TextInsertion. - */ - getIsInMultiLineComment(fileName: string, position: number): string; - /** * Returns JSON-encoded boolean to indicate whether we should support brace location * at the current position. @@ -259,6 +254,11 @@ namespace ts { */ isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): string; + /** + * Returns JSON-encoded boolean to indicate whether the caret at the current position is in a multi-line comment. + */ + getisInMultiLineCommentAtPosition(fileName: string, position: number): string; + getEmitOutput(fileName: string): string; getEmitOutputObject(fileName: string): EmitOutput; } @@ -840,6 +840,14 @@ namespace ts { ); } + /// GET IS IN MULTI-LINE COMMENT + public getisInMultiLineCommentAtPosition(fileName: string, position: number): string { + return this.forwardJSONCall( + `getisInMultiLineCommentAtPosition('${fileName}', ${position})`, + () => this.languageService.getisInMultiLineCommentAtPosition(fileName, position) + ); + } + /// GET SMART INDENT public getIndentationAtPosition(fileName: string, position: number, options: string /*Services.EditorOptions*/): string { return this.forwardJSONCall( @@ -940,13 +948,6 @@ namespace ts { ); } - public getIsInMultiLineComment(fileName: string, position: number): string { - return this.forwardJSONCall( - `getIsInMultiLineComment('${fileName}', ${position})`, - () => this.languageService.getIsInMultiLineComment(fileName, position) - ); - } - /// NAVIGATE TO /** Return a list of symbols that are interesting to navigate to */ diff --git a/src/services/types.ts b/src/services/types.ts index 62d2db1b65c..3a7a14d3dbc 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -257,10 +257,11 @@ namespace ts { getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[]; getDocCommentTemplateAtPosition(fileName: string, position: number): TextInsertion; - getIsInMultiLineComment(fileName: string, position: number): boolean; isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean; + getisInMultiLineCommentAtPosition(fileName: string, position: number): boolean; + getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[] | undefined; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 3984a074fd9..96cc152f0ff 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -149,8 +149,8 @@ declare namespace FourSlashInterface { typeDefinitionCountIs(expectedCount: number): void; implementationListIsEmpty(): void; isValidBraceCompletionAtPosition(openingBrace?: string): void; + isInMultiLineCommentAtPosition(): void; codeFixAvailable(): void; - isInMultiLineComment(): void; applicableRefactorAvailableAtMarker(markerName: string): void; codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; applicableRefactorAvailableForRange(): void; diff --git a/tests/cases/fourslash/isInMultiLineComment.ts b/tests/cases/fourslash/isInMultiLineComment.ts index ebcac7215f9..39aac71a8bd 100644 --- a/tests/cases/fourslash/isInMultiLineComment.ts +++ b/tests/cases/fourslash/isInMultiLineComment.ts @@ -8,29 +8,29 @@ for (let i = 1; i < 7; ++i) { goTo.position(i); - verify.isInMultiLineComment(); + verify.isInMultiLineCommentAtPosition(); } for (let i = 0; i < 2; ++i) { goTo.position(i * 7); - verify.not.isInMultiLineComment(); + verify.not.isInMultiLineCommentAtPosition(); } const jsDocStart = 8; for (let i = 1; i < 8; ++i) { goTo.position(jsDocStart + i); - verify.isInMultiLineComment(); + verify.isInMultiLineCommentAtPosition(); } for (let i = 0; i < 2; ++i) { goTo.position(jsDocStart + i * 8); - verify.not.isInMultiLineComment(); + verify.not.isInMultiLineCommentAtPosition(); } const singleLineCommentStart = 17; for (let i = 0; i < 5; ++i) { goTo.position(singleLineCommentStart + i); - verify.not.isInMultiLineComment(); + verify.not.isInMultiLineCommentAtPosition(); } diff --git a/tests/cases/fourslash/isInMultiLineCommentEOF.ts b/tests/cases/fourslash/isInMultiLineCommentEOF.ts index 2e34264b1f6..2b65a958340 100644 --- a/tests/cases/fourslash/isInMultiLineCommentEOF.ts +++ b/tests/cases/fourslash/isInMultiLineCommentEOF.ts @@ -8,5 +8,5 @@ for (let i = 0; i < 4; ++i) { goTo.marker(`${i}`); - verify.isInMultiLineComment(); + verify.isInMultiLineCommentAtPosition(); } \ No newline at end of file From b02963b2380f8cd4c2675a72f974552e5acd7f1e Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 9 Jun 2017 14:43:28 -0700 Subject: [PATCH 04/50] make indent work with trailing comments --- src/harness/fourslash.ts | 8 +-- src/harness/harnessLanguageService.ts | 2 +- src/server/client.ts | 2 +- src/server/session.ts | 2 +- src/services/formatting/formatting.ts | 35 +++++++---- src/services/formatting/smartIndenter.ts | 18 +++--- src/services/services.ts | 6 +- src/services/shims.ts | 2 +- src/services/types.ts | 2 +- tests/cases/fourslash/isInMultiLineComment.ts | 58 +++++++++++-------- 10 files changed, 79 insertions(+), 56 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 54b5b48da2b..080614fc623 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2500,13 +2500,13 @@ namespace FourSlash { } } - public verifyisInMultiLineCommentAtPosition(negative: boolean) { + public verifyIsInMultiLineCommentAtPosition(negative: boolean) { const expected = !negative; const position = this.currentCaretPosition; const fileName = this.activeFile.fileName; - const actual = this.languageService.getisInMultiLineCommentAtPosition(fileName, position); + const actual = this.languageService.isInMultiLineCommentAtPosition(fileName, position); if (expected !== actual) { - this.raiseError(`verifyIsInDocComment failed: at position '${position}' in '${fileName}', expected '${expected}'.`); + this.raiseError(`verifyIsInMultiLineCommentAtPosition failed: at position '${position}' in '${fileName}', expected '${expected}'.`); } } @@ -3584,7 +3584,7 @@ namespace FourSlashInterface { } public isInMultiLineCommentAtPosition() { - this.state.verifyisInMultiLineCommentAtPosition(this.negative); + this.state.verifyIsInMultiLineCommentAtPosition(this.negative); } public codeFixAvailable() { diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index c3a09b3926f..962f9a0df5d 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -486,7 +486,7 @@ namespace Harness.LanguageService { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { return unwrapJSONCallResult(this.shim.isValidBraceCompletionAtPosition(fileName, position, openingBrace)); } - getisInMultiLineCommentAtPosition(fileName: string, position: number): boolean { + isInMultiLineCommentAtPosition(fileName: string, position: number): boolean { return unwrapJSONCallResult(this.shim.getisInMultiLineCommentAtPosition(fileName, position)); } getCodeFixesAtPosition(): ts.CodeAction[] { diff --git a/src/server/client.ts b/src/server/client.ts index 5a162c1041d..0e66516da40 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -676,7 +676,7 @@ namespace ts.server { return notImplemented(); } - getisInMultiLineCommentAtPosition(_fileName: string, _position: number): boolean { + isInMultiLineCommentAtPosition(_fileName: string, _position: number): boolean { return notImplemented(); } diff --git a/src/server/session.ts b/src/server/session.ts index e4e900ddce7..9657c666238 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -965,7 +965,7 @@ namespace ts.server { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const position = this.getPosition(args, scriptInfo); - return project.getLanguageService(/*ensureSynchronized*/ false).getisInMultiLineCommentAtPosition(file, position); + return project.getLanguageService(/*ensureSynchronized*/ false).isInMultiLineCommentAtPosition(file, position); } private getIndentation(args: protocol.IndentationRequestArgs) { diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index fa49ee852b1..7feba4b7dd2 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1118,23 +1118,36 @@ namespace ts.formatting { } /** - * @returns -1 iff the position is not in a multi-line comment. + * Gets the indentation level of the multi-line comment enclosing position, + * and a negative value if the position is not in a multi-line comment. */ - export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number): number { - const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); - const leadingCommentRanges = getLeadingCommentRangesOfNode(token, sourceFile); - if (leadingCommentRanges) { - loop: for (const range of leadingCommentRanges) { + export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number, options: EditorSettings): number { + const range = getRangeOfEnclosingComment(sourceFile, position, SyntaxKind.MultiLineCommentTrivia); + if (range) { + const commentStart = range.pos; + const commentLineStart = getLineStartPositionForPosition(commentStart, sourceFile); + const { column, character } = SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(commentLineStart, commentStart, sourceFile, options); + return range.pos - character + column; + } + return undefined; + } + + export function getRangeOfEnclosingComment(sourceFile: SourceFile, position: number, kind: CommentKind): CommentRange | undefined { + const precedingToken = findPrecedingToken(position, sourceFile); + const trailingRangesOfPreviousToken = precedingToken && getTrailingCommentRanges(sourceFile.text, precedingToken.end); + const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), sourceFile); + const commentRanges = trailingRangesOfPreviousToken && leadingCommentRangesOfNextToken ? + trailingRangesOfPreviousToken.concat(leadingCommentRangesOfNextToken) : + trailingRangesOfPreviousToken || leadingCommentRangesOfNextToken; + if (commentRanges) { + for (const range of commentRanges) { // We need to extend the range when in an unclosed multi-line comment. if (range.pos < position && (position < range.end || position === range.end && position === sourceFile.getFullWidth())) { - if (range.kind === SyntaxKind.MultiLineCommentTrivia) { - return range.pos - getLineStartPositionForPosition(range.pos, sourceFile); - } - break loop; + return range.kind === kind ? range : undefined; } } } - return -1; + return undefined; } diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 8acab0e6603..c5aee567a41 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -44,8 +44,8 @@ namespace ts.formatting { const lineAtPosition = sourceFile.getLineAndCharacterOfPosition(position).line; - const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position); - if (indentationOfEnclosingMultiLineComment !== -1) { + const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position, options); + if (indentationOfEnclosingMultiLineComment >= 0) { return indentationOfEnclosingMultiLineComment; } @@ -410,13 +410,13 @@ namespace ts.formatting { return findFirstNonWhitespaceColumn(lineStart, lineStart + lineAndCharacter.character, sourceFile, options); } - /* - Character is the actual index of the character since the beginning of the line. - Column - position of the character after expanding tabs to spaces - "0\t2$" - value of 'character' for '$' is 3 - value of 'column' for '$' is 6 (assuming that tab size is 4) - */ + /** + * Character is the actual index of the character since the beginning of the line. + * Column - position of the character after expanding tabs to spaces. + * "0\t2$" + * value of 'character' for '$' is 3 + * value of 'column' for '$' is 6 (assuming that tab size is 4) + */ export function findFirstNonWhitespaceCharacterAndColumn(startPos: number, endPos: number, sourceFile: SourceFileLike, options: EditorSettings) { let character = 0; let column = 0; diff --git a/src/services/services.ts b/src/services/services.ts index ab398db7c35..294afec6029 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1818,9 +1818,9 @@ namespace ts { return true; } - function getisInMultiLineCommentAtPosition(fileName: string, position: number): boolean { + function isInMultiLineCommentAtPosition(fileName: string, position: number): boolean { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - return ts.formatting.getIndentationOfEnclosingMultiLineComment(sourceFile, position) !== -1; + return !!ts.formatting.getRangeOfEnclosingComment(sourceFile, position, SyntaxKind.MultiLineCommentTrivia); } function getTodoComments(fileName: string, descriptors: TodoCommentDescriptor[]): TodoComment[] { @@ -2045,7 +2045,7 @@ namespace ts { getFormattingEditsAfterKeystroke, getDocCommentTemplateAtPosition, isValidBraceCompletionAtPosition, - getisInMultiLineCommentAtPosition, + isInMultiLineCommentAtPosition, getCodeFixesAtPosition, getEmitOutput, getNonBoundSourceFile, diff --git a/src/services/shims.ts b/src/services/shims.ts index 75c5e8445c1..fd2ba1cca72 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -844,7 +844,7 @@ namespace ts { public getisInMultiLineCommentAtPosition(fileName: string, position: number): string { return this.forwardJSONCall( `getisInMultiLineCommentAtPosition('${fileName}', ${position})`, - () => this.languageService.getisInMultiLineCommentAtPosition(fileName, position) + () => this.languageService.isInMultiLineCommentAtPosition(fileName, position) ); } diff --git a/src/services/types.ts b/src/services/types.ts index 3a7a14d3dbc..bbd9fa9986c 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -260,7 +260,7 @@ namespace ts { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean; - getisInMultiLineCommentAtPosition(fileName: string, position: number): boolean; + isInMultiLineCommentAtPosition(fileName: string, position: number): boolean; getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; diff --git a/tests/cases/fourslash/isInMultiLineComment.ts b/tests/cases/fourslash/isInMultiLineComment.ts index 39aac71a8bd..58e4e4d3337 100644 --- a/tests/cases/fourslash/isInMultiLineComment.ts +++ b/tests/cases/fourslash/isInMultiLineComment.ts @@ -1,36 +1,46 @@ /// //// /* x */ -//// /** x */ +//// /** +//// * @param this doesn't make sense here. +//// */ //// // x -//// let x = 1; +//// let x = 1; /* +//// * +const firstCommentStart = 0; +const firstCommentEnd = 7; +goTo.position(firstCommentStart); +verify.not.isInMultiLineCommentAtPosition(); -for (let i = 1; i < 7; ++i) { - goTo.position(i); - verify.isInMultiLineCommentAtPosition(); -} +goTo.position(firstCommentStart + 1); +verify.isInMultiLineCommentAtPosition(); +goTo.position(firstCommentEnd - 1); +verify.isInMultiLineCommentAtPosition(); -for (let i = 0; i < 2; ++i) { - goTo.position(i * 7); - verify.not.isInMultiLineCommentAtPosition(); -} +goTo.position(firstCommentEnd); +verify.not.isInMultiLineCommentAtPosition(); -const jsDocStart = 8; +const multilineJsDocStart = firstCommentEnd + 1; +const multilineJsDocEnd = multilineJsDocStart + 49; -for (let i = 1; i < 8; ++i) { - goTo.position(jsDocStart + i); - verify.isInMultiLineCommentAtPosition(); -} +goTo.position(multilineJsDocStart); +verify.not.isInMultiLineCommentAtPosition(); +goTo.position(multilineJsDocStart + 1); +verify.isInMultiLineCommentAtPosition(); +goTo.position(multilineJsDocEnd - 1); +verify.isInMultiLineCommentAtPosition(); +goTo.position(multilineJsDocEnd); +verify.not.isInMultiLineCommentAtPosition(); -for (let i = 0; i < 2; ++i) { - goTo.position(jsDocStart + i * 8); - verify.not.isInMultiLineCommentAtPosition(); -} +const singleLineCommentStart = multilineJsDocEnd + 1; -const singleLineCommentStart = 17; +goTo.position(singleLineCommentStart + 1); +verify.not.isInMultiLineCommentAtPosition(); -for (let i = 0; i < 5; ++i) { - goTo.position(singleLineCommentStart + i); - verify.not.isInMultiLineCommentAtPosition(); -} +const postNodeCommentStart = singleLineCommentStart + 16; + +goTo.position(postNodeCommentStart); +verify.not.isInMultiLineCommentAtPosition(); +goTo.position(postNodeCommentStart + 1); +verify.isInMultiLineCommentAtPosition(); From 8fc3fd9a20fb30e8602d20c68c799449497a7870 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 9 Jun 2017 18:02:42 -0700 Subject: [PATCH 05/50] request returns span --- src/harness/fourslash.ts | 14 +++++++++----- src/harness/harnessLanguageService.ts | 4 ++-- src/server/client.ts | 2 +- src/server/protocol.ts | 16 ++++++++++++---- src/server/session.ts | 9 +++++---- src/services/formatting/formatting.ts | 6 +++--- src/services/services.ts | 7 ++++--- src/services/shims.ts | 11 +++++------ src/services/types.ts | 2 +- 9 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 080614fc623..3feb5c0b19d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2500,13 +2500,17 @@ namespace FourSlash { } } - public verifyIsInMultiLineCommentAtPosition(negative: boolean) { + public verifySpanOfEnclosingComment(negative: boolean, onlyMultiLine: boolean) { const expected = !negative; const position = this.currentCaretPosition; const fileName = this.activeFile.fileName; - const actual = this.languageService.isInMultiLineCommentAtPosition(fileName, position); + const actual = !!this.languageService.getSpanOfEnclosingComment(fileName, position, /*onlyMultiLine*/ onlyMultiLine); if (expected !== actual) { - this.raiseError(`verifyIsInMultiLineCommentAtPosition failed: at position '${position}' in '${fileName}', expected '${expected}'.`); + this.raiseError(`verifySpanOfEnclosingComment failed: + position: '${position}' + fileName: '${fileName}' + onlyMultiLine: '${onlyMultiLine}' + expected: '${expected}'.`); } } @@ -3583,8 +3587,8 @@ namespace FourSlashInterface { this.state.verifyBraceCompletionAtPosition(this.negative, openingBrace); } - public isInMultiLineCommentAtPosition() { - this.state.verifyIsInMultiLineCommentAtPosition(this.negative); + public isInCommentAtPosition(onlyMultiLine: boolean) { + this.state.verifySpanOfEnclosingComment(this.negative, onlyMultiLine); } public codeFixAvailable() { diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 962f9a0df5d..6d7eee5757f 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -486,8 +486,8 @@ namespace Harness.LanguageService { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean { return unwrapJSONCallResult(this.shim.isValidBraceCompletionAtPosition(fileName, position, openingBrace)); } - isInMultiLineCommentAtPosition(fileName: string, position: number): boolean { - return unwrapJSONCallResult(this.shim.getisInMultiLineCommentAtPosition(fileName, position)); + getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean): ts.TextSpan { + return unwrapJSONCallResult(this.shim.getSpanOfEnclosingComment(fileName, position, onlyMultiLine)); } getCodeFixesAtPosition(): ts.CodeAction[] { throw new Error("Not supported on the shim."); diff --git a/src/server/client.ts b/src/server/client.ts index 0e66516da40..5e5a6fcfcd5 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -676,7 +676,7 @@ namespace ts.server { return notImplemented(); } - isInMultiLineCommentAtPosition(_fileName: string, _position: number): boolean { + getSpanOfEnclosingComment(_fileName: string, _position: number, _onlyMultiLine: boolean): TextSpan { return notImplemented(); } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 299ba4fceed..0dc510c4f6e 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -8,7 +8,7 @@ namespace ts.server.protocol { /* @internal */ BraceFull = "brace-full", BraceCompletion = "braceCompletion", - isInMultiLineComment = "isInMultiLineComment", + GetSpanOfEnclosingComment = "getSpanOfEnclosingComment", Change = "change", Close = "close", Completions = "completions", @@ -240,10 +240,18 @@ namespace ts.server.protocol { } /** - * A request to determine if the caret is inside a multi-line comment. + * A request to determine if the caret is inside a comment. */ - export interface IsInMultiLineCommentAtPositionRequest extends FileLocationRequest { - command: CommandTypes.isInMultiLineComment; + export interface SpanOfEnclosingCommentRequest extends FileLocationRequest { + command: CommandTypes.GetSpanOfEnclosingComment; + arguments: SpanOfEnclosingCommentRequestArgs; + } + + export interface SpanOfEnclosingCommentRequestArgs extends FileLocationRequestArgs { + /** + * Requires that the enclosing span be a multi-line comment, or else the request returns undefined. + */ + onlyMultiLine: boolean; } /** diff --git a/src/server/session.ts b/src/server/session.ts index 9657c666238..2be7cade587 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -961,11 +961,12 @@ namespace ts.server { return project.getLanguageService(/*ensureSynchronized*/ false).getDocCommentTemplateAtPosition(file, position); } - private getisInMultiLineCommentAtPosition(args: protocol.FileLocationRequestArgs) { + private getSpanOfEnclosingComment(args: protocol.SpanOfEnclosingCommentRequestArgs) { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); + const onlyMultiLine = args.onlyMultiLine; const position = this.getPosition(args, scriptInfo); - return project.getLanguageService(/*ensureSynchronized*/ false).isInMultiLineCommentAtPosition(file, position); + return project.getLanguageService(/*ensureSynchronized*/ false).getSpanOfEnclosingComment(file, position, onlyMultiLine); } private getIndentation(args: protocol.IndentationRequestArgs) { @@ -1701,8 +1702,8 @@ namespace ts.server { [CommandNames.DocCommentTemplate]: (request: protocol.DocCommentTemplateRequest) => { return this.requiredResponse(this.getDocCommentTemplate(request.arguments)); }, - [CommandNames.isInMultiLineComment]: (request: protocol.IsInMultiLineCommentAtPositionRequest) => { - return this.requiredResponse(this.getisInMultiLineCommentAtPosition(request.arguments)); + [CommandNames.GetSpanOfEnclosingComment]: (request: protocol.SpanOfEnclosingCommentRequest) => { + return this.requiredResponse(this.getSpanOfEnclosingComment(request.arguments)); }, [CommandNames.Format]: (request: protocol.FormatRequest) => { return this.requiredResponse(this.getFormattingEditsForRange(request.arguments)); diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 7feba4b7dd2..08d2bd83616 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1122,7 +1122,7 @@ namespace ts.formatting { * and a negative value if the position is not in a multi-line comment. */ export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number, options: EditorSettings): number { - const range = getRangeOfEnclosingComment(sourceFile, position, SyntaxKind.MultiLineCommentTrivia); + const range = getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ true); if (range) { const commentStart = range.pos; const commentLineStart = getLineStartPositionForPosition(commentStart, sourceFile); @@ -1132,7 +1132,7 @@ namespace ts.formatting { return undefined; } - export function getRangeOfEnclosingComment(sourceFile: SourceFile, position: number, kind: CommentKind): CommentRange | undefined { + export function getRangeOfEnclosingComment(sourceFile: SourceFile, position: number, onlyMultiLine: boolean): CommentRange | undefined { const precedingToken = findPrecedingToken(position, sourceFile); const trailingRangesOfPreviousToken = precedingToken && getTrailingCommentRanges(sourceFile.text, precedingToken.end); const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), sourceFile); @@ -1143,7 +1143,7 @@ namespace ts.formatting { for (const range of commentRanges) { // We need to extend the range when in an unclosed multi-line comment. if (range.pos < position && (position < range.end || position === range.end && position === sourceFile.getFullWidth())) { - return range.kind === kind ? range : undefined; + return onlyMultiLine && range.kind !== SyntaxKind.MultiLineCommentTrivia ? undefined : range; } } } diff --git a/src/services/services.ts b/src/services/services.ts index 294afec6029..8234a100522 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1818,9 +1818,10 @@ namespace ts { return true; } - function isInMultiLineCommentAtPosition(fileName: string, position: number): boolean { + function getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean) { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - return !!ts.formatting.getRangeOfEnclosingComment(sourceFile, position, SyntaxKind.MultiLineCommentTrivia); + const range = ts.formatting.getRangeOfEnclosingComment(sourceFile, position, onlyMultiLine); + return range && createTextSpanFromRange(range); } function getTodoComments(fileName: string, descriptors: TodoCommentDescriptor[]): TodoComment[] { @@ -2045,7 +2046,7 @@ namespace ts { getFormattingEditsAfterKeystroke, getDocCommentTemplateAtPosition, isValidBraceCompletionAtPosition, - isInMultiLineCommentAtPosition, + getSpanOfEnclosingComment, getCodeFixesAtPosition, getEmitOutput, getNonBoundSourceFile, diff --git a/src/services/shims.ts b/src/services/shims.ts index fd2ba1cca72..2e9825e1457 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -255,9 +255,9 @@ namespace ts { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): string; /** - * Returns JSON-encoded boolean to indicate whether the caret at the current position is in a multi-line comment. + * Returns a JSON-encoded TextSpan | undefined indicating the range of the enclosing comment, if it exists. */ - getisInMultiLineCommentAtPosition(fileName: string, position: number): string; + getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean): string; getEmitOutput(fileName: string): string; getEmitOutputObject(fileName: string): EmitOutput; @@ -840,11 +840,10 @@ namespace ts { ); } - /// GET IS IN MULTI-LINE COMMENT - public getisInMultiLineCommentAtPosition(fileName: string, position: number): string { + public getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean): string { return this.forwardJSONCall( - `getisInMultiLineCommentAtPosition('${fileName}', ${position})`, - () => this.languageService.isInMultiLineCommentAtPosition(fileName, position) + `getSpanOfEnclosingComment('${fileName}', ${position})`, + () => this.languageService.getSpanOfEnclosingComment(fileName, position, onlyMultiLine) ); } diff --git a/src/services/types.ts b/src/services/types.ts index bbd9fa9986c..73bf75b5508 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -260,7 +260,7 @@ namespace ts { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean; - isInMultiLineCommentAtPosition(fileName: string, position: number): boolean; + getSpanOfEnclosingComment(fileName: string, position: number, onlyMultiLine: boolean): TextSpan; getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; From cc880915d5b1077477387fb6e4d16cb2fa21e30d Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 9 Jun 2017 21:19:29 -0700 Subject: [PATCH 06/50] fix offsetting and tests --- src/services/formatting/formatting.ts | 5 ++-- tests/cases/fourslash/fourslash.ts | 2 +- tests/cases/fourslash/isInMultiLineComment.ts | 23 ++++++++++--------- .../fourslash/isInMultiLineCommentEOF.ts | 12 ---------- 4 files changed, 16 insertions(+), 26 deletions(-) delete mode 100644 tests/cases/fourslash/isInMultiLineCommentEOF.ts diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 08d2bd83616..b5cede24e95 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1127,7 +1127,7 @@ namespace ts.formatting { const commentStart = range.pos; const commentLineStart = getLineStartPositionForPosition(commentStart, sourceFile); const { column, character } = SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(commentLineStart, commentStart, sourceFile, options); - return range.pos - character + column; + return column + /*length after whitespace ends*/ range.pos - (commentLineStart + character); } return undefined; } @@ -1142,7 +1142,8 @@ namespace ts.formatting { if (commentRanges) { for (const range of commentRanges) { // We need to extend the range when in an unclosed multi-line comment. - if (range.pos < position && (position < range.end || position === range.end && position === sourceFile.getFullWidth())) { + if (range.pos < position && position < range.end || + position === range.end && (range.kind === SyntaxKind.SingleLineCommentTrivia || position === sourceFile.getFullWidth())) { return onlyMultiLine && range.kind !== SyntaxKind.MultiLineCommentTrivia ? undefined : range; } } diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 96cc152f0ff..67ab1d2a2e2 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -149,7 +149,7 @@ declare namespace FourSlashInterface { typeDefinitionCountIs(expectedCount: number): void; implementationListIsEmpty(): void; isValidBraceCompletionAtPosition(openingBrace?: string): void; - isInMultiLineCommentAtPosition(): void; + isInCommentAtPosition(onlyMultiLine: boolean): void; codeFixAvailable(): void; applicableRefactorAvailableAtMarker(markerName: string): void; codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; diff --git a/tests/cases/fourslash/isInMultiLineComment.ts b/tests/cases/fourslash/isInMultiLineComment.ts index 58e4e4d3337..3dc8a6e15d1 100644 --- a/tests/cases/fourslash/isInMultiLineComment.ts +++ b/tests/cases/fourslash/isInMultiLineComment.ts @@ -11,36 +11,37 @@ const firstCommentStart = 0; const firstCommentEnd = 7; goTo.position(firstCommentStart); -verify.not.isInMultiLineCommentAtPosition(); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(firstCommentStart + 1); -verify.isInMultiLineCommentAtPosition(); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(firstCommentEnd - 1); -verify.isInMultiLineCommentAtPosition(); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(firstCommentEnd); -verify.not.isInMultiLineCommentAtPosition(); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); const multilineJsDocStart = firstCommentEnd + 1; const multilineJsDocEnd = multilineJsDocStart + 49; goTo.position(multilineJsDocStart); -verify.not.isInMultiLineCommentAtPosition(); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(multilineJsDocStart + 1); -verify.isInMultiLineCommentAtPosition(); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(multilineJsDocEnd - 1); -verify.isInMultiLineCommentAtPosition(); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(multilineJsDocEnd); -verify.not.isInMultiLineCommentAtPosition(); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); const singleLineCommentStart = multilineJsDocEnd + 1; goTo.position(singleLineCommentStart + 1); -verify.not.isInMultiLineCommentAtPosition(); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(/*onlyMultiLine*/ false); const postNodeCommentStart = singleLineCommentStart + 16; goTo.position(postNodeCommentStart); -verify.not.isInMultiLineCommentAtPosition(); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); goTo.position(postNodeCommentStart + 1); -verify.isInMultiLineCommentAtPosition(); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); diff --git a/tests/cases/fourslash/isInMultiLineCommentEOF.ts b/tests/cases/fourslash/isInMultiLineCommentEOF.ts deleted file mode 100644 index 2b65a958340..00000000000 --- a/tests/cases/fourslash/isInMultiLineCommentEOF.ts +++ /dev/null @@ -1,12 +0,0 @@ -/// - -// @Filename: f1.ts -//// /* /*0*/ blah /*1*/ */ - -// @Filename: f2.ts -//// /* /*2*/ blah /*3*/ - -for (let i = 0; i < 4; ++i) { - goTo.marker(`${i}`); - verify.isInMultiLineCommentAtPosition(); -} \ No newline at end of file From 777bc575ac8117994acd34c9716854c40c92c59e Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 4 Aug 2017 15:51:06 -0700 Subject: [PATCH 07/50] implementation comment --- src/services/formatting/formatting.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index c4c4bad5de8..fbda42d3840 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1168,6 +1168,14 @@ namespace ts.formatting { } export function getRangeOfEnclosingComment(sourceFile: SourceFile, position: number, onlyMultiLine: boolean): CommentRange | undefined { + // Considering a fixed position, + // - trailing comments are those following and on the same line as the position. + // - leading comments are those in the range [position, start of next non-trivia token) + // that are not trailing comments of that position. + // + // Note, `node.start` is the start-position of the first comment following the previous + // token that is not a trailing comment, so the leading and trailing comments of all + // tokens contain all comments in a sourcefile disjointly. const precedingToken = findPrecedingToken(position, sourceFile); const trailingRangesOfPreviousToken = precedingToken && getTrailingCommentRanges(sourceFile.text, precedingToken.end); const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), sourceFile); From 091376f46ff928feb79b6faee3dd1b88804fd336 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 7 Aug 2017 15:45:56 -0700 Subject: [PATCH 08/50] supressFormatOnKeyInComments --- src/services/formatting/smartIndenter.ts | 10 +++++----- src/services/services.ts | 25 +++++++++++++----------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index d0651ce140f..b0d58ddad12 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -31,6 +31,11 @@ namespace ts.formatting { return 0; } + const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position, options); + if (indentationOfEnclosingMultiLineComment >= 0) { + return indentationOfEnclosingMultiLineComment; + } + const precedingToken = findPrecedingToken(position, sourceFile); if (!precedingToken) { return getBaseIndentation(options); @@ -44,11 +49,6 @@ namespace ts.formatting { const lineAtPosition = sourceFile.getLineAndCharacterOfPosition(position).line; - const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position, options); - if (indentationOfEnclosingMultiLineComment >= 0) { - return indentationOfEnclosingMultiLineComment; - } - // indentation is first non-whitespace character in a previous line // for block indentation, we should look for a line which contains something that's not // whitespace. diff --git a/src/services/services.ts b/src/services/services.ts index d6310258885..5a70ccca501 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1757,17 +1757,20 @@ namespace ts { function getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[] { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); const settings = toEditorSettings(options); - if (key === "{") { - return formatting.formatOnOpeningCurly(position, sourceFile, getRuleProvider(settings), settings); - } - else if (key === "}") { - return formatting.formatOnClosingCurly(position, sourceFile, getRuleProvider(settings), settings); - } - else if (key === ";") { - return formatting.formatOnSemicolon(position, sourceFile, getRuleProvider(settings), settings); - } - else if (key === "\n") { - return formatting.formatOnEnter(position, sourceFile, getRuleProvider(settings), settings); + + if (!isInComment(sourceFile, position)) { + if (key === "{") { + return formatting.formatOnOpeningCurly(position, sourceFile, getRuleProvider(settings), settings); + } + else if (key === "}") { + return formatting.formatOnClosingCurly(position, sourceFile, getRuleProvider(settings), settings); + } + else if (key === ";") { + return formatting.formatOnSemicolon(position, sourceFile, getRuleProvider(settings), settings); + } + else if (key === "\n") { + return formatting.formatOnEnter(position, sourceFile, getRuleProvider(settings), settings); + } } return []; From 1663d01844ee79b9d87ad07008e5c6d7cb2cc28b Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Wed, 9 Aug 2017 14:38:26 -0700 Subject: [PATCH 09/50] Fix outlining of objects in array --- src/services/outliningElementsCollector.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 5099f8b66bd..0958892c1e6 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -16,6 +16,18 @@ namespace ts.OutliningElementsCollector { } } + function addOutliningForObjectLiteralsInArray(startElement: Node, endElement: Node, autoCollapse: boolean) { + if (startElement && endElement) { + const span: OutliningSpan = { + textSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + bannerText: collapseText, + autoCollapse + }; + elements.push(span); + } + } + function addOutliningSpanComments(commentSpan: CommentRange, autoCollapse: boolean) { if (commentSpan) { const span: OutliningSpan = { @@ -161,6 +173,10 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.CaseBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); + if (n.kind === SyntaxKind.ObjectLiteralExpression && n.parent.kind === SyntaxKind.ArrayLiteralExpression) { + addOutliningForObjectLiteralsInArray(openBrace, closeBrace, autoCollapse(n)); + break; + } addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n)); break; } From df5e1a0f6971a06916be4f6046267c0b72ddedde Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Wed, 9 Aug 2017 15:55:02 -0700 Subject: [PATCH 10/50] add fourslash test --- .../getOutliningForObjectsInArray.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 tests/cases/fourslash/getOutliningForObjectsInArray.ts diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts new file mode 100644 index 00000000000..87d2bc96e2e --- /dev/null +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -0,0 +1,23 @@ +/// + +////// objects in x should generate outlining spans that do not render in VS +//// const x =[| [ +//// [|{ a: 0 }|], +//// [|{ b: 1 }|], +//// [|{ c: 2 }|] +//// ]|]; +//// +////// objects in y should generate outlining spans that render as expected +//// const y =[| [ +//// [|{ +//// a: 0 +//// }|], +//// [|{ +//// b: 1 +//// }|], +//// [|{ +//// c: 2 +//// }|] +//// ]|]; + +verify.outliningSpansInCurrentFile(test.ranges()); \ No newline at end of file From de92e98770384e5e6023e10fce0609bbb0d029d8 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 10 Aug 2017 10:01:42 -0700 Subject: [PATCH 11/50] fix end-of-file assert failure --- src/services/services.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/services/services.ts b/src/services/services.ts index 648cc6c8320..24e85a2a019 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -107,12 +107,14 @@ namespace ts { scanner.setTextPos(pos); while (pos < end) { const token = scanner.scan(); - Debug.assert(token !== SyntaxKind.EndOfFileToken); // Else it would infinitely loop const textPos = scanner.getTextPos(); if (textPos <= end) { nodes.push(createNode(token, pos, textPos, this)); } pos = textPos; + if (token === SyntaxKind.EndOfFileToken) { + return pos; + } } return pos; } From c7d691dc15550b69cfec59bcb10c8e261e7dd7db Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Thu, 10 Aug 2017 13:27:24 -0700 Subject: [PATCH 12/50] Generalize to nested arrays and refactor --- src/services/outliningElementsCollector.ts | 40 +++++++------------ .../getOutliningForObjectsInArray.ts | 19 +++++++++ 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 0958892c1e6..edebf98a24b 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -4,25 +4,13 @@ namespace ts.OutliningElementsCollector { const elements: OutliningSpan[] = []; const collapseText = "..."; - function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean) { + function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, fullStart: boolean) { if (hintSpanNode && startElement && endElement) { const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(startElement.pos, endElement.end), - hintSpan: createTextSpanFromNode(hintSpanNode, sourceFile), - bannerText: collapseText, - autoCollapse, - }; - elements.push(span); - } - } - - function addOutliningForObjectLiteralsInArray(startElement: Node, endElement: Node, autoCollapse: boolean) { - if (startElement && endElement) { - const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + textSpan: createTextSpanFromBounds(fullStart ? startElement.pos : startElement.getStart(), endElement.end), hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), bannerText: collapseText, - autoCollapse + autoCollapse, }; elements.push(span); } @@ -125,7 +113,7 @@ namespace ts.OutliningElementsCollector { parent.kind === SyntaxKind.WithStatement || parent.kind === SyntaxKind.CatchClause) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } @@ -133,13 +121,13 @@ namespace ts.OutliningElementsCollector { // Could be the try-block, or the finally-block. const tryStatement = parent; if (tryStatement.tryBlock === n) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } else if (tryStatement.finallyBlock === n) { const finallyKeyword = findChildOfKind(tryStatement, SyntaxKind.FinallyKeyword, sourceFile); if (finallyKeyword) { - addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } } @@ -163,27 +151,27 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.ModuleBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: case SyntaxKind.EnumDeclaration: - case SyntaxKind.ObjectLiteralExpression: case SyntaxKind.CaseBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - if (n.kind === SyntaxKind.ObjectLiteralExpression && n.parent.kind === SyntaxKind.ArrayLiteralExpression) { - addOutliningForObjectLiteralsInArray(openBrace, closeBrace, autoCollapse(n)); - break; - } - addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n)); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } + case SyntaxKind.ObjectLiteralExpression: + const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); + const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); + break; case SyntaxKind.ArrayLiteralExpression: const openBracket = findChildOfKind(n, SyntaxKind.OpenBracketToken, sourceFile); const closeBracket = findChildOfKind(n, SyntaxKind.CloseBracketToken, sourceFile); - addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n)); + addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); break; } depth++; diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts index 87d2bc96e2e..207da1997b4 100644 --- a/tests/cases/fourslash/getOutliningForObjectsInArray.ts +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -19,5 +19,24 @@ //// c: 2 //// }|] //// ]|]; +//// +////// same behavior for nested arrays +//// const w =[| [ +//// [|[ a: 0 ]|], +//// [|[ b: 1 ]|], +//// [|[ c: 2 ]|] +//// ]|]; +//// +//// const z =[| [ +//// [|[ +//// a: 0 +//// ]|], +//// [|[ +//// b: 1 +//// ]|], +//// [|[ +//// c: 2 +//// ]|] +//// ]|]; verify.outliningSpansInCurrentFile(test.ranges()); \ No newline at end of file From d6ccee67661987373949af9c97bf3c636a22d496 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Fri, 11 Aug 2017 13:42:14 -0700 Subject: [PATCH 13/50] Cleans up and adds nested case to test --- .../getOutliningForObjectsInArray.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts index 207da1997b4..2d57d3335bd 100644 --- a/tests/cases/fourslash/getOutliningForObjectsInArray.ts +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -22,20 +22,34 @@ //// ////// same behavior for nested arrays //// const w =[| [ -//// [|[ a: 0 ]|], -//// [|[ b: 1 ]|], -//// [|[ c: 2 ]|] +//// [|[ 0 ]|], +//// [|[ 1 ]|], +//// [|[ 2 ]|] //// ]|]; //// //// const z =[| [ //// [|[ -//// a: 0 +//// 0 //// ]|], //// [|[ -//// b: 1 +//// 1 //// ]|], //// [|[ -//// c: 2 +//// 2 +//// ]|] +//// ]|]; +//// +////// multiple levels of nesting work as expected +//// const z =[| [ +//// [|[ +//// [|{ hello: 0 }|] +//// ]|], +//// [|[ +//// [|{ hello: 3 }|] +//// ]|], +//// [|[ +//// [|{ hello: 5 }|], +//// [|{ hello: 7 }|] //// ]|] //// ]|]; From 760812f7143ccc311746e0c3eac191d2905722c9 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Mon, 14 Aug 2017 09:27:45 -0700 Subject: [PATCH 14/50] Add explanatory comments, consolidate main body --- src/services/outliningElementsCollector.ts | 18 +++++++++++------- .../fourslash/getOutliningForObjectsInArray.ts | 8 ++++---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index edebf98a24b..cdeba21b32b 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -3,11 +3,17 @@ namespace ts.OutliningElementsCollector { export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; const collapseText = "..."; + let depth = 0; + const maxDepth = 20; - function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, fullStart: boolean) { + walk(sourceFile); + return elements; + + // If useFullStart is true, then the collapsing span includes leading whitespace, including linebreaks. + function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, useFullStart: boolean) { if (hintSpanNode && startElement && endElement) { const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(fullStart ? startElement.pos : startElement.getStart(), endElement.end), + textSpan: createTextSpanFromBounds(useFullStart ? startElement.pos : startElement.getStart(), endElement.end), hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), bannerText: collapseText, autoCollapse, @@ -82,8 +88,6 @@ namespace ts.OutliningElementsCollector { return isFunctionBlock(node) && node.parent.kind !== SyntaxKind.ArrowFunction; } - let depth = 0; - const maxDepth = 20; function walk(n: Node): void { cancellationToken.throwIfCancellationRequested(); if (depth > maxDepth) { @@ -163,6 +167,9 @@ namespace ts.OutliningElementsCollector { addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); break; } + // If the block has no leading keywords and is a member of an array literal, + // we again want to only collapse the span of the block. + // Otherwise, the collapsed section will include the end of the previous line. case SyntaxKind.ObjectLiteralExpression: const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); @@ -178,8 +185,5 @@ namespace ts.OutliningElementsCollector { forEachChild(n, walk); depth--; } - - walk(sourceFile); - return elements; } } \ No newline at end of file diff --git a/tests/cases/fourslash/getOutliningForObjectsInArray.ts b/tests/cases/fourslash/getOutliningForObjectsInArray.ts index 2d57d3335bd..89634224832 100644 --- a/tests/cases/fourslash/getOutliningForObjectsInArray.ts +++ b/tests/cases/fourslash/getOutliningForObjectsInArray.ts @@ -1,13 +1,13 @@ /// -////// objects in x should generate outlining spans that do not render in VS +// objects in x should generate outlining spans that do not render in VS //// const x =[| [ //// [|{ a: 0 }|], //// [|{ b: 1 }|], //// [|{ c: 2 }|] //// ]|]; //// -////// objects in y should generate outlining spans that render as expected +// objects in y should generate outlining spans that render as expected //// const y =[| [ //// [|{ //// a: 0 @@ -20,7 +20,7 @@ //// }|] //// ]|]; //// -////// same behavior for nested arrays +// same behavior for nested arrays //// const w =[| [ //// [|[ 0 ]|], //// [|[ 1 ]|], @@ -39,7 +39,7 @@ //// ]|] //// ]|]; //// -////// multiple levels of nesting work as expected +// multiple levels of nesting work as expected //// const z =[| [ //// [|[ //// [|{ hello: 0 }|] From b2188ad66c03e575c5bfd13314add12677e26ad3 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 14 Aug 2017 16:43:20 -0700 Subject: [PATCH 15/50] cleanup --- src/services/formatting/formatting.ts | 31 +++++++++++++++++++++------ 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index fbda42d3840..13a7f466f46 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1164,10 +1164,15 @@ namespace ts.formatting { const { column, character } = SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(commentLineStart, commentStart, sourceFile, options); return column + /*length after whitespace ends*/ range.pos - (commentLineStart + character); } - return undefined; + return -1; } - export function getRangeOfEnclosingComment(sourceFile: SourceFile, position: number, onlyMultiLine: boolean): CommentRange | undefined { + export function getRangeOfEnclosingComment( + sourceFile: SourceFile, + position: number, + onlyMultiLine: boolean, + tokenAtPosition = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), + predicate?: (c: CommentRange) => boolean): CommentRange | undefined { // Considering a fixed position, // - trailing comments are those following and on the same line as the position. // - leading comments are those in the range [position, start of next non-trivia token) @@ -1178,16 +1183,28 @@ namespace ts.formatting { // tokens contain all comments in a sourcefile disjointly. const precedingToken = findPrecedingToken(position, sourceFile); const trailingRangesOfPreviousToken = precedingToken && getTrailingCommentRanges(sourceFile.text, precedingToken.end); - const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), sourceFile); + const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(tokenAtPosition, sourceFile); const commentRanges = trailingRangesOfPreviousToken && leadingCommentRangesOfNextToken ? trailingRangesOfPreviousToken.concat(leadingCommentRangesOfNextToken) : trailingRangesOfPreviousToken || leadingCommentRangesOfNextToken; if (commentRanges) { for (const range of commentRanges) { - // We need to extend the range when in an unclosed multi-line comment. - if (range.pos < position && position < range.end || - position === range.end && (range.kind === SyntaxKind.SingleLineCommentTrivia || position === sourceFile.getFullWidth())) { - return onlyMultiLine && range.kind !== SyntaxKind.MultiLineCommentTrivia ? undefined : range; + // The end marker of a single-line comment does not include the newline character. + // With caret at `^`, in the following case, we are inside a comment (^ denotes the cursor position): + // + // // asdf ^\n + // + // But for closed multi-line comments, we don't want to be inside the comment in the following case: + // + // /* asdf */^ + // + // However, unterminated multi-line comments *do* contain their end. + // + // Internally, we represent the end of the comment at the newline and closing '/', respectively. + // + if ((range.pos < position && position < range.end || + position === range.end && (range.kind === SyntaxKind.SingleLineCommentTrivia || position === sourceFile.getFullWidth()))) { + return (range.kind === SyntaxKind.MultiLineCommentTrivia || !onlyMultiLine) && (!predicate || predicate(range)) ? range : undefined; } } } From 472ad9d3131c2738c4400e22f8696bad1522f342 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 14 Aug 2017 16:43:31 -0700 Subject: [PATCH 16/50] findPrevious changes --- src/services/utilities.ts | 64 +++++++++++---------------------------- 1 file changed, 18 insertions(+), 46 deletions(-) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index a5b035154be..c5b2a2b4f90 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -742,18 +742,18 @@ namespace ts { const children = n.getChildren(); for (let i = 0; i < children.length; i++) { const child = children[i]; - // condition 'position < child.end' checks if child node end after the position - // in the example below this condition will be false for 'aaaa' and 'bbbb' and true for 'ccc' - // aaaa___bbbb___$__ccc - // after we found child node with end after the position we check if start of the node is after the position. - // if yes - then position is in the trivia and we need to look into the previous child to find the token in question. - // if no - position is in the node itself so we should recurse in it. - // NOTE: JsxText is a weird kind of node that can contain only whitespaces (since they are not counted as trivia). - // if this is the case - then we should assume that token in question is located in previous child. - if (position < child.end && (nodeHasTokens(child) || child.kind === SyntaxKind.JsxText)) { + // Note that the span of a node's tokens is [node.getStart(...), node.end). + // Given that `position < child.end` and child has constiutent tokens*, we distinguish these cases: + // 1) `position` precedes `child`'s tokens or `child` has no tokens (ie: in a comment or whitespace preceding `child`): + // we need to find the last token in a previous child. + // 2) `position` is within the same span: we recurse on `child`. + // * JsxText is exceptional in that its tokens are (non-trivia) whitespace, which we do not want to return. + // TODO(arozga): shouldn't `findRightmost...` need to handle JsxText? + if (position < child.end) { const start = child.getStart(sourceFile, includeJsDoc); const lookInPreviousChild = (start >= position) || // cursor in the leading trivia + nodeHasTokens(child) || (child.kind === SyntaxKind.JsxText && start === child.end); // whitespace only JsxText if (lookInPreviousChild) { @@ -768,7 +768,7 @@ namespace ts { } } - Debug.assert(startNode !== undefined || n.kind === SyntaxKind.SourceFile || isJSDocCommentContainingNode(n)); + Debug.assert(startNode !== undefined || n.kind === SyntaxKind.SourceFile || isJSDocCommentContainingNode(n) || n.kind === SyntaxKind.JsxSelfClosingElement); // 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. @@ -780,7 +780,9 @@ namespace ts { } } - /// finds last node that is considered as candidate for search (isCandidate(node) === true) starting from 'exclusiveStartPosition' + /** + * Finds the rightmost child to the left of `children[exclusiveStartPosition]` which has constituent tokens. + */ function findRightmostChildNodeWithTokens(children: Node[], exclusiveStartPosition: number): Node { for (let i = exclusiveStartPosition - 1; i >= 0; i--) { if (nodeHasTokens(children[i])) { @@ -863,41 +865,11 @@ namespace ts { * @param predicate Additional predicate to test on the comment range. */ export function isInComment( - sourceFile: SourceFile, - position: number, - tokenAtPosition = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), - predicate?: (c: CommentRange) => boolean): boolean { - return position <= tokenAtPosition.getStart(sourceFile) && - (isInCommentRange(getLeadingCommentRanges(sourceFile.text, tokenAtPosition.pos)) || - isInCommentRange(getTrailingCommentRanges(sourceFile.text, tokenAtPosition.pos))); - - function isInCommentRange(commentRanges: CommentRange[]): boolean { - return forEach(commentRanges, c => isPositionInCommentRange(c, position, sourceFile.text) && (!predicate || predicate(c))); - } - } - - function isPositionInCommentRange({ pos, end, kind }: ts.CommentRange, position: number, text: string): boolean { - if (pos < position && position < end) { - return true; - } - else if (position === end) { - // The end marker of a single-line comment does not include the newline character. - // In the following case, we are inside a comment (^ denotes the cursor position): - // - // // asdf ^\n - // - // But for multi-line comments, we don't want to be inside the comment in the following case: - // - // /* asdf */^ - // - // Internally, we represent the end of the comment at the newline and closing '/', respectively. - return kind === SyntaxKind.SingleLineCommentTrivia || - // true for unterminated multi-line comment - !(text.charCodeAt(end - 1) === CharacterCodes.slash && text.charCodeAt(end - 2) === CharacterCodes.asterisk); - } - else { - return false; - } + sourceFile: SourceFile, + position: number, + tokenAtPosition = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), + predicate?: (c: CommentRange) => boolean): boolean { + return !!formatting.getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ false, tokenAtPosition, predicate); } export function hasDocComment(sourceFile: SourceFile, position: number) { From f3e0cbbd526b4975c342a936cfb328a19dccd6a2 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 15 Aug 2017 11:26:47 -0700 Subject: [PATCH 17/50] `findPrecedingToken` handles EOF child more gracefully --- src/services/services.ts | 2 +- src/services/utilities.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 24e85a2a019..c623fe15b02 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -113,7 +113,7 @@ namespace ts { } pos = textPos; if (token === SyntaxKind.EndOfFileToken) { - return pos; + break; } } return pos; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index c5b2a2b4f90..5336d47c1f3 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -753,7 +753,7 @@ namespace ts { const start = child.getStart(sourceFile, includeJsDoc); const lookInPreviousChild = (start >= position) || // cursor in the leading trivia - nodeHasTokens(child) || + !nodeHasTokens(child) || (child.kind === SyntaxKind.JsxText && start === child.end); // whitespace only JsxText if (lookInPreviousChild) { From a209db7bb62b8ab99675d0a7a2047d1a2915f436 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 15 Aug 2017 12:00:15 -0700 Subject: [PATCH 18/50] dont compute preceding token twice --- src/services/formatting/formatting.ts | 11 ++++++++--- src/services/formatting/smartIndenter.ts | 4 ++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 13a7f466f46..c34585c5bca 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1155,9 +1155,11 @@ namespace ts.formatting { /** * Gets the indentation level of the multi-line comment enclosing position, * and a negative value if the position is not in a multi-line comment. + * + * @param precedingToken Must be the result of `findPrecedingToken(position, sourceFile)`. */ - export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number, options: EditorSettings): number { - const range = getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ true); + export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number, precedingToken: Node | undefined, options: EditorSettings): number { + const range = getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ true, precedingToken || null); // tslint:disable-line:no-null-keyword if (range) { const commentStart = range.pos; const commentLineStart = getLineStartPositionForPosition(commentStart, sourceFile); @@ -1167,10 +1169,14 @@ namespace ts.formatting { return -1; } + /** + * @param precedingToken pass `null` if preceding token was already computed and result was `undefined`. + */ export function getRangeOfEnclosingComment( sourceFile: SourceFile, position: number, onlyMultiLine: boolean, + precedingToken: Node | null | undefined = findPrecedingToken(position, sourceFile), // tslint:disable-line:no-null-keyword tokenAtPosition = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), predicate?: (c: CommentRange) => boolean): CommentRange | undefined { // Considering a fixed position, @@ -1181,7 +1187,6 @@ namespace ts.formatting { // Note, `node.start` is the start-position of the first comment following the previous // token that is not a trailing comment, so the leading and trailing comments of all // tokens contain all comments in a sourcefile disjointly. - const precedingToken = findPrecedingToken(position, sourceFile); const trailingRangesOfPreviousToken = precedingToken && getTrailingCommentRanges(sourceFile.text, precedingToken.end); const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(tokenAtPosition, sourceFile); const commentRanges = trailingRangesOfPreviousToken && leadingCommentRangesOfNextToken ? diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index b0d58ddad12..c868ce04847 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -31,12 +31,12 @@ namespace ts.formatting { return 0; } - const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position, options); + const precedingToken = findPrecedingToken(position, sourceFile); + const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position, precedingToken, options); if (indentationOfEnclosingMultiLineComment >= 0) { return indentationOfEnclosingMultiLineComment; } - const precedingToken = findPrecedingToken(position, sourceFile); if (!precedingToken) { return getBaseIndentation(options); } From a08d18af579b0c492e98dd2d9c16358c1c3b2d67 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 15 Aug 2017 12:01:43 -0700 Subject: [PATCH 19/50] consolidate isInComment and getRangeOfEnclosingComment --- src/services/utilities.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 5336d47c1f3..160ef6db451 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -867,9 +867,9 @@ namespace ts { export function isInComment( sourceFile: SourceFile, position: number, - tokenAtPosition = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), + tokenAtPosition?: Node, predicate?: (c: CommentRange) => boolean): boolean { - return !!formatting.getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ false, tokenAtPosition, predicate); + return !!formatting.getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ false, /*precedingToken*/ undefined, tokenAtPosition, predicate); } export function hasDocComment(sourceFile: SourceFile, position: number) { From ad9c29b9281baed9d736adca0ffbf3d9634e2ccf Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 15 Aug 2017 12:39:40 -0700 Subject: [PATCH 20/50] add test --- tests/cases/fourslash/formatOnEnterInComment.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 tests/cases/fourslash/formatOnEnterInComment.ts diff --git a/tests/cases/fourslash/formatOnEnterInComment.ts b/tests/cases/fourslash/formatOnEnterInComment.ts new file mode 100644 index 00000000000..489eb6ac6ce --- /dev/null +++ b/tests/cases/fourslash/formatOnEnterInComment.ts @@ -0,0 +1,14 @@ +/// + +//// /** +//// * /*1*/ +//// */ + +goTo.marker("1"); +edit.insertLine(""); +verify.currentFileContentIs( +` /** + * + + */` +); \ No newline at end of file From af68a61ba7204d767bb09d450d90f080c755bbcb Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Wed, 16 Aug 2017 11:50:00 -0700 Subject: [PATCH 21/50] Ignore scripts for types packages --- src/server/typingsInstaller/nodeTypingsInstaller.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index de23b2649a6..f80b7a70df6 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -102,7 +102,7 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`Updating ${TypesRegistryPackageName} npm package...`); } - this.execSync(`${this.npmPath} install ${TypesRegistryPackageName}`, { cwd: globalTypingsCacheLocation, stdio: "ignore" }); + this.execSync(`${this.npmPath} install --ignore-scripts ${TypesRegistryPackageName}`, { cwd: globalTypingsCacheLocation, stdio: "ignore" }); if (this.log.isEnabled()) { this.log.writeLine(`Updated ${TypesRegistryPackageName} npm package`); } @@ -152,7 +152,7 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`#${requestId} with arguments'${JSON.stringify(args)}'.`); } - const command = `${this.npmPath} install ${args.join(" ")} --save-dev --user-agent="typesInstaller/${version}"`; + const command = `${this.npmPath} install --ignore-scripts ${args.join(" ")} --save-dev --user-agent="typesInstaller/${version}"`; const start = Date.now(); let stdout: Buffer; let stderr: Buffer; From 153b94aeb4931437802d02e193be88f5240a1978 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 15:28:47 -0700 Subject: [PATCH 22/50] `JsxText` has no leading comments --- src/compiler/utilities.ts | 2 +- .../fourslash/isInMultiLineCommentJsxText.ts | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/isInMultiLineCommentJsxText.ts diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 76d242e8b08..8c82b9d26df 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -635,7 +635,7 @@ namespace ts { } export function getLeadingCommentRangesOfNode(node: Node, sourceFileOfNode: SourceFile) { - return getLeadingCommentRanges(sourceFileOfNode.text, node.pos); + return node.kind !== SyntaxKind.JsxText ? getLeadingCommentRanges(sourceFileOfNode.text, node.pos) : undefined; } export function getLeadingCommentRangesOfNodeFromText(node: Node, text: string) { diff --git a/tests/cases/fourslash/isInMultiLineCommentJsxText.ts b/tests/cases/fourslash/isInMultiLineCommentJsxText.ts new file mode 100644 index 00000000000..db88f6b9a55 --- /dev/null +++ b/tests/cases/fourslash/isInMultiLineCommentJsxText.ts @@ -0,0 +1,27 @@ +/// + +// @Filename: file.jsx +////
+//// // /*0*/ +//// /* /*1*/ */ +//// /** +//// * /*2*/ +//// */ +//// foo() /* /*3*/ */ +//// // /*4*/ +//// /* /*5*/ */ +//// /** +//// * /*6*/ +//// */ +////
+////
+//// // /*7*/ +//// /* /*8*/ */ +//// /** +//// * /*9*/ +//// */ + +for (let i = 0; i < 10; ++i) { + goTo.marker(i.toString()); + verify.not.isInCommentAtPosition(/*onlyMultiLine*/ false); +} From 70e4f346bb12ce773173a1d0edad2e3ca6632748 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 17:35:14 -0700 Subject: [PATCH 23/50] update test --- .../fourslash/indentionsOfCommentBlockAfterFormatting.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/cases/fourslash/indentionsOfCommentBlockAfterFormatting.ts b/tests/cases/fourslash/indentionsOfCommentBlockAfterFormatting.ts index 3b686285b99..b965b314234 100644 --- a/tests/cases/fourslash/indentionsOfCommentBlockAfterFormatting.ts +++ b/tests/cases/fourslash/indentionsOfCommentBlockAfterFormatting.ts @@ -27,13 +27,13 @@ verify.indentationIs(4); goTo.marker("2"); verify.indentationIs(4); goTo.marker("3"); -verify.indentationIs(4); +verify.indentationIs(3); goTo.marker("4"); -verify.indentationIs(4); +verify.indentationIs(3); // Putting a marker in line "*" would bring some error when parsing code in automation. // So move right by 1 offset from marker 4 to locate the caret in this line. edit.moveRight(1); -verify.indentationIs(4); +verify.indentationIs(3); // Putting a marker in line " */" would bring some error when parsing code in automation. // So move left by 1 offset from marker 5 to locate the caret in this line. goTo.marker("5"); From 4b9f5a0f8fa7b1ef9bac8fc12b40479044ab9676 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 17:36:39 -0700 Subject: [PATCH 24/50] rename tests --- ...rFormatting.ts => indentationInBlockCommentAfterFormatting.ts} | 0 ...ultiLineCommentJsxText.ts => isInMultiLineCommentInJsxText.ts} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/cases/fourslash/{indentionsOfCommentBlockAfterFormatting.ts => indentationInBlockCommentAfterFormatting.ts} (100%) rename tests/cases/fourslash/{isInMultiLineCommentJsxText.ts => isInMultiLineCommentInJsxText.ts} (100%) diff --git a/tests/cases/fourslash/indentionsOfCommentBlockAfterFormatting.ts b/tests/cases/fourslash/indentationInBlockCommentAfterFormatting.ts similarity index 100% rename from tests/cases/fourslash/indentionsOfCommentBlockAfterFormatting.ts rename to tests/cases/fourslash/indentationInBlockCommentAfterFormatting.ts diff --git a/tests/cases/fourslash/isInMultiLineCommentJsxText.ts b/tests/cases/fourslash/isInMultiLineCommentInJsxText.ts similarity index 100% rename from tests/cases/fourslash/isInMultiLineCommentJsxText.ts rename to tests/cases/fourslash/isInMultiLineCommentInJsxText.ts From 62f16bee557f2cb604a4399c67dea623fc455afe Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 17:36:50 -0700 Subject: [PATCH 25/50] add tests --- .../cases/fourslash/indentationInComments.ts | 32 ++++++++++++++++ .../isInMultiLineCommentInTemplateLiteral.ts | 27 +++++++++++++ .../isInMultiLineCommentOnlyTrivia.ts | 38 +++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 tests/cases/fourslash/indentationInComments.ts create mode 100644 tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts create mode 100644 tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts diff --git a/tests/cases/fourslash/indentationInComments.ts b/tests/cases/fourslash/indentationInComments.ts new file mode 100644 index 00000000000..d54ec9daacb --- /dev/null +++ b/tests/cases/fourslash/indentationInComments.ts @@ -0,0 +1,32 @@ +/// + +//// // /*0_0*/ +//// /* /*0_1*/ +//// some text /*0_2*/ +//// some text /*1_0*/ +//// * some text /*0_3*/ +//// /*0_4*/ +//// */ +//// function foo() { +//// // /*4_0*/ +//// /** /*4_1*/ +//// * /*4_2*/ +//// * /*4_3*/ +//// /*7_0*/ +//// */ +//// /* /*4_4*/ */ +//// } + +for (let i = 0; i < 5; ++i) { + goTo.marker(`0_${i}`); + verify.indentationIs(0); + + goTo.marker(`4_${i}`); + verify.indentationIs(4); +} + +goTo.marker(`1_0`); +verify.indentationIs(1); + +goTo.marker(`7_0`); +verify.indentationIs(7); diff --git a/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts b/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts new file mode 100644 index 00000000000..ca6f11499da --- /dev/null +++ b/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts @@ -0,0 +1,27 @@ +/// + +// @Filename: file.jsx +//// ` +//// // /*0*/ +//// /* /*1*/ */ +//// /** +//// * /*2*/ +//// */ +//// foo() +//// // /*3*/ +//// /* /*4*/ */ +//// /** +//// * /*5*/ +//// */ +//// ` +//// ` +//// // /*6*/ +//// /* /*7*/ */ +//// /** +//// * /*8*/ +//// */ + +for (let i = 0; i < 9; ++i) { + goTo.marker(i.toString()); + verify.not.isInCommentAtPosition(/*onlyMultiLine*/ false); +} \ No newline at end of file diff --git a/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts b/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts new file mode 100644 index 00000000000..e279af47699 --- /dev/null +++ b/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts @@ -0,0 +1,38 @@ +/// + +//// /* x */ +//// /** +//// * @param this doesn't make sense here. +//// */ +//// // x + +const firstCommentStart = 0; +const firstCommentEnd = 7; +goTo.position(firstCommentStart); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); + +goTo.position(firstCommentStart + 1); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +goTo.position(firstCommentEnd - 1); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); + +goTo.position(firstCommentEnd); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); + +const multilineJsDocStart = firstCommentEnd + 1; +const multilineJsDocEnd = multilineJsDocStart + 49; + +goTo.position(multilineJsDocStart); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +goTo.position(multilineJsDocStart + 1); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +goTo.position(multilineJsDocEnd - 1); +verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +goTo.position(multilineJsDocEnd); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); + +const singleLineCommentStart = multilineJsDocEnd + 1; + +goTo.position(singleLineCommentStart + 1); +verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(/*onlyMultiLine*/ false); \ No newline at end of file From 23ca368020548d4a9b6d56e6719c5eb86cdcd68f Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 17:51:29 -0700 Subject: [PATCH 26/50] Use simpler indentation for comments * When in a multi-line comment, we would have liked to use the start of the comment as a reference point for the indentation inside the comment, but determining the number of columns shifted for the comment start woudl require determining the length w/r/t graphemes, which we do not currently implement. We would like to avoid taking on a runtime dependency on a grapheme-parsing library. Instead, we look at the indentation level on the previoud line or start of the comment as a reference point, and correct shift for lines starting with an asterisk. --- src/services/formatting/formatting.ts | 17 ---------------- src/services/formatting/smartIndenter.ts | 25 +++++++++++++++++++++--- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index c34585c5bca..22602d6031c 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1152,23 +1152,6 @@ namespace ts.formatting { } } - /** - * Gets the indentation level of the multi-line comment enclosing position, - * and a negative value if the position is not in a multi-line comment. - * - * @param precedingToken Must be the result of `findPrecedingToken(position, sourceFile)`. - */ - export function getIndentationOfEnclosingMultiLineComment(sourceFile: SourceFile, position: number, precedingToken: Node | undefined, options: EditorSettings): number { - const range = getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ true, precedingToken || null); // tslint:disable-line:no-null-keyword - if (range) { - const commentStart = range.pos; - const commentLineStart = getLineStartPositionForPosition(commentStart, sourceFile); - const { column, character } = SmartIndenter.findFirstNonWhitespaceCharacterAndColumn(commentLineStart, commentStart, sourceFile, options); - return column + /*length after whitespace ends*/ range.pos - (commentLineStart + character); - } - return -1; - } - /** * @param precedingToken pass `null` if preceding token was already computed and result was `undefined`. */ diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index c868ce04847..5af93bc025f 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -32,9 +32,28 @@ namespace ts.formatting { } const precedingToken = findPrecedingToken(position, sourceFile); - const indentationOfEnclosingMultiLineComment = getIndentationOfEnclosingMultiLineComment(sourceFile, position, precedingToken, options); - if (indentationOfEnclosingMultiLineComment >= 0) { - return indentationOfEnclosingMultiLineComment; + + const enclosingCommentRange = getRangeOfEnclosingComment(sourceFile, position, /*onlyMultiLine*/ true, precedingToken || null); // tslint:disable-line:no-null-keyword + if (enclosingCommentRange) { + const previousLine = getLineAndCharacterOfPosition(sourceFile, position).line - 1; + const commentStartLine = getLineAndCharacterOfPosition(sourceFile, enclosingCommentRange.pos).line; + + Debug.assert(commentStartLine >= 0); + + if (previousLine <= commentStartLine) { + return findFirstNonWhitespaceColumn(getStartPositionOfLine(commentStartLine, sourceFile), position, sourceFile, options); + } + + // get first character of previous line -- if it is '*', move back one more character (or stay at 0) + const startPostionOfLine = getStartPositionOfLine(previousLine, sourceFile); + const { column, character } = findFirstNonWhitespaceCharacterAndColumn(startPostionOfLine, position, sourceFile, options); + + if (column === 0) { + return column; + } + + const firstNonWhitespaceCharacterCode = sourceFile.text.charCodeAt(startPostionOfLine + character); + return firstNonWhitespaceCharacterCode === CharacterCodes.asterisk ? column - 1 : column; } if (!precedingToken) { From b7bc7d889ec432cea7c9a681ed284eb6162f3baf Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 17:56:20 -0700 Subject: [PATCH 27/50] clarify `JsxText` handling --- src/services/utilities.ts | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 160ef6db451..2ee46c28ca9 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -720,8 +720,14 @@ namespace ts { } } - export function findPrecedingToken(position: number, sourceFile: SourceFile, startNode?: Node, includeJsDoc?: boolean): Node { - return find(startNode || sourceFile); + /** + * Finds the rightmost token satisfying `token.end <= position`, + * excluding `JsxText` tokens containing only whitespace. + */ + export function findPrecedingToken(position: number, sourceFile: SourceFile, startNode?: Node, includeJsDoc?: boolean): Node | undefined { + const result = find(startNode || sourceFile); + Debug.assert(!(result && isWhiteSpaceOnlyJsxText(result))); + return result; function findRightmostToken(n: Node): Node { if (isToken(n)) { @@ -743,18 +749,16 @@ namespace ts { for (let i = 0; i < children.length; i++) { const child = children[i]; // Note that the span of a node's tokens is [node.getStart(...), node.end). - // Given that `position < child.end` and child has constiutent tokens*, we distinguish these cases: + // Given that `position < child.end` and child has constituent tokens, we distinguish these cases: // 1) `position` precedes `child`'s tokens or `child` has no tokens (ie: in a comment or whitespace preceding `child`): // we need to find the last token in a previous child. // 2) `position` is within the same span: we recurse on `child`. - // * JsxText is exceptional in that its tokens are (non-trivia) whitespace, which we do not want to return. - // TODO(arozga): shouldn't `findRightmost...` need to handle JsxText? if (position < child.end) { const start = child.getStart(sourceFile, includeJsDoc); const lookInPreviousChild = (start >= position) || // cursor in the leading trivia !nodeHasTokens(child) || - (child.kind === SyntaxKind.JsxText && start === child.end); // whitespace only JsxText + isWhiteSpaceOnlyJsxText(child); if (lookInPreviousChild) { // actual start of the node is past the position - previous token should be at the end of previous child @@ -781,11 +785,16 @@ namespace ts { } /** - * Finds the rightmost child to the left of `children[exclusiveStartPosition]` which has constituent tokens. + * Finds the rightmost child to the left of `children[exclusiveStartPosition]` which is a non-all-whitespace token or has constituent tokens. */ function findRightmostChildNodeWithTokens(children: Node[], exclusiveStartPosition: number): Node { for (let i = exclusiveStartPosition - 1; i >= 0; i--) { - if (nodeHasTokens(children[i])) { + const child = children[i]; + + if (isWhiteSpaceOnlyJsxText(child)) { + Debug.assert(i > 0, "`JsxText` tokens should not be the first child of `JsxElement | JsxSelfClosingElement`"); + } + else if (nodeHasTokens(children[i])) { return children[i]; } } @@ -853,6 +862,11 @@ namespace ts { return false; } + export function isWhiteSpaceOnlyJsxText(node: Node): node is JsxText { + return isJsxText(node) && node.containsOnlyWhiteSpaces; + } + + export function isInTemplateString(sourceFile: SourceFile, position: number) { const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); return isTemplateLiteralKind(token.kind) && position > token.getStart(sourceFile); @@ -888,7 +902,7 @@ namespace ts { function nodeHasTokens(n: Node): boolean { // If we have a token or node that has a non-zero width, it must have tokens. - // Note, that getWidth() does not take trivia into account. + // Note: getWidth() does not take trivia into account. return n.getWidth() !== 0; } From 6029b5cce80fe2172273c9a253e230c5813dde09 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 18:12:28 -0700 Subject: [PATCH 28/50] cleanup --- src/services/formatting/smartIndenter.ts | 1 - src/services/utilities.ts | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts index 5af93bc025f..4de2e5765e5 100644 --- a/src/services/formatting/smartIndenter.ts +++ b/src/services/formatting/smartIndenter.ts @@ -44,7 +44,6 @@ namespace ts.formatting { return findFirstNonWhitespaceColumn(getStartPositionOfLine(commentStartLine, sourceFile), position, sourceFile, options); } - // get first character of previous line -- if it is '*', move back one more character (or stay at 0) const startPostionOfLine = getStartPositionOfLine(previousLine, sourceFile); const { column, character } = findFirstNonWhitespaceCharacterAndColumn(startPostionOfLine, position, sourceFile, options); diff --git a/src/services/utilities.ts b/src/services/utilities.ts index ee099a50a73..e8f01bb0785 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -772,7 +772,7 @@ namespace ts { } } - Debug.assert(startNode !== undefined || n.kind === SyntaxKind.SourceFile || isJSDocCommentContainingNode(n) || n.kind === SyntaxKind.JsxSelfClosingElement); + Debug.assert(startNode !== undefined || n.kind === SyntaxKind.SourceFile || isJSDocCommentContainingNode(n)); // 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. @@ -866,7 +866,6 @@ namespace ts { return isJsxText(node) && node.containsOnlyWhiteSpaces; } - export function isInTemplateString(sourceFile: SourceFile, position: number) { const token = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); return isTemplateLiteralKind(token.kind) && position > token.getStart(sourceFile); From 760ef44c36e3dc6e4a88800fd094f925882ec94b Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Wed, 16 Aug 2017 18:48:27 -0700 Subject: [PATCH 29/50] test if `onlyMultiLine` flag changes answer --- src/harness/fourslash.ts | 15 +++++++----- tests/cases/fourslash/fourslash.ts | 2 +- tests/cases/fourslash/isInMultiLineComment.ts | 23 +++++++++---------- .../isInMultiLineCommentInJsxText.ts | 2 +- .../isInMultiLineCommentInTemplateLiteral.ts | 2 +- .../isInMultiLineCommentOnlyTrivia.ts | 19 ++++++++------- 6 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index f50ebcc3a14..32c9fb67da0 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2508,16 +2508,19 @@ namespace FourSlash { } } - public verifySpanOfEnclosingComment(negative: boolean, onlyMultiLine: boolean) { + public verifySpanOfEnclosingComment(negative: boolean, onlyMultiLineDiverges?: boolean) { const expected = !negative; const position = this.currentCaretPosition; const fileName = this.activeFile.fileName; - const actual = !!this.languageService.getSpanOfEnclosingComment(fileName, position, /*onlyMultiLine*/ onlyMultiLine); - if (expected !== actual) { + const actual = !!this.languageService.getSpanOfEnclosingComment(fileName, position, /*onlyMultiLine*/ false); + const actualOnlyMultiLine = !!this.languageService.getSpanOfEnclosingComment(fileName, position, /*onlyMultiLine*/ true); + if (expected !== actual || onlyMultiLineDiverges === (actual === actualOnlyMultiLine)) { this.raiseError(`verifySpanOfEnclosingComment failed: position: '${position}' fileName: '${fileName}' - onlyMultiLine: '${onlyMultiLine}' + onlyMultiLineDiverges: '${onlyMultiLineDiverges}' + actual: '${actual}' + actualOnlyMultiLine: '${actualOnlyMultiLine}' expected: '${expected}'.`); } } @@ -3662,8 +3665,8 @@ namespace FourSlashInterface { this.state.verifyBraceCompletionAtPosition(this.negative, openingBrace); } - public isInCommentAtPosition(onlyMultiLine: boolean) { - this.state.verifySpanOfEnclosingComment(this.negative, onlyMultiLine); + public isInCommentAtPosition(onlyMultiLineDiverges?: boolean) { + this.state.verifySpanOfEnclosingComment(this.negative, onlyMultiLineDiverges); } public codeFixAvailable() { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 088cbfaa490..87e11ae4262 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -153,7 +153,7 @@ declare namespace FourSlashInterface { typeDefinitionCountIs(expectedCount: number): void; implementationListIsEmpty(): void; isValidBraceCompletionAtPosition(openingBrace?: string): void; - isInCommentAtPosition(onlyMultiLine: boolean): void; + isInCommentAtPosition(onlyMultiLineDiverges?: boolean): void; codeFixAvailable(): void; applicableRefactorAvailableAtMarker(markerName: string): void; codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; diff --git a/tests/cases/fourslash/isInMultiLineComment.ts b/tests/cases/fourslash/isInMultiLineComment.ts index 3dc8a6e15d1..96da4f0021f 100644 --- a/tests/cases/fourslash/isInMultiLineComment.ts +++ b/tests/cases/fourslash/isInMultiLineComment.ts @@ -11,37 +11,36 @@ const firstCommentStart = 0; const firstCommentEnd = 7; goTo.position(firstCommentStart); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); goTo.position(firstCommentStart + 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(firstCommentEnd - 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(firstCommentEnd); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); const multilineJsDocStart = firstCommentEnd + 1; const multilineJsDocEnd = multilineJsDocStart + 49; goTo.position(multilineJsDocStart); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); goTo.position(multilineJsDocStart + 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(multilineJsDocEnd - 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(multilineJsDocEnd); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); const singleLineCommentStart = multilineJsDocEnd + 1; goTo.position(singleLineCommentStart + 1); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); -verify.isInCommentAtPosition(/*onlyMultiLine*/ false); +verify.isInCommentAtPosition(/*onlyMultiLineDiverges*/ true); const postNodeCommentStart = singleLineCommentStart + 16; goTo.position(postNodeCommentStart); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); goTo.position(postNodeCommentStart + 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); diff --git a/tests/cases/fourslash/isInMultiLineCommentInJsxText.ts b/tests/cases/fourslash/isInMultiLineCommentInJsxText.ts index db88f6b9a55..1664ca0cd1b 100644 --- a/tests/cases/fourslash/isInMultiLineCommentInJsxText.ts +++ b/tests/cases/fourslash/isInMultiLineCommentInJsxText.ts @@ -23,5 +23,5 @@ for (let i = 0; i < 10; ++i) { goTo.marker(i.toString()); - verify.not.isInCommentAtPosition(/*onlyMultiLine*/ false); + verify.not.isInCommentAtPosition(); } diff --git a/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts b/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts index ca6f11499da..c3e2ae92e8a 100644 --- a/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts +++ b/tests/cases/fourslash/isInMultiLineCommentInTemplateLiteral.ts @@ -23,5 +23,5 @@ for (let i = 0; i < 9; ++i) { goTo.marker(i.toString()); - verify.not.isInCommentAtPosition(/*onlyMultiLine*/ false); + verify.not.isInCommentAtPosition(); } \ No newline at end of file diff --git a/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts b/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts index e279af47699..41c9bbadfd8 100644 --- a/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts +++ b/tests/cases/fourslash/isInMultiLineCommentOnlyTrivia.ts @@ -9,30 +9,29 @@ const firstCommentStart = 0; const firstCommentEnd = 7; goTo.position(firstCommentStart); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); goTo.position(firstCommentStart + 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(firstCommentEnd - 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(firstCommentEnd); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); const multilineJsDocStart = firstCommentEnd + 1; const multilineJsDocEnd = multilineJsDocStart + 49; goTo.position(multilineJsDocStart); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); goTo.position(multilineJsDocStart + 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(multilineJsDocEnd - 1); -verify.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.isInCommentAtPosition(); goTo.position(multilineJsDocEnd); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); +verify.not.isInCommentAtPosition(); const singleLineCommentStart = multilineJsDocEnd + 1; goTo.position(singleLineCommentStart + 1); -verify.not.isInCommentAtPosition(/*onlyMultiLine*/ true); -verify.isInCommentAtPosition(/*onlyMultiLine*/ false); \ No newline at end of file +verify.isInCommentAtPosition(/*onlyMultiLineDiverges*/ true); \ No newline at end of file From b8e0dedac03c0049e7e8500a93f99b8f383c75d4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 17 Aug 2017 12:40:10 -0700 Subject: [PATCH 30/50] Fix #17069 and #15371 1. `T[K]` now correctly produces `number` when `K extends string, T extends Record`. 2. `T[K]` no longer allows any type to be assigned to it when `T extends object, K extends keyof T`. Previously both of these cases failed in getConstraintOfIndexedAccessType because both bases followed `K`'s base constraint to `string` and then incorrectly produced `any` for types (like `object`) with no string index signature. In (1), this produced an error in checkBinaryLikeExpression`. In (2), this failed to produce an error in `checkTypeRelatedTo`. --- src/compiler/checker.ts | 15 +++++++++++---- src/compiler/core.ts | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6778bacd05a..ff311177629 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5912,7 +5912,13 @@ namespace ts { return transformed; } const baseObjectType = getBaseConstraintOfType(type.objectType); - const baseIndexType = getBaseConstraintOfType(type.indexType); + const keepTypeParameterForMappedType = baseObjectType && getObjectFlags(baseObjectType) & ObjectFlags.Mapped && type.indexType.flags & TypeFlags.TypeParameter; + const baseIndexType = !keepTypeParameterForMappedType && getBaseConstraintOfType(type.indexType); + if (baseObjectType && baseIndexType === stringType && !getIndexInfoOfType(baseObjectType, IndexKind.String)) { + // getIndexedAccessType returns `any` for X[string] where X doesn't have an index signature. + // instead, return undefined so that the indexed access check fails + return undefined; + } return baseObjectType || baseIndexType ? getIndexedAccessType(baseObjectType || type.objectType, baseIndexType || type.indexType) : undefined; } @@ -5962,8 +5968,9 @@ namespace ts { function computeBaseConstraint(t: Type): Type { if (t.flags & TypeFlags.TypeParameter) { const constraint = getConstraintFromTypeParameter(t); - return (t).isThisType ? constraint : - constraint ? getBaseConstraint(constraint) : undefined; + return ((t as TypeParameter).isThisType || !constraint || getObjectFlags(constraint) & ObjectFlags.Mapped) ? + constraint : + getBaseConstraint(constraint); } if (t.flags & TypeFlags.UnionOrIntersection) { const types = (t).types; @@ -9290,7 +9297,7 @@ namespace ts { } else if (target.flags & TypeFlags.IndexedAccess) { // A type S is related to a type T[K] if S is related to A[K], where K is string-like and - // A is the apparent type of S. + // A is the apparent type of T. const constraint = getConstraintOfType(target); if (constraint) { if (result = isRelatedTo(source, constraint, reportErrors)) { diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 85514987b3b..6649bb21fe5 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -718,7 +718,7 @@ namespace ts { export function sum, K extends string>(array: T[], prop: K): number { let result = 0; for (const v of array) { - // Note: we need the following type assertion because of GH #17069 + // TODO: Remove the following type assertion once the fix for #17069 is merged result += v[prop] as number; } return result; From 1b4f90705fb042ecf7773c00a6f366a716fa7aa4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 17 Aug 2017 12:45:20 -0700 Subject: [PATCH 31/50] Test getConstraintOfIndexedAccess fixes and update baselines --- ...ionOperatorWithConstrainedTypeParameter.js | 27 ++++++++ ...eratorWithConstrainedTypeParameter.symbols | 54 +++++++++++++++ ...OperatorWithConstrainedTypeParameter.types | 64 ++++++++++++++++++ ...tiveConstraintOfIndexAccessType.errors.txt | 67 +++++++++++++++++++ ...nonPrimitiveConstraintOfIndexAccessType.js | 67 +++++++++++++++++++ ...ionOperatorWithConstrainedTypeParameter.ts | 11 +++ ...nonPrimitiveConstraintOfIndexAccessType.ts | 32 +++++++++ 7 files changed, 322 insertions(+) create mode 100644 tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js create mode 100644 tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols create mode 100644 tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types create mode 100644 tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt create mode 100644 tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js create mode 100644 tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts create mode 100644 tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts diff --git a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js new file mode 100644 index 00000000000..1366e242182 --- /dev/null +++ b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js @@ -0,0 +1,27 @@ +//// [additionOperatorWithConstrainedTypeParameter.ts] +// test for #17069 +function sum, K extends string>(n: number, v: T, k: K) { + n = n + v[k]; + n += v[k]; // += should work the same way +} +function realSum, K extends string>(n: number, vs: T[], k: K) { + for (const v of vs) { + n = n + v[k]; + n += v[k]; + } +} + + +//// [additionOperatorWithConstrainedTypeParameter.js] +// test for #17069 +function sum(n, v, k) { + n = n + v[k]; + n += v[k]; // += should work the same way +} +function realSum(n, vs, k) { + for (var _i = 0, vs_1 = vs; _i < vs_1.length; _i++) { + var v = vs_1[_i]; + n = n + v[k]; + n += v[k]; + } +} diff --git a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols new file mode 100644 index 00000000000..e7055c1e38f --- /dev/null +++ b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols @@ -0,0 +1,54 @@ +=== tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts === +// test for #17069 +function sum, K extends string>(n: number, v: T, k: K) { +>sum : Symbol(sum, Decl(additionOperatorWithConstrainedTypeParameter.ts, 0, 0)) +>T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 13)) +>Record : Symbol(Record, Decl(lib.d.ts, --, --)) +>K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 41)) +>K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 41)) +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) +>v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 70)) +>T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 13)) +>k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 76)) +>K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 41)) + + n = n + v[k]; +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) +>v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 70)) +>k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 76)) + + n += v[k]; // += should work the same way +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) +>v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 70)) +>k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 76)) +} +function realSum, K extends string>(n: number, vs: T[], k: K) { +>realSum : Symbol(realSum, Decl(additionOperatorWithConstrainedTypeParameter.ts, 4, 1)) +>T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 17)) +>Record : Symbol(Record, Decl(lib.d.ts, --, --)) +>K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 45)) +>K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 45)) +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) +>vs : Symbol(vs, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 74)) +>T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 17)) +>k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 83)) +>K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 45)) + + for (const v of vs) { +>v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 6, 14)) +>vs : Symbol(vs, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 74)) + + n = n + v[k]; +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) +>v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 6, 14)) +>k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 83)) + + n += v[k]; +>n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) +>v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 6, 14)) +>k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 83)) + } +} + diff --git a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types new file mode 100644 index 00000000000..d52c77a94fd --- /dev/null +++ b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types @@ -0,0 +1,64 @@ +=== tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts === +// test for #17069 +function sum, K extends string>(n: number, v: T, k: K) { +>sum : , K extends string>(n: number, v: T, k: K) => void +>T : T +>Record : Record +>K : K +>K : K +>n : number +>v : T +>T : T +>k : K +>K : K + + n = n + v[k]; +>n = n + v[k] : number +>n : number +>n + v[k] : number +>n : number +>v[k] : T[K] +>v : T +>k : K + + n += v[k]; // += should work the same way +>n += v[k] : number +>n : number +>v[k] : T[K] +>v : T +>k : K +} +function realSum, K extends string>(n: number, vs: T[], k: K) { +>realSum : , K extends string>(n: number, vs: T[], k: K) => void +>T : T +>Record : Record +>K : K +>K : K +>n : number +>vs : T[] +>T : T +>k : K +>K : K + + for (const v of vs) { +>v : T +>vs : T[] + + n = n + v[k]; +>n = n + v[k] : number +>n : number +>n + v[k] : number +>n : number +>v[k] : T[K] +>v : T +>k : K + + n += v[k]; +>n += v[k] : number +>n : number +>v[k] : T[K] +>v : T +>k : K + } +} + diff --git a/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt b/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt new file mode 100644 index 00000000000..56781157131 --- /dev/null +++ b/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt @@ -0,0 +1,67 @@ +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(3,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(6,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(9,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(12,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(15,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(18,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(21,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(24,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(27,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. +tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(30,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. + Type 'string' is not assignable to type 'number'. + + +==== tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts (10 errors) ==== + // test for #15371 + function f(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function g(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function h(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function i(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function j(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function k(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function o(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function l(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function m(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. + } + function n(s: string, tp: T[P]): void { + tp = s; + ~~ +!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. +!!! error TS2322: Type 'string' is not assignable to type 'number'. + } + \ No newline at end of file diff --git a/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js b/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js new file mode 100644 index 00000000000..7a24345a94b --- /dev/null +++ b/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js @@ -0,0 +1,67 @@ +//// [nonPrimitiveConstraintOfIndexAccessType.ts] +// test for #15371 +function f(s: string, tp: T[P]): void { + tp = s; +} +function g(s: string, tp: T[P]): void { + tp = s; +} +function h(s: string, tp: T[P]): void { + tp = s; +} +function i(s: string, tp: T[P]): void { + tp = s; +} +function j(s: string, tp: T[P]): void { + tp = s; +} +function k(s: string, tp: T[P]): void { + tp = s; +} +function o(s: string, tp: T[P]): void { + tp = s; +} +function l(s: string, tp: T[P]): void { + tp = s; +} +function m(s: string, tp: T[P]): void { + tp = s; +} +function n(s: string, tp: T[P]): void { + tp = s; +} + + +//// [nonPrimitiveConstraintOfIndexAccessType.js] +"use strict"; +// test for #15371 +function f(s, tp) { + tp = s; +} +function g(s, tp) { + tp = s; +} +function h(s, tp) { + tp = s; +} +function i(s, tp) { + tp = s; +} +function j(s, tp) { + tp = s; +} +function k(s, tp) { + tp = s; +} +function o(s, tp) { + tp = s; +} +function l(s, tp) { + tp = s; +} +function m(s, tp) { + tp = s; +} +function n(s, tp) { + tp = s; +} diff --git a/tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts b/tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts new file mode 100644 index 00000000000..2303bb33944 --- /dev/null +++ b/tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts @@ -0,0 +1,11 @@ +// test for #17069 +function sum, K extends string>(n: number, v: T, k: K) { + n = n + v[k]; + n += v[k]; // += should work the same way +} +function realSum, K extends string>(n: number, vs: T[], k: K) { + for (const v of vs) { + n = n + v[k]; + n += v[k]; + } +} diff --git a/tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts b/tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts new file mode 100644 index 00000000000..1e852f12f92 --- /dev/null +++ b/tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts @@ -0,0 +1,32 @@ +// @strict: true +// test for #15371 +function f(s: string, tp: T[P]): void { + tp = s; +} +function g(s: string, tp: T[P]): void { + tp = s; +} +function h(s: string, tp: T[P]): void { + tp = s; +} +function i(s: string, tp: T[P]): void { + tp = s; +} +function j(s: string, tp: T[P]): void { + tp = s; +} +function k(s: string, tp: T[P]): void { + tp = s; +} +function o(s: string, tp: T[P]): void { + tp = s; +} +function l(s: string, tp: T[P]): void { + tp = s; +} +function m(s: string, tp: T[P]): void { + tp = s; +} +function n(s: string, tp: T[P]): void { + tp = s; +} From a187b17e97fece8012bc8529ea3b1a9ebe4804e9 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Thu, 17 Aug 2017 13:09:21 -0700 Subject: [PATCH 32/50] Simplify mapped-type handling in computeBaseConstraint --- src/compiler/checker.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ff311177629..b3e7b01123d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5968,7 +5968,7 @@ namespace ts { function computeBaseConstraint(t: Type): Type { if (t.flags & TypeFlags.TypeParameter) { const constraint = getConstraintFromTypeParameter(t); - return ((t as TypeParameter).isThisType || !constraint || getObjectFlags(constraint) & ObjectFlags.Mapped) ? + return (t as TypeParameter).isThisType || !constraint ? constraint : getBaseConstraint(constraint); } @@ -5998,9 +5998,6 @@ namespace ts { const baseIndexedAccess = baseObjectType && baseIndexType ? getIndexedAccessType(baseObjectType, baseIndexType) : undefined; return baseIndexedAccess && baseIndexedAccess !== unknownType ? getBaseConstraint(baseIndexedAccess) : undefined; } - if (isGenericMappedType(t)) { - return emptyObjectType; - } return t; } } From e7d2af0d72cbd0981174f76b3b66259e9a591ae5 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 17 Aug 2017 20:06:34 -0700 Subject: [PATCH 33/50] remove duplicate verify --- .../cases/fourslash/completionListAndMemberListOnCommentedDot.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cases/fourslash/completionListAndMemberListOnCommentedDot.ts b/tests/cases/fourslash/completionListAndMemberListOnCommentedDot.ts index d0517d87279..b57e7b84b53 100644 --- a/tests/cases/fourslash/completionListAndMemberListOnCommentedDot.ts +++ b/tests/cases/fourslash/completionListAndMemberListOnCommentedDot.ts @@ -14,5 +14,4 @@ //////c./**/ goTo.marker(); -verify.completionListIsEmpty(); verify.completionListIsEmpty(); \ No newline at end of file From e4e969a210325190097c963cc3e4023682125c4a Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 17 Aug 2017 20:06:46 -0700 Subject: [PATCH 34/50] respond to comments --- src/services/formatting/formatting.ts | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 22602d6031c..85cab126559 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -1159,17 +1159,20 @@ namespace ts.formatting { sourceFile: SourceFile, position: number, onlyMultiLine: boolean, - precedingToken: Node | null | undefined = findPrecedingToken(position, sourceFile), // tslint:disable-line:no-null-keyword + precedingToken?: Node | null, // tslint:disable-line:no-null-keyword tokenAtPosition = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false), predicate?: (c: CommentRange) => boolean): CommentRange | undefined { - // Considering a fixed position, - // - trailing comments are those following and on the same line as the position. - // - leading comments are those in the range [position, start of next non-trivia token) - // that are not trailing comments of that position. - // - // Note, `node.start` is the start-position of the first comment following the previous - // token that is not a trailing comment, so the leading and trailing comments of all - // tokens contain all comments in a sourcefile disjointly. + const tokenStart = tokenAtPosition.getStart(sourceFile); + if (tokenStart <= position && position < tokenAtPosition.getEnd()) { + return undefined; + } + + if (precedingToken === undefined) { + precedingToken = findPrecedingToken(position, sourceFile); + } + + // Between two consecutive tokens, all comments are either trailing on the former + // or leading on the latter (and none are in both lists). const trailingRangesOfPreviousToken = precedingToken && getTrailingCommentRanges(sourceFile.text, precedingToken.end); const leadingCommentRangesOfNextToken = getLeadingCommentRangesOfNode(tokenAtPosition, sourceFile); const commentRanges = trailingRangesOfPreviousToken && leadingCommentRangesOfNextToken ? From fa6773e685e2c1f19d6e35cb12f06edfa9c31ad5 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 18 Aug 2017 11:00:46 +0200 Subject: [PATCH 35/50] Type parameters from class should not be in scope in base class expression --- src/compiler/checker.ts | 12 +++++++++++- src/compiler/diagnosticMessages.json | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1c41be84ba7..d5518eee3da 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1016,7 +1016,17 @@ namespace ts { } } break; - + case SyntaxKind.ExpressionWithTypeArguments: + if (lastLocation === (location).expression && (location.parent).token === SyntaxKind.ExtendsKeyword) { + const container = location.parent.parent; + if (isClassLike(container) && (result = lookup(getSymbolOfNode(container).members, name, meaning & SymbolFlags.Type))) { + if (nameNotFoundMessage) { + error(errorLocation, Diagnostics.Base_class_expressions_cannot_reference_class_type_parameters); + } + return undefined; + } + } + break; // It is not legal to reference a class's own type parameters from a computed property name that // belongs to the class. For example: // diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 77e7f7e7b62..049483fdb8f 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1912,6 +1912,10 @@ "category": "Error", "code": 2560 }, + "Base class expressions cannot reference class type parameters.": { + "category": "Error", + "code": 2561 + }, "JSX element attributes type '{0}' may not be a union type.": { "category": "Error", "code": 2600 From ade3b565ae9a5d483fcb85400ca183a90041c66f Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 18 Aug 2017 11:20:07 -0700 Subject: [PATCH 36/50] Revert public API changes to logger (#17899) --- src/harness/harnessLanguageService.ts | 5 +-- .../unittests/tsserverProjectSystem.ts | 5 +-- src/server/editorServices.ts | 32 +++++++++---------- src/server/server.ts | 28 ++++++++-------- src/server/session.ts | 4 +-- src/server/utilities.ts | 17 +++++++--- 6 files changed, 52 insertions(+), 39 deletions(-) diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 5e8d6e76716..58df0d9a5e2 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -686,7 +686,7 @@ namespace Harness.LanguageService { this.host.log(message); } - err(message: string): void { + msg(message: string): void { this.host.log(message); } @@ -702,7 +702,8 @@ namespace Harness.LanguageService { return false; } - group() { throw ts.notImplemented(); } + startGroup() { throw ts.notImplemented(); } + endGroup() { throw ts.notImplemented(); } perftrc(message: string): void { return this.host.log(message); diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 596e080430c..6e548231c91 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -39,8 +39,9 @@ namespace ts.projectSystem { loggingEnabled: () => false, perftrc: noop, info: noop, - err: noop, - group: noop, + msg: noop, + startGroup: noop, + endGroup: noop, getLogFileName: (): string => undefined }; diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index fe7946fc0f7..cb497ff774f 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -958,28 +958,28 @@ namespace ts.server { return; } - this.logger.group(info => { - let counter = 0; - counter = printProjects(this.externalProjects, info, counter); - counter = printProjects(this.configuredProjects, info, counter); - printProjects(this.inferredProjects, info, counter); - - info("Open files: "); - for (const rootFile of this.openFiles) { - info(`\t${rootFile.fileName}`); - } - }); - - function printProjects(projects: Project[], info: (msg: string) => void, counter: number): number { + this.logger.startGroup(); + let counter = 0; + const printProjects = (projects: Project[], counter: number): number => { for (const project of projects) { project.updateGraph(); - info(`Project '${project.getProjectName()}' (${ProjectKind[project.projectKind]}) ${counter}`); - info(project.filesToString()); - info("-----------------------------------------------"); + this.logger.info(`Project '${project.getProjectName()}' (${ProjectKind[project.projectKind]}) ${counter}`); + this.logger.info(project.filesToString()); + this.logger.info("-----------------------------------------------"); counter++; } return counter; + }; + counter = printProjects(this.externalProjects, counter); + counter = printProjects(this.configuredProjects, counter); + printProjects(this.inferredProjects, counter); + + this.logger.info("Open files: "); + for (const rootFile of this.openFiles) { + this.logger.info(`\t${rootFile.fileName}`); } + + this.logger.endGroup(); } private findConfiguredProjectByProjectName(configFileName: NormalizedPath) { diff --git a/src/server/server.ts b/src/server/server.ts index 66467fd46ae..0fe37dd8ba0 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -140,6 +140,8 @@ namespace ts.server { class Logger implements server.Logger { private fd = -1; private seq = 0; + private inGroup = false; + private firstInGroup = true; constructor(private readonly logFilename: string, private readonly traceToConsole: boolean, @@ -169,24 +171,24 @@ namespace ts.server { } perftrc(s: string) { - this.msg(s, "Perf"); + this.msg(s, Msg.Perf); } info(s: string) { - this.msg(s, "Info"); + this.msg(s, Msg.Info); } err(s: string) { - this.msg(s, "Err"); + this.msg(s, Msg.Err); } - group(logGroupEntries: (log: (msg: string) => void) => void) { - let firstInGroup = false; - logGroupEntries(s => { - this.msg(s, "Info", /*inGroup*/ true, firstInGroup); - firstInGroup = false; - }); - this.seq++; + startGroup() { + this.inGroup = true; + this.firstInGroup = true; + } + + endGroup() { + this.inGroup = false; } loggingEnabled() { @@ -197,16 +199,16 @@ namespace ts.server { return this.loggingEnabled() && this.level >= level; } - private msg(s: string, type: string, inGroup = false, firstInGroup = false) { + msg(s: string, type: Msg.Types = Msg.Err) { if (!this.canWrite) return; s = `[${nowString()}] ${s}\n`; - if (!inGroup || firstInGroup) { + if (!this.inGroup || this.firstInGroup) { const prefix = Logger.padStringRight(type + " " + this.seq.toString(), " "); s = prefix + s; } this.write(s); - if (!inGroup) { + if (!this.inGroup) { this.seq++; } } diff --git a/src/server/session.ts b/src/server/session.ts index 4122408cfa6..6eb0413f6d8 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -368,7 +368,7 @@ namespace ts.server { msg += "\n" + (err).stack; } } - this.logger.err(msg); + this.logger.msg(msg, Msg.Err); } public send(msg: protocol.Message) { @@ -1947,7 +1947,7 @@ namespace ts.server { return this.executeWithRequestId(request.seq, () => handler(request)); } else { - this.logger.err(`Unrecognized JSON command: ${JSON.stringify(request)}`); + this.logger.msg(`Unrecognized JSON command: ${JSON.stringify(request)}`, Msg.Err); this.output(undefined, CommandNames.Unknown, request.seq, `Unrecognized JSON command: ${request.command}`); return { responseRequired: false }; } diff --git a/src/server/utilities.ts b/src/server/utilities.ts index 0d4bc101ff6..e2e2a67eaf1 100644 --- a/src/server/utilities.ts +++ b/src/server/utilities.ts @@ -17,11 +17,22 @@ namespace ts.server { loggingEnabled(): boolean; perftrc(s: string): void; info(s: string): void; - err(s: string): void; - group(logGroupEntries: (log: (msg: string) => void) => void): void; + startGroup(): void; + endGroup(): void; + msg(s: string, type?: Msg.Types): void; getLogFileName(): string; } + export namespace Msg { + export type Err = "Err"; + export const Err: Err = "Err"; + export type Info = "Info"; + export const Info: Info = "Info"; + export type Perf = "Perf"; + export const Perf: Perf = "Perf"; + export type Types = Err | Info | Perf; + } + function getProjectRootPath(project: Project): Path { switch (project.projectKind) { case ProjectKind.Configured: @@ -115,9 +126,7 @@ namespace ts.server { } export function createNormalizedPathMap(): NormalizedPathMap { -/* tslint:disable:no-null-keyword */ const map = createMap(); -/* tslint:enable:no-null-keyword */ return { get(path) { return map.get(path); From 6b68da1185d80a04a910240493dd7a8b5852bc93 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 18 Aug 2017 11:32:53 -0700 Subject: [PATCH 37/50] Revert "Fix getConstraintOfIndexedAccess" --- src/compiler/checker.ts | 18 ++--- src/compiler/core.ts | 2 +- ...ionOperatorWithConstrainedTypeParameter.js | 27 -------- ...eratorWithConstrainedTypeParameter.symbols | 54 --------------- ...OperatorWithConstrainedTypeParameter.types | 64 ------------------ ...tiveConstraintOfIndexAccessType.errors.txt | 67 ------------------- ...nonPrimitiveConstraintOfIndexAccessType.js | 67 ------------------- ...ionOperatorWithConstrainedTypeParameter.ts | 11 --- ...nonPrimitiveConstraintOfIndexAccessType.ts | 32 --------- 9 files changed, 8 insertions(+), 334 deletions(-) delete mode 100644 tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js delete mode 100644 tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols delete mode 100644 tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types delete mode 100644 tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt delete mode 100644 tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js delete mode 100644 tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts delete mode 100644 tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 41921e3c789..6df6b1f9ce1 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5911,13 +5911,7 @@ namespace ts { return transformed; } const baseObjectType = getBaseConstraintOfType(type.objectType); - const keepTypeParameterForMappedType = baseObjectType && getObjectFlags(baseObjectType) & ObjectFlags.Mapped && type.indexType.flags & TypeFlags.TypeParameter; - const baseIndexType = !keepTypeParameterForMappedType && getBaseConstraintOfType(type.indexType); - if (baseObjectType && baseIndexType === stringType && !getIndexInfoOfType(baseObjectType, IndexKind.String)) { - // getIndexedAccessType returns `any` for X[string] where X doesn't have an index signature. - // instead, return undefined so that the indexed access check fails - return undefined; - } + const baseIndexType = getBaseConstraintOfType(type.indexType); return baseObjectType || baseIndexType ? getIndexedAccessType(baseObjectType || type.objectType, baseIndexType || type.indexType) : undefined; } @@ -5967,9 +5961,8 @@ namespace ts { function computeBaseConstraint(t: Type): Type { if (t.flags & TypeFlags.TypeParameter) { const constraint = getConstraintFromTypeParameter(t); - return (t as TypeParameter).isThisType || !constraint ? - constraint : - getBaseConstraint(constraint); + return (t).isThisType ? constraint : + constraint ? getBaseConstraint(constraint) : undefined; } if (t.flags & TypeFlags.UnionOrIntersection) { const types = (t).types; @@ -5997,6 +5990,9 @@ namespace ts { const baseIndexedAccess = baseObjectType && baseIndexType ? getIndexedAccessType(baseObjectType, baseIndexType) : undefined; return baseIndexedAccess && baseIndexedAccess !== unknownType ? getBaseConstraint(baseIndexedAccess) : undefined; } + if (isGenericMappedType(t)) { + return emptyObjectType; + } return t; } } @@ -9293,7 +9289,7 @@ namespace ts { } else if (target.flags & TypeFlags.IndexedAccess) { // A type S is related to a type T[K] if S is related to A[K], where K is string-like and - // A is the apparent type of T. + // A is the apparent type of S. const constraint = getConstraintOfType(target); if (constraint) { if (result = isRelatedTo(source, constraint, reportErrors)) { diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 422df2ead05..8cab1b41155 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -718,7 +718,7 @@ namespace ts { export function sum, K extends string>(array: T[], prop: K): number { let result = 0; for (const v of array) { - // TODO: Remove the following type assertion once the fix for #17069 is merged + // Note: we need the following type assertion because of GH #17069 result += v[prop] as number; } return result; diff --git a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js deleted file mode 100644 index 1366e242182..00000000000 --- a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.js +++ /dev/null @@ -1,27 +0,0 @@ -//// [additionOperatorWithConstrainedTypeParameter.ts] -// test for #17069 -function sum, K extends string>(n: number, v: T, k: K) { - n = n + v[k]; - n += v[k]; // += should work the same way -} -function realSum, K extends string>(n: number, vs: T[], k: K) { - for (const v of vs) { - n = n + v[k]; - n += v[k]; - } -} - - -//// [additionOperatorWithConstrainedTypeParameter.js] -// test for #17069 -function sum(n, v, k) { - n = n + v[k]; - n += v[k]; // += should work the same way -} -function realSum(n, vs, k) { - for (var _i = 0, vs_1 = vs; _i < vs_1.length; _i++) { - var v = vs_1[_i]; - n = n + v[k]; - n += v[k]; - } -} diff --git a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols deleted file mode 100644 index e7055c1e38f..00000000000 --- a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.symbols +++ /dev/null @@ -1,54 +0,0 @@ -=== tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts === -// test for #17069 -function sum, K extends string>(n: number, v: T, k: K) { ->sum : Symbol(sum, Decl(additionOperatorWithConstrainedTypeParameter.ts, 0, 0)) ->T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 13)) ->Record : Symbol(Record, Decl(lib.d.ts, --, --)) ->K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 41)) ->K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 41)) ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) ->v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 70)) ->T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 13)) ->k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 76)) ->K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 41)) - - n = n + v[k]; ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) ->v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 70)) ->k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 76)) - - n += v[k]; // += should work the same way ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 60)) ->v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 70)) ->k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 1, 76)) -} -function realSum, K extends string>(n: number, vs: T[], k: K) { ->realSum : Symbol(realSum, Decl(additionOperatorWithConstrainedTypeParameter.ts, 4, 1)) ->T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 17)) ->Record : Symbol(Record, Decl(lib.d.ts, --, --)) ->K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 45)) ->K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 45)) ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) ->vs : Symbol(vs, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 74)) ->T : Symbol(T, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 17)) ->k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 83)) ->K : Symbol(K, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 45)) - - for (const v of vs) { ->v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 6, 14)) ->vs : Symbol(vs, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 74)) - - n = n + v[k]; ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) ->v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 6, 14)) ->k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 83)) - - n += v[k]; ->n : Symbol(n, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 64)) ->v : Symbol(v, Decl(additionOperatorWithConstrainedTypeParameter.ts, 6, 14)) ->k : Symbol(k, Decl(additionOperatorWithConstrainedTypeParameter.ts, 5, 83)) - } -} - diff --git a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types b/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types deleted file mode 100644 index d52c77a94fd..00000000000 --- a/tests/baselines/reference/additionOperatorWithConstrainedTypeParameter.types +++ /dev/null @@ -1,64 +0,0 @@ -=== tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts === -// test for #17069 -function sum, K extends string>(n: number, v: T, k: K) { ->sum : , K extends string>(n: number, v: T, k: K) => void ->T : T ->Record : Record ->K : K ->K : K ->n : number ->v : T ->T : T ->k : K ->K : K - - n = n + v[k]; ->n = n + v[k] : number ->n : number ->n + v[k] : number ->n : number ->v[k] : T[K] ->v : T ->k : K - - n += v[k]; // += should work the same way ->n += v[k] : number ->n : number ->v[k] : T[K] ->v : T ->k : K -} -function realSum, K extends string>(n: number, vs: T[], k: K) { ->realSum : , K extends string>(n: number, vs: T[], k: K) => void ->T : T ->Record : Record ->K : K ->K : K ->n : number ->vs : T[] ->T : T ->k : K ->K : K - - for (const v of vs) { ->v : T ->vs : T[] - - n = n + v[k]; ->n = n + v[k] : number ->n : number ->n + v[k] : number ->n : number ->v[k] : T[K] ->v : T ->k : K - - n += v[k]; ->n += v[k] : number ->n : number ->v[k] : T[K] ->v : T ->k : K - } -} - diff --git a/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt b/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt deleted file mode 100644 index 56781157131..00000000000 --- a/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.errors.txt +++ /dev/null @@ -1,67 +0,0 @@ -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(3,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(6,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(9,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(12,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(15,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(18,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(21,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(24,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(27,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. -tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts(30,5): error TS2322: Type 'string' is not assignable to type 'T[P]'. - Type 'string' is not assignable to type 'number'. - - -==== tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts (10 errors) ==== - // test for #15371 - function f(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function g(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function h(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function i(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function j(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function k(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function o(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function l(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function m(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. - } - function n(s: string, tp: T[P]): void { - tp = s; - ~~ -!!! error TS2322: Type 'string' is not assignable to type 'T[P]'. -!!! error TS2322: Type 'string' is not assignable to type 'number'. - } - \ No newline at end of file diff --git a/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js b/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js deleted file mode 100644 index 7a24345a94b..00000000000 --- a/tests/baselines/reference/nonPrimitiveConstraintOfIndexAccessType.js +++ /dev/null @@ -1,67 +0,0 @@ -//// [nonPrimitiveConstraintOfIndexAccessType.ts] -// test for #15371 -function f(s: string, tp: T[P]): void { - tp = s; -} -function g(s: string, tp: T[P]): void { - tp = s; -} -function h(s: string, tp: T[P]): void { - tp = s; -} -function i(s: string, tp: T[P]): void { - tp = s; -} -function j(s: string, tp: T[P]): void { - tp = s; -} -function k(s: string, tp: T[P]): void { - tp = s; -} -function o(s: string, tp: T[P]): void { - tp = s; -} -function l(s: string, tp: T[P]): void { - tp = s; -} -function m(s: string, tp: T[P]): void { - tp = s; -} -function n(s: string, tp: T[P]): void { - tp = s; -} - - -//// [nonPrimitiveConstraintOfIndexAccessType.js] -"use strict"; -// test for #15371 -function f(s, tp) { - tp = s; -} -function g(s, tp) { - tp = s; -} -function h(s, tp) { - tp = s; -} -function i(s, tp) { - tp = s; -} -function j(s, tp) { - tp = s; -} -function k(s, tp) { - tp = s; -} -function o(s, tp) { - tp = s; -} -function l(s, tp) { - tp = s; -} -function m(s, tp) { - tp = s; -} -function n(s, tp) { - tp = s; -} diff --git a/tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts b/tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts deleted file mode 100644 index 2303bb33944..00000000000 --- a/tests/cases/conformance/expressions/binaryOperators/additionOperator/additionOperatorWithConstrainedTypeParameter.ts +++ /dev/null @@ -1,11 +0,0 @@ -// test for #17069 -function sum, K extends string>(n: number, v: T, k: K) { - n = n + v[k]; - n += v[k]; // += should work the same way -} -function realSum, K extends string>(n: number, vs: T[], k: K) { - for (const v of vs) { - n = n + v[k]; - n += v[k]; - } -} diff --git a/tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts b/tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts deleted file mode 100644 index 1e852f12f92..00000000000 --- a/tests/cases/conformance/types/nonPrimitive/nonPrimitiveConstraintOfIndexAccessType.ts +++ /dev/null @@ -1,32 +0,0 @@ -// @strict: true -// test for #15371 -function f(s: string, tp: T[P]): void { - tp = s; -} -function g(s: string, tp: T[P]): void { - tp = s; -} -function h(s: string, tp: T[P]): void { - tp = s; -} -function i(s: string, tp: T[P]): void { - tp = s; -} -function j(s: string, tp: T[P]): void { - tp = s; -} -function k(s: string, tp: T[P]): void { - tp = s; -} -function o(s: string, tp: T[P]): void { - tp = s; -} -function l(s: string, tp: T[P]): void { - tp = s; -} -function m(s: string, tp: T[P]): void { - tp = s; -} -function n(s: string, tp: T[P]): void { - tp = s; -} From ecd2ae8dac34ab8cc9f41d054824b1fc9cfa73ff Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 18 Aug 2017 11:44:36 -0700 Subject: [PATCH 38/50] Deduplicate inputfiles before running RWC tests (#17877) * Deduplicate inputfiles before running RWC tests We deduplicate in the compiler, but we don't in the harness - this causes tests where the same file is listed multiple times in the arguments to not have the expected errors written, because we write out the same file multiple times when we should not. * Don't completely omit both copied of duplicates * Remove trailing whitespace * Maintain list order while filtering duplicates * Merge adjacent loops --- src/harness/rwcRunner.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/harness/rwcRunner.ts b/src/harness/rwcRunner.ts index a1dc50349e4..cddb9b19d4a 100644 --- a/src/harness/rwcRunner.ts +++ b/src/harness/rwcRunner.ts @@ -90,9 +90,16 @@ namespace RWC { ts.setConfigFileInOptions(opts.options, configParseResult.options.configFile); } - // Load the files + // Deduplicate files so they are only printed once in baselines (they are deduplicated within the compiler already) + const uniqueNames = ts.createMap(); for (const fileName of fileNames) { - inputFiles.push(getHarnessCompilerInputUnit(fileName)); + // Must maintain order, build result list while checking map + const normalized = ts.normalizeSlashes(fileName); + if (!uniqueNames.has(normalized)) { + uniqueNames.set(normalized, true); + // Load the file + inputFiles.push(getHarnessCompilerInputUnit(fileName)); + } } // Add files to compilation From 8e5e6c626b24253a55e048c698c9283b25dce5b2 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Fri, 18 Aug 2017 13:00:29 -0700 Subject: [PATCH 39/50] Update .npmignore (#17905) --- .npmignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.npmignore b/.npmignore index d3c27afeff4..50ae145423a 100644 --- a/.npmignore +++ b/.npmignore @@ -16,4 +16,5 @@ Jakefile.js .gitattributes .settings/ .travis.yml -.vscode/ \ No newline at end of file +.vscode/ +test.config \ No newline at end of file From a14aaf47721bd1728897c1b527a1326a3ca7fbb0 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Fri, 18 Aug 2017 15:58:21 -0700 Subject: [PATCH 40/50] Add release-2.5 to covered branches --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 4a99aaf2237..27b14739d8b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,6 +16,7 @@ matrix: branches: only: - master + - release-2.5 install: - npm uninstall typescript --no-save From e6c1afb4a0b8f04f6202a7dd1f39d7cdffeaf096 Mon Sep 17 00:00:00 2001 From: uniqueiniquity Date: Fri, 18 Aug 2017 15:59:22 -0700 Subject: [PATCH 41/50] Style changes and cleanup --- src/services/outliningElementsCollector.ts | 29 +++++++++++----------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index cdeba21b32b..e0d99d1d271 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -1,20 +1,21 @@ /* @internal */ namespace ts.OutliningElementsCollector { + const collapseText = "..."; + const maxDepth = 20; + export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; - const collapseText = "..."; let depth = 0; - const maxDepth = 20; walk(sourceFile); return elements; - // If useFullStart is true, then the collapsing span includes leading whitespace, including linebreaks. + /** If useFullStart is true, then the collapsing span includes leading whitespace, including linebreaks. */ function addOutliningSpan(hintSpanNode: Node, startElement: Node, endElement: Node, autoCollapse: boolean, useFullStart: boolean) { if (hintSpanNode && startElement && endElement) { const span: OutliningSpan = { - textSpan: createTextSpanFromBounds(useFullStart ? startElement.pos : startElement.getStart(), endElement.end), - hintSpan: createTextSpanFromBounds(startElement.getStart(), endElement.end), + textSpan: createTextSpanFromBounds(useFullStart ? startElement.getFullStart() : startElement.getStart(), endElement.getEnd()), + hintSpan: createTextSpanFromNode(hintSpanNode, sourceFile), bannerText: collapseText, autoCollapse, }; @@ -117,7 +118,7 @@ namespace ts.OutliningElementsCollector { parent.kind === SyntaxKind.WithStatement || parent.kind === SyntaxKind.CatchClause) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } @@ -125,13 +126,13 @@ namespace ts.OutliningElementsCollector { // Could be the try-block, or the finally-block. const tryStatement = parent; if (tryStatement.tryBlock === n) { - addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(parent, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } else if (tryStatement.finallyBlock === n) { const finallyKeyword = findChildOfKind(tryStatement, SyntaxKind.FinallyKeyword, sourceFile); if (finallyKeyword) { - addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(finallyKeyword, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } } @@ -155,7 +156,7 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.ModuleBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(n.parent, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } case SyntaxKind.ClassDeclaration: @@ -164,21 +165,21 @@ namespace ts.OutliningElementsCollector { case SyntaxKind.CaseBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ true); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ true); break; } - // If the block has no leading keywords and is a member of an array literal, - // we again want to only collapse the span of the block. + // If the block has no leading keywords and is inside an array literal, + // we only want to collapse the span of the block. // Otherwise, the collapsed section will include the end of the previous line. case SyntaxKind.ObjectLiteralExpression: const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); const closeBrace = findChildOfKind(n, SyntaxKind.CloseBraceToken, sourceFile); - addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); + addOutliningSpan(n, openBrace, closeBrace, autoCollapse(n), /*useFullStart*/ !isArrayLiteralExpression(n.parent)); break; case SyntaxKind.ArrayLiteralExpression: const openBracket = findChildOfKind(n, SyntaxKind.OpenBracketToken, sourceFile); const closeBracket = findChildOfKind(n, SyntaxKind.CloseBracketToken, sourceFile); - addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n), /* fullStart */ n.parent.kind !== SyntaxKind.ArrayLiteralExpression); + addOutliningSpan(n, openBracket, closeBracket, autoCollapse(n), /*useFullStart*/ !isArrayLiteralExpression(n.parent)); break; } depth++; From a136f554a708f8caf2fe4b9060207e7d9869a7d2 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Fri, 18 Aug 2017 17:20:57 -0700 Subject: [PATCH 42/50] Fix stack overflow when resolving default construct signatures (#17878) * Fix stack overflow when resolving default construct signatures * No need for || emptyArray --- src/compiler/checker.ts | 16 ++++--- .../reference/cloduleGenericOnSelfMember.js | 42 +++++++++++++++++++ .../cloduleGenericOnSelfMember.symbols | 30 +++++++++++++ .../cloduleGenericOnSelfMember.types | 33 +++++++++++++++ .../compiler/cloduleGenericOnSelfMember.ts | 11 +++++ 5 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 tests/baselines/reference/cloduleGenericOnSelfMember.js create mode 100644 tests/baselines/reference/cloduleGenericOnSelfMember.symbols create mode 100644 tests/baselines/reference/cloduleGenericOnSelfMember.types create mode 100644 tests/cases/compiler/cloduleGenericOnSelfMember.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6df6b1f9ce1..6febaf0052d 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5657,17 +5657,12 @@ namespace ts { else { // Combinations of function, class, enum and module let members = emptySymbols; - let constructSignatures: Signature[] = emptyArray; let stringIndexInfo: IndexInfo = undefined; if (symbol.exports) { members = getExportsOfSymbol(symbol); } if (symbol.flags & SymbolFlags.Class) { const classType = getDeclaredTypeOfClassOrInterface(symbol); - constructSignatures = getSignaturesOfSymbol(symbol.members.get(InternalSymbolName.Constructor)); - if (!constructSignatures.length) { - constructSignatures = getDefaultConstructSignatures(classType); - } const baseConstructorType = getBaseConstructorTypeOfClass(classType); if (baseConstructorType.flags & (TypeFlags.Object | TypeFlags.Intersection | TypeFlags.TypeVariable)) { members = createSymbolTable(getNamedMembers(members)); @@ -5678,7 +5673,7 @@ namespace ts { } } const numberIndexInfo = symbol.flags & SymbolFlags.Enum ? enumNumberIndexInfo : undefined; - setStructuredTypeMembers(type, members, emptyArray, constructSignatures, stringIndexInfo, numberIndexInfo); + setStructuredTypeMembers(type, members, emptyArray, emptyArray, stringIndexInfo, numberIndexInfo); // We resolve the members before computing the signatures because a signature may use // typeof with a qualified name expression that circularly references the type we are // in the process of resolving (see issue #6072). The temporarily empty signature list @@ -5686,6 +5681,15 @@ namespace ts { if (symbol.flags & (SymbolFlags.Function | SymbolFlags.Method)) { (type).callSignatures = getSignaturesOfSymbol(symbol); } + // And likewise for construct signatures for classes + if (symbol.flags & SymbolFlags.Class) { + const classType = getDeclaredTypeOfClassOrInterface(symbol); + let constructSignatures = getSignaturesOfSymbol(symbol.members.get(InternalSymbolName.Constructor)); + if (!constructSignatures.length) { + constructSignatures = getDefaultConstructSignatures(classType); + } + (type).constructSignatures = constructSignatures; + } } } diff --git a/tests/baselines/reference/cloduleGenericOnSelfMember.js b/tests/baselines/reference/cloduleGenericOnSelfMember.js new file mode 100644 index 00000000000..972651f1045 --- /dev/null +++ b/tests/baselines/reference/cloduleGenericOnSelfMember.js @@ -0,0 +1,42 @@ +//// [cloduleGenericOnSelfMember.ts] +class ServiceBase { + field: T; +} +class Service extends ServiceBase { +} +namespace Service { + export const Base = { + name: "1", + value: 5 + }; +} + +//// [cloduleGenericOnSelfMember.js] +var __extends = (this && this.__extends) || (function () { + var extendStatics = Object.setPrototypeOf || + ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) || + function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; }; + return function (d, b) { + extendStatics(d, b); + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); + }; +})(); +var ServiceBase = /** @class */ (function () { + function ServiceBase() { + } + return ServiceBase; +}()); +var Service = /** @class */ (function (_super) { + __extends(Service, _super); + function Service() { + return _super !== null && _super.apply(this, arguments) || this; + } + return Service; +}(ServiceBase)); +(function (Service) { + Service.Base = { + name: "1", + value: 5 + }; +})(Service || (Service = {})); diff --git a/tests/baselines/reference/cloduleGenericOnSelfMember.symbols b/tests/baselines/reference/cloduleGenericOnSelfMember.symbols new file mode 100644 index 00000000000..c7e0126fa7a --- /dev/null +++ b/tests/baselines/reference/cloduleGenericOnSelfMember.symbols @@ -0,0 +1,30 @@ +=== tests/cases/compiler/cloduleGenericOnSelfMember.ts === +class ServiceBase { +>ServiceBase : Symbol(ServiceBase, Decl(cloduleGenericOnSelfMember.ts, 0, 0)) +>T : Symbol(T, Decl(cloduleGenericOnSelfMember.ts, 0, 18)) + + field: T; +>field : Symbol(ServiceBase.field, Decl(cloduleGenericOnSelfMember.ts, 0, 22)) +>T : Symbol(T, Decl(cloduleGenericOnSelfMember.ts, 0, 18)) +} +class Service extends ServiceBase { +>Service : Symbol(Service, Decl(cloduleGenericOnSelfMember.ts, 2, 1), Decl(cloduleGenericOnSelfMember.ts, 4, 1)) +>ServiceBase : Symbol(ServiceBase, Decl(cloduleGenericOnSelfMember.ts, 0, 0)) +>Service.Base : Symbol(Service.Base, Decl(cloduleGenericOnSelfMember.ts, 6, 16)) +>Service : Symbol(Service, Decl(cloduleGenericOnSelfMember.ts, 2, 1), Decl(cloduleGenericOnSelfMember.ts, 4, 1)) +>Base : Symbol(Service.Base, Decl(cloduleGenericOnSelfMember.ts, 6, 16)) +} +namespace Service { +>Service : Symbol(Service, Decl(cloduleGenericOnSelfMember.ts, 2, 1), Decl(cloduleGenericOnSelfMember.ts, 4, 1)) + + export const Base = { +>Base : Symbol(Base, Decl(cloduleGenericOnSelfMember.ts, 6, 16)) + + name: "1", +>name : Symbol(name, Decl(cloduleGenericOnSelfMember.ts, 6, 25)) + + value: 5 +>value : Symbol(value, Decl(cloduleGenericOnSelfMember.ts, 7, 18)) + + }; +} diff --git a/tests/baselines/reference/cloduleGenericOnSelfMember.types b/tests/baselines/reference/cloduleGenericOnSelfMember.types new file mode 100644 index 00000000000..87590745c6f --- /dev/null +++ b/tests/baselines/reference/cloduleGenericOnSelfMember.types @@ -0,0 +1,33 @@ +=== tests/cases/compiler/cloduleGenericOnSelfMember.ts === +class ServiceBase { +>ServiceBase : ServiceBase +>T : T + + field: T; +>field : T +>T : T +} +class Service extends ServiceBase { +>Service : Service +>ServiceBase : ServiceBase<{ name: string; value: number; }> +>Service.Base : { name: string; value: number; } +>Service : typeof Service +>Base : { name: string; value: number; } +} +namespace Service { +>Service : typeof Service + + export const Base = { +>Base : { name: string; value: number; } +>{ name: "1", value: 5 } : { name: string; value: number; } + + name: "1", +>name : string +>"1" : "1" + + value: 5 +>value : number +>5 : 5 + + }; +} diff --git a/tests/cases/compiler/cloduleGenericOnSelfMember.ts b/tests/cases/compiler/cloduleGenericOnSelfMember.ts new file mode 100644 index 00000000000..23f0e6af1b6 --- /dev/null +++ b/tests/cases/compiler/cloduleGenericOnSelfMember.ts @@ -0,0 +1,11 @@ +class ServiceBase { + field: T; +} +class Service extends ServiceBase { +} +namespace Service { + export const Base = { + name: "1", + value: 5 + }; +} \ No newline at end of file From 5e8e735db55e82ff0cf4bddaf4f0fe6d8a4d39f7 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 18 Aug 2017 17:21:25 -0700 Subject: [PATCH 43/50] quickInfo: Don't check for `type === undefined`, check for `any` (#17815) * quickInfo: Don't check for `type === undefined`, check for `any` * Fixes: * We still want to do some work even if type is `any` * Second test for `if (type)` is necessary because it might not have been assigned. --- src/services/symbolDisplay.ts | 215 ++++++++++---------- tests/cases/fourslash/quickInfoTypeError.ts | 10 + 2 files changed, 118 insertions(+), 107 deletions(-) create mode 100644 tests/cases/fourslash/quickInfoTypeError.ts diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts index 303bb8395ee..05c451cc3f3 100644 --- a/src/services/symbolDisplay.ts +++ b/src/services/symbolDisplay.ts @@ -111,124 +111,123 @@ namespace ts.SymbolDisplay { let signature: Signature; type = isThisExpression ? typeChecker.getTypeAtLocation(location) : typeChecker.getTypeOfSymbolAtLocation(symbol.exportSymbol || symbol, location); - if (type) { - if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) { - const right = (location.parent).name; - // Either the location is on the right of a property access, or on the left and the right is missing - if (right === location || (right && right.getFullWidth() === 0)) { - location = location.parent; + + if (location.parent && location.parent.kind === SyntaxKind.PropertyAccessExpression) { + const right = (location.parent).name; + // Either the location is on the right of a property access, or on the left and the right is missing + if (right === location || (right && right.getFullWidth() === 0)) { + location = location.parent; + } + } + + // try get the call/construct signature from the type if it matches + let callExpressionLike: CallExpression | NewExpression | JsxOpeningLikeElement; + if (isCallOrNewExpression(location)) { + callExpressionLike = location; + } + else if (isCallExpressionTarget(location) || isNewExpressionTarget(location)) { + callExpressionLike = location.parent; + } + else if (location.parent && isJsxOpeningLikeElement(location.parent) && isFunctionLike(symbol.valueDeclaration)) { + callExpressionLike = location.parent; + } + + if (callExpressionLike) { + const candidateSignatures: Signature[] = []; + signature = typeChecker.getResolvedSignature(callExpressionLike, candidateSignatures); + if (!signature && candidateSignatures.length) { + // Use the first candidate: + signature = candidateSignatures[0]; + } + + const useConstructSignatures = callExpressionLike.kind === SyntaxKind.NewExpression || (isCallExpression(callExpressionLike) && callExpressionLike.expression.kind === SyntaxKind.SuperKeyword); + + const allSignatures = useConstructSignatures ? type.getConstructSignatures() : type.getCallSignatures(); + + if (!contains(allSignatures, signature.target) && !contains(allSignatures, signature)) { + // Get the first signature if there is one -- allSignatures may contain + // either the original signature or its target, so check for either + signature = allSignatures.length ? allSignatures[0] : undefined; + } + + if (signature) { + if (useConstructSignatures && (symbolFlags & SymbolFlags.Class)) { + // Constructor + symbolKind = ScriptElementKind.constructorImplementationElement; + addPrefixForAnyFunctionOrVar(type.symbol, symbolKind); } - } - - // try get the call/construct signature from the type if it matches - let callExpressionLike: CallExpression | NewExpression | JsxOpeningLikeElement; - if (isCallOrNewExpression(location)) { - callExpressionLike = location; - } - else if (isCallExpressionTarget(location) || isNewExpressionTarget(location)) { - callExpressionLike = location.parent; - } - else if (location.parent && isJsxOpeningLikeElement(location.parent) && isFunctionLike(symbol.valueDeclaration)) { - callExpressionLike = location.parent; - } - - if (callExpressionLike) { - const candidateSignatures: Signature[] = []; - signature = typeChecker.getResolvedSignature(callExpressionLike, candidateSignatures); - if (!signature && candidateSignatures.length) { - // Use the first candidate: - signature = candidateSignatures[0]; - } - - const useConstructSignatures = callExpressionLike.kind === SyntaxKind.NewExpression || (isCallExpression(callExpressionLike) && callExpressionLike.expression.kind === SyntaxKind.SuperKeyword); - - const allSignatures = useConstructSignatures ? type.getConstructSignatures() : type.getCallSignatures(); - - if (!contains(allSignatures, signature.target) && !contains(allSignatures, signature)) { - // Get the first signature if there is one -- allSignatures may contain - // either the original signature or its target, so check for either - signature = allSignatures.length ? allSignatures[0] : undefined; - } - - if (signature) { - if (useConstructSignatures && (symbolFlags & SymbolFlags.Class)) { - // Constructor - symbolKind = ScriptElementKind.constructorImplementationElement; - addPrefixForAnyFunctionOrVar(type.symbol, symbolKind); + else if (symbolFlags & SymbolFlags.Alias) { + symbolKind = ScriptElementKind.alias; + pushTypePart(symbolKind); + displayParts.push(spacePart()); + if (useConstructSignatures) { + displayParts.push(keywordPart(SyntaxKind.NewKeyword)); + displayParts.push(spacePart()); } - else if (symbolFlags & SymbolFlags.Alias) { - symbolKind = ScriptElementKind.alias; - pushTypePart(symbolKind); + addFullSymbolName(symbol); + } + else { + addPrefixForAnyFunctionOrVar(symbol, symbolKind); + } + + switch (symbolKind) { + case ScriptElementKind.jsxAttribute: + case ScriptElementKind.memberVariableElement: + case ScriptElementKind.variableElement: + case ScriptElementKind.constElement: + case ScriptElementKind.letElement: + case ScriptElementKind.parameterElement: + case ScriptElementKind.localVariableElement: + // If it is call or construct signature of lambda's write type name + displayParts.push(punctuationPart(SyntaxKind.ColonToken)); displayParts.push(spacePart()); if (useConstructSignatures) { displayParts.push(keywordPart(SyntaxKind.NewKeyword)); displayParts.push(spacePart()); } - addFullSymbolName(symbol); - } - else { - addPrefixForAnyFunctionOrVar(symbol, symbolKind); - } + if (!(type.flags & TypeFlags.Object && (type).objectFlags & ObjectFlags.Anonymous) && type.symbol) { + addRange(displayParts, symbolToDisplayParts(typeChecker, type.symbol, enclosingDeclaration, /*meaning*/ undefined, SymbolFormatFlags.WriteTypeParametersOrArguments)); + } + addSignatureDisplayParts(signature, allSignatures, TypeFormatFlags.WriteArrowStyleSignature); + break; - switch (symbolKind) { - case ScriptElementKind.jsxAttribute: - case ScriptElementKind.memberVariableElement: - case ScriptElementKind.variableElement: - case ScriptElementKind.constElement: - case ScriptElementKind.letElement: - case ScriptElementKind.parameterElement: - case ScriptElementKind.localVariableElement: - // If it is call or construct signature of lambda's write type name - displayParts.push(punctuationPart(SyntaxKind.ColonToken)); - displayParts.push(spacePart()); - if (useConstructSignatures) { - displayParts.push(keywordPart(SyntaxKind.NewKeyword)); - displayParts.push(spacePart()); - } - if (!(type.flags & TypeFlags.Object && (type).objectFlags & ObjectFlags.Anonymous) && type.symbol) { - addRange(displayParts, symbolToDisplayParts(typeChecker, type.symbol, enclosingDeclaration, /*meaning*/ undefined, SymbolFormatFlags.WriteTypeParametersOrArguments)); - } - addSignatureDisplayParts(signature, allSignatures, TypeFormatFlags.WriteArrowStyleSignature); - break; - - default: - // Just signature - addSignatureDisplayParts(signature, allSignatures); - } - hasAddedSymbolInfo = true; + default: + // Just signature + addSignatureDisplayParts(signature, allSignatures); } + hasAddedSymbolInfo = true; } - else if ((isNameOfFunctionDeclaration(location) && !(symbolFlags & SymbolFlags.Accessor)) || // name of function declaration - (location.kind === SyntaxKind.ConstructorKeyword && location.parent.kind === SyntaxKind.Constructor)) { // At constructor keyword of constructor declaration - // get the signature from the declaration and write it - const functionDeclaration = location.parent; - // Use function declaration to write the signatures only if the symbol corresponding to this declaration - const locationIsSymbolDeclaration = find(symbol.declarations, declaration => - declaration === (location.kind === SyntaxKind.ConstructorKeyword ? functionDeclaration.parent : functionDeclaration)); + } + else if ((isNameOfFunctionDeclaration(location) && !(symbolFlags & SymbolFlags.Accessor)) || // name of function declaration + (location.kind === SyntaxKind.ConstructorKeyword && location.parent.kind === SyntaxKind.Constructor)) { // At constructor keyword of constructor declaration + // get the signature from the declaration and write it + const functionDeclaration = location.parent; + // Use function declaration to write the signatures only if the symbol corresponding to this declaration + const locationIsSymbolDeclaration = find(symbol.declarations, declaration => + declaration === (location.kind === SyntaxKind.ConstructorKeyword ? functionDeclaration.parent : functionDeclaration)); - if (locationIsSymbolDeclaration) { - const allSignatures = functionDeclaration.kind === SyntaxKind.Constructor ? type.getNonNullableType().getConstructSignatures() : type.getNonNullableType().getCallSignatures(); - if (!typeChecker.isImplementationOfOverload(functionDeclaration)) { - signature = typeChecker.getSignatureFromDeclaration(functionDeclaration); - } - else { - signature = allSignatures[0]; - } - - if (functionDeclaration.kind === SyntaxKind.Constructor) { - // show (constructor) Type(...) signature - symbolKind = ScriptElementKind.constructorImplementationElement; - addPrefixForAnyFunctionOrVar(type.symbol, symbolKind); - } - else { - // (function/method) symbol(..signature) - addPrefixForAnyFunctionOrVar(functionDeclaration.kind === SyntaxKind.CallSignature && - !(type.symbol.flags & SymbolFlags.TypeLiteral || type.symbol.flags & SymbolFlags.ObjectLiteral) ? type.symbol : symbol, symbolKind); - } - - addSignatureDisplayParts(signature, allSignatures); - hasAddedSymbolInfo = true; + if (locationIsSymbolDeclaration) { + const allSignatures = functionDeclaration.kind === SyntaxKind.Constructor ? type.getNonNullableType().getConstructSignatures() : type.getNonNullableType().getCallSignatures(); + if (!typeChecker.isImplementationOfOverload(functionDeclaration)) { + signature = typeChecker.getSignatureFromDeclaration(functionDeclaration); } + else { + signature = allSignatures[0]; + } + + if (functionDeclaration.kind === SyntaxKind.Constructor) { + // show (constructor) Type(...) signature + symbolKind = ScriptElementKind.constructorImplementationElement; + addPrefixForAnyFunctionOrVar(type.symbol, symbolKind); + } + else { + // (function/method) symbol(..signature) + addPrefixForAnyFunctionOrVar(functionDeclaration.kind === SyntaxKind.CallSignature && + !(type.symbol.flags & SymbolFlags.TypeLiteral || type.symbol.flags & SymbolFlags.ObjectLiteral) ? type.symbol : symbol, symbolKind); + } + + addSignatureDisplayParts(signature, allSignatures); + hasAddedSymbolInfo = true; } } } @@ -417,7 +416,9 @@ namespace ts.SymbolDisplay { symbolFlags & SymbolFlags.Accessor || symbolKind === ScriptElementKind.memberFunctionElement) { const allSignatures = type.getNonNullableType().getCallSignatures(); - addSignatureDisplayParts(allSignatures[0], allSignatures); + if (allSignatures.length) { + addSignatureDisplayParts(allSignatures[0], allSignatures); + } } } } diff --git a/tests/cases/fourslash/quickInfoTypeError.ts b/tests/cases/fourslash/quickInfoTypeError.ts new file mode 100644 index 00000000000..7e0c9b20303 --- /dev/null +++ b/tests/cases/fourslash/quickInfoTypeError.ts @@ -0,0 +1,10 @@ +/// + +////foo({ +//// /**/f: function() {}, +//// f() {} +////}); + +// The symbol indicates that this is a funciton, but the type is `any`. +// Regression test that we don't crash (by trying to get signatures from `any`). +verify.quickInfoAt("", "(method) f"); From 35addce79ede5c2f439623401a0fcc2d58943215 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 19 Aug 2017 09:46:02 +0200 Subject: [PATCH 44/50] Add comment --- src/compiler/checker.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d5518eee3da..3c1ed9a9492 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1017,6 +1017,7 @@ namespace ts { } break; case SyntaxKind.ExpressionWithTypeArguments: + // The type parameters of a class are not in scope in the base class expression. if (lastLocation === (location).expression && (location.parent).token === SyntaxKind.ExtendsKeyword) { const container = location.parent.parent; if (isClassLike(container) && (result = lookup(getSymbolOfNode(container).members, name, meaning & SymbolFlags.Type))) { From 049b33693633b5dc5970db230b28fff492efcce0 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 19 Aug 2017 09:46:09 +0200 Subject: [PATCH 45/50] Accept new baselines --- .../reference/inheritFromGenericTypeParameter.errors.txt | 4 ++-- .../reference/typeParameterAsBaseClass.errors.txt | 4 ++-- .../reference/typeParameterAsBaseType.errors.txt | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/baselines/reference/inheritFromGenericTypeParameter.errors.txt b/tests/baselines/reference/inheritFromGenericTypeParameter.errors.txt index 08d138f2e5d..c043e37575c 100644 --- a/tests/baselines/reference/inheritFromGenericTypeParameter.errors.txt +++ b/tests/baselines/reference/inheritFromGenericTypeParameter.errors.txt @@ -1,11 +1,11 @@ -tests/cases/compiler/inheritFromGenericTypeParameter.ts(1,20): error TS2693: 'T' only refers to a type, but is being used as a value here. +tests/cases/compiler/inheritFromGenericTypeParameter.ts(1,20): error TS2304: Cannot find name 'T'. tests/cases/compiler/inheritFromGenericTypeParameter.ts(2,24): error TS2312: An interface may only extend a class or another interface. ==== tests/cases/compiler/inheritFromGenericTypeParameter.ts (2 errors) ==== class C extends T { } ~ -!!! error TS2693: 'T' only refers to a type, but is being used as a value here. +!!! error TS2304: Cannot find name 'T'. interface I extends T { } ~ !!! error TS2312: An interface may only extend a class or another interface. \ No newline at end of file diff --git a/tests/baselines/reference/typeParameterAsBaseClass.errors.txt b/tests/baselines/reference/typeParameterAsBaseClass.errors.txt index e633a6dd396..dec7f7f2a4a 100644 --- a/tests/baselines/reference/typeParameterAsBaseClass.errors.txt +++ b/tests/baselines/reference/typeParameterAsBaseClass.errors.txt @@ -1,11 +1,11 @@ -tests/cases/compiler/typeParameterAsBaseClass.ts(1,20): error TS2693: 'T' only refers to a type, but is being used as a value here. +tests/cases/compiler/typeParameterAsBaseClass.ts(1,20): error TS2304: Cannot find name 'T'. tests/cases/compiler/typeParameterAsBaseClass.ts(2,24): error TS2422: A class may only implement another class or interface. ==== tests/cases/compiler/typeParameterAsBaseClass.ts (2 errors) ==== class C extends T {} ~ -!!! error TS2693: 'T' only refers to a type, but is being used as a value here. +!!! error TS2304: Cannot find name 'T'. class C2 implements T {} ~ !!! error TS2422: A class may only implement another class or interface. \ No newline at end of file diff --git a/tests/baselines/reference/typeParameterAsBaseType.errors.txt b/tests/baselines/reference/typeParameterAsBaseType.errors.txt index c16b16d0b1a..835b2369948 100644 --- a/tests/baselines/reference/typeParameterAsBaseType.errors.txt +++ b/tests/baselines/reference/typeParameterAsBaseType.errors.txt @@ -1,5 +1,5 @@ -tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(4,20): error TS2693: 'T' only refers to a type, but is being used as a value here. -tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(5,24): error TS2693: 'U' only refers to a type, but is being used as a value here. +tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(4,20): error TS2304: Cannot find name 'T'. +tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(5,24): error TS2304: Cannot find name 'U'. tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(7,24): error TS2312: An interface may only extend a class or another interface. tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(8,28): error TS2312: An interface may only extend a class or another interface. @@ -10,10 +10,10 @@ tests/cases/conformance/types/typeParameters/typeParameterAsBaseType.ts(8,28): e class C extends T { } ~ -!!! error TS2693: 'T' only refers to a type, but is being used as a value here. +!!! error TS2304: Cannot find name 'T'. class C2 extends U { } ~ -!!! error TS2693: 'U' only refers to a type, but is being used as a value here. +!!! error TS2304: Cannot find name 'U'. interface I extends T { } ~ From 914d428ff12a7c7c8a7c2fc778a83810b852bd5f Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 19 Aug 2017 09:53:46 +0200 Subject: [PATCH 46/50] Add regression test --- .../baseExpressionTypeParameters.errors.txt | 19 +++++++ .../reference/baseExpressionTypeParameters.js | 50 +++++++++++++++++++ .../compiler/baseExpressionTypeParameters.ts | 13 +++++ 3 files changed, 82 insertions(+) create mode 100644 tests/baselines/reference/baseExpressionTypeParameters.errors.txt create mode 100644 tests/baselines/reference/baseExpressionTypeParameters.js create mode 100644 tests/cases/compiler/baseExpressionTypeParameters.ts diff --git a/tests/baselines/reference/baseExpressionTypeParameters.errors.txt b/tests/baselines/reference/baseExpressionTypeParameters.errors.txt new file mode 100644 index 00000000000..4fa8f2f6d3d --- /dev/null +++ b/tests/baselines/reference/baseExpressionTypeParameters.errors.txt @@ -0,0 +1,19 @@ +tests/cases/compiler/baseExpressionTypeParameters.ts(10,27): error TS2561: Base class expressions cannot reference class type parameters. + + +==== tests/cases/compiler/baseExpressionTypeParameters.ts (1 errors) ==== + // Repro from #17829 + + function base() { + class Base { + static prop: T; + } + return Base; + } + + class Gen extends base() {} // Error, T not in scope + ~ +!!! error TS2561: Base class expressions cannot reference class type parameters. + class Spec extends Gen {} + + Spec.prop; \ No newline at end of file diff --git a/tests/baselines/reference/baseExpressionTypeParameters.js b/tests/baselines/reference/baseExpressionTypeParameters.js new file mode 100644 index 00000000000..a3a77cbc6e3 --- /dev/null +++ b/tests/baselines/reference/baseExpressionTypeParameters.js @@ -0,0 +1,50 @@ +//// [baseExpressionTypeParameters.ts] +// Repro from #17829 + +function base() { + class Base { + static prop: T; + } + return Base; +} + +class Gen extends base() {} // Error, T not in scope +class Spec extends Gen {} + +Spec.prop; + +//// [baseExpressionTypeParameters.js] +// Repro from #17829 +var __extends = (this && this.__extends) || (function () { + var extendStatics = Object.setPrototypeOf || + ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) || + function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; }; + return function (d, b) { + extendStatics(d, b); + function __() { this.constructor = d; } + d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); + }; +})(); +function base() { + var Base = /** @class */ (function () { + function Base() { + } + return Base; + }()); + return Base; +} +var Gen = /** @class */ (function (_super) { + __extends(Gen, _super); + function Gen() { + return _super !== null && _super.apply(this, arguments) || this; + } + return Gen; +}(base())); // Error, T not in scope +var Spec = /** @class */ (function (_super) { + __extends(Spec, _super); + function Spec() { + return _super !== null && _super.apply(this, arguments) || this; + } + return Spec; +}(Gen)); +Spec.prop; diff --git a/tests/cases/compiler/baseExpressionTypeParameters.ts b/tests/cases/compiler/baseExpressionTypeParameters.ts new file mode 100644 index 00000000000..ca864f9a7f0 --- /dev/null +++ b/tests/cases/compiler/baseExpressionTypeParameters.ts @@ -0,0 +1,13 @@ +// Repro from #17829 + +function base() { + class Base { + static prop: T; + } + return Base; +} + +class Gen extends base() {} // Error, T not in scope +class Spec extends Gen {} + +Spec.prop; \ No newline at end of file From 07e1d3b13d66e82f901ba8ceb02e6eb90de797ef Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Mon, 21 Aug 2017 09:44:03 -0700 Subject: [PATCH 47/50] Ensure string enums are generated in protocol.d.ts (#17914) --- scripts/buildProtocol.ts | 14 +++++++++----- src/server/protocol.ts | 4 ++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/scripts/buildProtocol.ts b/scripts/buildProtocol.ts index e03338bf60d..c2ac33c83fc 100644 --- a/scripts/buildProtocol.ts +++ b/scripts/buildProtocol.ts @@ -7,11 +7,15 @@ function endsWith(s: string, suffix: string) { return s.lastIndexOf(suffix, s.length - suffix.length) !== -1; } +function isStringEnum(declaration: ts.EnumDeclaration) { + return declaration.members.length && declaration.members.every(m => m.initializer && m.initializer.kind === ts.SyntaxKind.StringLiteral); +} + class DeclarationsWalker { private visitedTypes: ts.Type[] = []; private text = ""; private removedTypes: ts.Type[] = []; - + private constructor(private typeChecker: ts.TypeChecker, private protocolFile: ts.SourceFile) { } @@ -19,7 +23,7 @@ class DeclarationsWalker { let text = "declare namespace ts.server.protocol {\n"; var walker = new DeclarationsWalker(typeChecker, protocolFile); walker.visitTypeNodes(protocolFile); - text = walker.text + text = walker.text ? `declare namespace ts.server.protocol {\n${walker.text}}` : ""; if (walker.removedTypes) { @@ -52,7 +56,7 @@ class DeclarationsWalker { if (sourceFile === this.protocolFile || path.basename(sourceFile.fileName) === "lib.d.ts") { return; } - if (decl.kind === ts.SyntaxKind.EnumDeclaration) { + if (decl.kind === ts.SyntaxKind.EnumDeclaration && !isStringEnum(decl as ts.EnumDeclaration)) { this.removedTypes.push(type); return; } @@ -91,7 +95,7 @@ class DeclarationsWalker { for (const type of heritageClauses[0].types) { this.processTypeOfNode(type); } - } + } break; } } @@ -110,7 +114,7 @@ class DeclarationsWalker { this.processType(type); } } - } + } } function writeProtocolFile(outputFile: string, protocolTs: string, typeScriptServicesDts: string) { diff --git a/src/server/protocol.ts b/src/server/protocol.ts index f50c8731126..37bf79837c9 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -2489,6 +2489,7 @@ namespace ts.server.protocol { System = "System", ES6 = "ES6", ES2015 = "ES2015", + ESNext = "ESNext" } export const enum ModuleResolutionKind { @@ -2506,5 +2507,8 @@ namespace ts.server.protocol { ES5 = "ES5", ES6 = "ES6", ES2015 = "ES2015", + ES2016 = "ES2016", + ES2017 = "ES2017", + ESNext = "ESNext" } } From 2b28916e5ee9f3f02f780b448ac8a58d31b84628 Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 21 Aug 2017 11:34:53 -0700 Subject: [PATCH 48/50] Simplify resolveBaseTypesOfClass (#17918) --- src/compiler/checker.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6febaf0052d..f7921181568 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4929,7 +4929,7 @@ namespace ts { } function resolveBaseTypesOfClass(type: InterfaceType): void { - type.resolvedBaseTypes = type.resolvedBaseTypes || emptyArray; + type.resolvedBaseTypes = emptyArray; const baseConstructorType = getApparentType(getBaseConstructorTypeOfClass(type)); if (!(baseConstructorType.flags & (TypeFlags.Object | TypeFlags.Intersection | TypeFlags.Any))) { return; @@ -4976,17 +4976,12 @@ namespace ts { error(baseTypeNode.expression, Diagnostics.Base_constructor_return_type_0_is_not_a_class_or_interface_type, typeToString(baseType)); return; } - if (type === baseType || hasBaseType(baseType, type)) { + if (type === baseType || hasBaseType(baseType, type)) { error(valueDecl, Diagnostics.Type_0_recursively_references_itself_as_a_base_type, typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType)); return; } - if (type.resolvedBaseTypes === emptyArray) { - type.resolvedBaseTypes = [baseType]; - } - else { - type.resolvedBaseTypes.push(baseType); - } + type.resolvedBaseTypes = [baseType]; } function areAllOuterTypeParametersApplied(type: Type): boolean { @@ -5003,7 +4998,7 @@ namespace ts { // A valid base type is `any`, any non-generic object type or intersection of non-generic // object types. - function isValidBaseType(type: Type): boolean { + function isValidBaseType(type: Type): type is BaseType { return type.flags & (TypeFlags.Object | TypeFlags.NonPrimitive | TypeFlags.Any) && !isGenericMappedType(type) || type.flags & TypeFlags.Intersection && !forEach((type).types, t => !isValidBaseType(t)); } @@ -5016,12 +5011,12 @@ namespace ts { const baseType = getTypeFromTypeNode(node); if (baseType !== unknownType) { if (isValidBaseType(baseType)) { - if (type !== baseType && !hasBaseType(baseType, type)) { + if (type !== baseType && !hasBaseType(baseType, type)) { if (type.resolvedBaseTypes === emptyArray) { type.resolvedBaseTypes = [baseType]; } else { - type.resolvedBaseTypes.push(baseType); + type.resolvedBaseTypes.push(baseType); } } else { From ac098535cbd0db5f7f401d7e391f54274fd44639 Mon Sep 17 00:00:00 2001 From: Basarat Ali Syed Date: Tue, 22 Aug 2017 09:55:40 +1000 Subject: [PATCH 49/50] =?UTF-8?q?export=20UsageEntry=20used=20by=20already?= =?UTF-8?q?=20exported=20functions=20=F0=9F=8C=B9=20(#17853)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/services/refactors/extractMethod.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index cf58ede99bd..90e33041695 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -856,7 +856,7 @@ namespace ts.refactor.extractMethod { Write = 2 } - interface UsageEntry { + export interface UsageEntry { readonly usage: Usage; readonly symbol: Symbol; readonly node: Node; From 6678d961aabdaea865308a81000d0b5cdf48e6d3 Mon Sep 17 00:00:00 2001 From: Matt Mitchell Date: Tue, 22 Aug 2017 09:47:29 -0700 Subject: [PATCH 50/50] Update imaged with Java 8 and other patches (#17965) Updated image, java 8 required --- netci.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netci.groovy b/netci.groovy index bc512f6b245..c60957730c4 100644 --- a/netci.groovy +++ b/netci.groovy @@ -17,6 +17,6 @@ nodeVersions.each { nodeVer -> } Utilities.standardJobSetup(newJob, project, true, "*/${branch}") - Utilities.setMachineAffinity(newJob, 'Ubuntu', '20161020') + Utilities.setMachineAffinity(newJob, 'Ubuntu14.04', '20170821-1') Utilities.addGithubPRTriggerForBranch(newJob, branch, "TypeScript Test Run ${newJobName}") }