From bfca3c55485a2ffe6d610c8ac25d60b251e56de8 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Nov 2014 18:49:05 -0800 Subject: [PATCH 1/7] Fix the line preservation between jsdoc comments while displaying it Fixes #891 --- src/services/services.ts | 31 +++- .../fourslash/commentsLinePreservation.ts | 161 ++++++++++++++++++ 2 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/commentsLinePreservation.ts diff --git a/src/services/services.ts b/src/services/services.ts index 4f3e84661ad..08e76f08ec7 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -346,7 +346,8 @@ module ts { function isName(pos: number, end: number, sourceFile: SourceFile, name: string) { return pos + name.length < end && sourceFile.text.substr(pos, name.length) === name && - isWhiteSpace(sourceFile.text.charCodeAt(pos + name.length)); + (isWhiteSpace(sourceFile.text.charCodeAt(pos + name.length)) || + isLineBreak(sourceFile.text.charCodeAt(pos + name.length))); } function isParamTag(pos: number, end: number, sourceFile: SourceFile) { @@ -354,9 +355,19 @@ module ts { return isName(pos, end, sourceFile, paramTag); } + function pushDocCommentLineText(docComments: SymbolDisplayPart[], text: string, blankLineCount: number) { + // Add the empty lines in between texts + while (blankLineCount) { + docComments.push(textPart("")); + blankLineCount--; + } + docComments.push(textPart(text)); + } + function getCleanedJsDocComment(pos: number, end: number, sourceFile: SourceFile) { var spacesToRemoveAfterAsterisk: number; var docComments: SymbolDisplayPart[] = []; + var blankLineCount = 0; var isInParamTag = false; while (pos < end) { @@ -405,7 +416,12 @@ module ts { // Continue with next line pos = consumeLineBreaks(pos, end, sourceFile); if (docCommentTextOfLine) { - docComments.push(textPart(docCommentTextOfLine)); + pushDocCommentLineText(docComments, docCommentTextOfLine, blankLineCount); + blankLineCount = 0; + } + else if (!isInParamTag && docComments.length) { + // This is blank line when there is text already parsed + blankLineCount++; } } @@ -417,6 +433,8 @@ module ts { var paramDocComments: SymbolDisplayPart[] = []; while (pos < end) { if (isParamTag(pos, end, sourceFile)) { + var blankLineCount = 0; + var recordedParamTag = false; // Consume leading spaces pos = consumeWhiteSpaces(pos + paramTag.length); if (pos >= end) { @@ -478,8 +496,13 @@ module ts { // at line break, set this comment line text and go to next line if (isLineBreak(ch)) { if (paramHelpString) { - paramDocComments.push(textPart(paramHelpString)); + pushDocCommentLineText(paramDocComments, paramHelpString, blankLineCount); paramHelpString = ""; + blankLineCount = 0; + recordedParamTag = true; + } + else if (recordedParamTag) { + blankLineCount++; } // Get the pos after cleaning start of the line @@ -500,7 +523,7 @@ module ts { // If there is param help text, add it top the doc comments if (paramHelpString) { - paramDocComments.push(textPart(paramHelpString)); + pushDocCommentLineText(paramDocComments, paramHelpString, blankLineCount); } paramHelpStringMargin = undefined; } diff --git a/tests/cases/fourslash/commentsLinePreservation.ts b/tests/cases/fourslash/commentsLinePreservation.ts new file mode 100644 index 00000000000..012272d7544 --- /dev/null +++ b/tests/cases/fourslash/commentsLinePreservation.ts @@ -0,0 +1,161 @@ +/// + +/////** This is firstLine +//// * This is second Line +//// * +//// * This is fourth Line +//// */ +////var /*a*/a: string; +/////** +//// * This is firstLine +//// * This is second Line +//// * +//// * This is fourth Line +//// */ +////var /*b*/b: string; +/////** +//// * This is firstLine +//// * This is second Line +//// * +//// * This is fourth Line +//// * +//// */ +////var /*c*/c: string; +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param +//// * @random tag This should be third line +//// */ +////function /*d*/d(param: string) { /*1*/param = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param +//// */ +////function /*e*/e(param: string) { /*2*/param = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 first line of param +//// * +//// * param information third line +//// * @random tag This should be third line +//// */ +////function /*f*/f(param1: string) { /*3*/param1 = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 +//// * +//// * param information first line +//// * @random tag This should be third line +//// */ +////function /*g*/g(param1: string) { /*4*/param1 = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 +//// * +//// * param information first line +//// * +//// * param information third line +//// * @random tag This should be third line +//// */ +////function /*h*/h(param1: string) { /*5*/param1 = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 +//// * +//// * param information first line +//// * +//// * param information third line +//// * +//// */ +////function /*i*/i(param1: string) { /*6*/param1 = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 +//// * +//// * param information first line +//// * +//// * param information third line +//// */ +////function /*j*/j(param1: string) { /*7*/param1 = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 hello @randomtag +//// * +//// * random information first line +//// * +//// * random information third line +//// */ +////function /*k*/k(param1: string) { /*8*/param1 = "hello"; } +/////** +//// * This is firstLine +//// * This is second Line +//// * @param param1 first Line text +//// * +//// * @param param1 +//// * +//// * blank line that shouldnt be shown when starting this +//// * second time information about the param again +//// */ +////function /*l*/l(param1: string) { /*9*/param1 = "hello"; } + +goTo.marker('a'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n\nThis is fourth Line"); + +goTo.marker('b'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n\nThis is fourth Line"); + +goTo.marker('c'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n\nThis is fourth Line"); + +goTo.marker('d'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line"); +goTo.marker('1'); +verify.quickInfoIs(undefined, ""); + +goTo.marker('e'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line"); +goTo.marker('2'); +verify.quickInfoIs(undefined, ""); + +goTo.marker('f'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line"); +goTo.marker('3'); +verify.quickInfoIs(undefined, "first line of param\n\nparam information third line"); + +goTo.marker('g'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line"); +goTo.marker('4'); +verify.quickInfoIs(undefined, "param information first line"); + +goTo.marker('h'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line"); +goTo.marker('5'); +verify.quickInfoIs(undefined, "param information first line\n\nparam information third line"); + +goTo.marker('i'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line"); +goTo.marker('6'); +verify.quickInfoIs(undefined, "param information first line\n\nparam information third line"); + +goTo.marker('j'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line"); +goTo.marker('7'); +verify.quickInfoIs(undefined, "param information first line\n\nparam information third line"); + +goTo.marker('k'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@randomtag \n\n random information first line\n\n random information third line"); +goTo.marker('8'); +verify.quickInfoIs(undefined, "hello "); + +goTo.marker('l'); +verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line"); +goTo.marker('9'); +verify.quickInfoIs(undefined, "first Line text\nblank line that shouldnt be shown when starting this \nsecond time information about the param again"); From 28fa6029eb319e235f5e1df2d1a77db801da9189 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Nov 2014 19:00:44 -0800 Subject: [PATCH 2/7] Fix the crash in getCompilerOptionsDiagnostics by not using file name as compiler options do not have file name Fixes #988 --- src/services/shims.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/services/shims.ts b/src/services/shims.ts index b93c699be73..c6705e34363 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -507,17 +507,6 @@ module ts { }; } - private realizeDiagnosticWithFileName(diagnostic: Diagnostic): { fileName: string; message: string; start: number; length: number; category: string; } { - return { - fileName: diagnostic.file.filename, - message: diagnostic.messageText, - start: diagnostic.start, - length: diagnostic.length, - /// TODO: no need for the tolowerCase call - category: DiagnosticCategory[diagnostic.category].toLowerCase() - }; - } - public getSyntacticClassifications(fileName: string, start: number, length: number): string { return this.forwardJSONCall( "getSyntacticClassifications('" + fileName + "', " + start + ", " + length + ")", @@ -559,7 +548,7 @@ module ts { "getCompilerOptionsDiagnostics()", () => { var errors = this.languageService.getCompilerOptionsDiagnostics(); - return errors.map(d => this.realizeDiagnosticWithFileName(d)) + return errors.map(LanguageServiceShimObject.realizeDiagnostic) }); } From 127aa49e55d004e1235cae53610ebbfaa5ad5672 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Nov 2014 19:18:20 -0800 Subject: [PATCH 3/7] Fix the union property kind if the property is union of exported variable of module Fixes #929 --- src/services/services.ts | 4 ++-- ...mpletionEntryForPropertyFromUnionOfModuleType.ts | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/completionEntryForPropertyFromUnionOfModuleType.ts diff --git a/src/services/services.ts b/src/services/services.ts index 4f3e84661ad..42f25a8a184 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2784,10 +2784,10 @@ module ts { if (flags & SymbolFlags.Property) { if (flags & SymbolFlags.UnionProperty) { - // If union property is result of union of non method (property/accessors), it is labeled as property + // If union property is result of union of non method (property/accessors/variables), it is labeled as property var unionPropertyKind = forEach(typeInfoResolver.getRootSymbols(symbol), rootSymbol => { var rootSymbolFlags = rootSymbol.getFlags(); - if (rootSymbolFlags & (SymbolFlags.Property | SymbolFlags.GetAccessor | SymbolFlags.SetAccessor)) { + if (rootSymbolFlags & (SymbolFlags.PropertyOrAccessor | SymbolFlags.Variable)) { return ScriptElementKind.memberVariableElement; } Debug.assert(!!(rootSymbolFlags & SymbolFlags.Method)); diff --git a/tests/cases/fourslash/completionEntryForPropertyFromUnionOfModuleType.ts b/tests/cases/fourslash/completionEntryForPropertyFromUnionOfModuleType.ts new file mode 100644 index 00000000000..1e072acd1fa --- /dev/null +++ b/tests/cases/fourslash/completionEntryForPropertyFromUnionOfModuleType.ts @@ -0,0 +1,13 @@ +/// + +////module E { +//// export var n = 1; +////} +////module F { +//// export var n = 1; +////} +////var q: typeof E | typeof F; +////var j = q./*1*/ + +goTo.marker('1'); +verify.completionListContains('n', "(property) n: number"); \ No newline at end of file From 38f14b5b1db8fd1ca2ed8f25e33086e731090060 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Nov 2014 19:37:05 -0800 Subject: [PATCH 4/7] Block completion list on import declaration name and fix the crash in symbol display name when import declaration is incomplete Fixes #991 --- src/services/services.ts | 14 ++++++++++---- .../fourslash/completionEntryForImportName.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/completionEntryForImportName.ts diff --git a/src/services/services.ts b/src/services/services.ts index 4f3e84661ad..dd22a6bde56 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2614,6 +2614,7 @@ module ts { case SyntaxKind.VarKeyword: case SyntaxKind.GetKeyword: case SyntaxKind.SetKeyword: + case SyntaxKind.ImportKeyword: return true; } @@ -3061,13 +3062,13 @@ module ts { displayParts.push(keywordPart(SyntaxKind.ImportKeyword)); displayParts.push(spacePart()); addFullSymbolName(symbol); - displayParts.push(spacePart()); - displayParts.push(punctuationPart(SyntaxKind.EqualsToken)); - displayParts.push(spacePart()); ts.forEach(symbol.declarations, declaration => { if (declaration.kind === SyntaxKind.ImportDeclaration) { var importDeclaration = declaration; if (importDeclaration.externalModuleName) { + displayParts.push(spacePart()); + displayParts.push(punctuationPart(SyntaxKind.EqualsToken)); + displayParts.push(spacePart()); displayParts.push(keywordPart(SyntaxKind.RequireKeyword)); displayParts.push(punctuationPart(SyntaxKind.OpenParenToken)); displayParts.push(displayPart(getTextOfNode(importDeclaration.externalModuleName), SymbolDisplayPartKind.stringLiteral)); @@ -3075,7 +3076,12 @@ module ts { } else { var internalAliasSymbol = typeResolver.getSymbolInfo(importDeclaration.entityName); - addFullSymbolName(internalAliasSymbol, enclosingDeclaration); + if (internalAliasSymbol) { + displayParts.push(spacePart()); + displayParts.push(punctuationPart(SyntaxKind.EqualsToken)); + displayParts.push(spacePart()); + addFullSymbolName(internalAliasSymbol, enclosingDeclaration); + } } return true; } diff --git a/tests/cases/fourslash/completionEntryForImportName.ts b/tests/cases/fourslash/completionEntryForImportName.ts new file mode 100644 index 00000000000..08e46c40720 --- /dev/null +++ b/tests/cases/fourslash/completionEntryForImportName.ts @@ -0,0 +1,14 @@ +/// + +////import /*1*/q = /*2*/ + +verifyIncompleteImport(); +goTo.marker('2'); +edit.insert("a."); +verifyIncompleteImport(); + +function verifyIncompleteImport() { + goTo.marker('1'); + verify.completionListIsEmpty(); + verify.quickInfoIs("import q"); +} \ No newline at end of file From 5acc1a1bd21e7aafb3bd5066969af6a7ddfd547b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 6 Nov 2014 19:51:00 -0800 Subject: [PATCH 5/7] Fix the crash in completion entry by fixing the unknown token check Fixes #1069 --- src/services/utilities.ts | 5 ++++- tests/cases/fourslash/completionAfterAtChar.ts | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionAfterAtChar.ts diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 8dcd06f5621..88e3601b610 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -224,7 +224,10 @@ module ts { return nodeHasTokens((n).expression); } - if (n.kind === SyntaxKind.EndOfFileToken || n.kind === SyntaxKind.OmittedExpression || n.kind === SyntaxKind.Missing) { + if (n.kind === SyntaxKind.EndOfFileToken || + n.kind === SyntaxKind.OmittedExpression || + n.kind === SyntaxKind.Missing || + n.kind === SyntaxKind.Unknown) { return false; } diff --git a/tests/cases/fourslash/completionAfterAtChar.ts b/tests/cases/fourslash/completionAfterAtChar.ts new file mode 100644 index 00000000000..4f3581cd483 --- /dev/null +++ b/tests/cases/fourslash/completionAfterAtChar.ts @@ -0,0 +1,6 @@ +/// + +////@a/**/ + +goTo.marker(); +verify.not.completionListIsEmpty(); \ No newline at end of file From b743a50677616e0fb34ac90b7c6cfd3ddc1a2fa8 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 7 Nov 2014 12:19:06 -0800 Subject: [PATCH 6/7] Simplified while loop as per code review feedback --- src/services/services.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 08e76f08ec7..c915966b515 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -357,10 +357,7 @@ module ts { function pushDocCommentLineText(docComments: SymbolDisplayPart[], text: string, blankLineCount: number) { // Add the empty lines in between texts - while (blankLineCount) { - docComments.push(textPart("")); - blankLineCount--; - } + while (blankLineCount--) docComments.push(textPart("")); docComments.push(textPart(text)); } From 3c600608d3ebbae13738ee6b7361f5ef52a734e5 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 7 Nov 2014 12:32:08 -0800 Subject: [PATCH 7/7] Test now covers completion entry at name of the import when there is no name and there is name --- .../fourslash/completionEntryForImportName.ts | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/cases/fourslash/completionEntryForImportName.ts b/tests/cases/fourslash/completionEntryForImportName.ts index 08e46c40720..5773eb5376e 100644 --- a/tests/cases/fourslash/completionEntryForImportName.ts +++ b/tests/cases/fourslash/completionEntryForImportName.ts @@ -1,13 +1,23 @@ /// -////import /*1*/q = /*2*/ +////import /*1*/ /*2*/ + +goTo.marker('1'); +verify.completionListIsEmpty(); +edit.insert('q'); +verify.completionListIsEmpty(); +verifyIncompleteImportName(); -verifyIncompleteImport(); goTo.marker('2'); -edit.insert("a."); -verifyIncompleteImport(); +edit.insert(" = "); +verifyIncompleteImportName(); -function verifyIncompleteImport() { +goTo.marker("2"); +edit.moveRight(" = ".length); +edit.insert("a."); +verifyIncompleteImportName(); + +function verifyIncompleteImportName() { goTo.marker('1'); verify.completionListIsEmpty(); verify.quickInfoIs("import q");