From ce133ec2dd88a7d5fdfe0d5019f6fc0704a36436 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 1 Aug 2019 17:21:29 -0700 Subject: [PATCH 01/13] Fixed issue for navbar when having multiline string literals --- src/services/navigationBar.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 60f1283f7e6..1e60ac824f3 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -194,7 +194,7 @@ namespace ts.NavigationBar { // Handle named bindings in imports e.g.: // import * as NS from "mod"; // import {a, b as B} from "mod"; - const {namedBindings} = importClause; + const { namedBindings } = importClause; if (namedBindings) { if (namedBindings.kind === SyntaxKind.NamespaceImport) { addLeafNode(namedBindings); @@ -660,7 +660,16 @@ namespace ts.NavigationBar { else if (isCallExpression(parent)) { const name = getCalledExpressionName(parent.expression); if (name !== undefined) { - const args = mapDefined(parent.arguments, a => isStringLiteralLike(a) ? a.getText(curSourceFile) : undefined).join(", "); + const args = mapDefined(parent.arguments, a => { + if (isStringLiteralLike(a)) { + const line = curSourceFile.getLineAndCharacterOfPosition(a.pos).line; + const endOfLine = getEndLinePosition(line, curSourceFile); + + return a.getText(curSourceFile).substring(0, endOfLine - a.pos); + } + + return undefined; + }).join(", "); return `${name}(${args}) callback`; } } From 98729b44627c315fc14f20bba42099a6a701f442 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Wed, 7 Aug 2019 13:50:18 -0700 Subject: [PATCH 02/13] Truncate to 150 chars and added unit tests --- src/services/navigationBar.ts | 43 +++++++--- ...ionBarItemsMultilineStringIdentifiers1.ts} | 50 ++++++----- ...tionBarItemsMultilineStringIdentifiers2.ts | 85 +++++++++++++++++++ ...tionBarItemsMultilineStringIdentifiers3.ts | 40 +++++++++ 4 files changed, 181 insertions(+), 37 deletions(-) rename tests/cases/fourslash/{navigationBarItemsMultilineStringIdentifiers.ts => navigationBarItemsMultilineStringIdentifiers1.ts} (85%) create mode 100644 tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts create mode 100644 tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 1e60ac824f3..6d85f76a830 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -15,6 +15,12 @@ namespace ts.NavigationBar { */ const whiteSpaceRegex = /\s+/g; + /** + * Maximum amount of characters to return + * The amount was choosen arbitrarily. + */ + const maxLength = 150; + // Keep sourceFile handy so we don't have to search for it every time we need to call `getText`. let curCancellationToken: CancellationToken; let curSourceFile: SourceFile; @@ -74,7 +80,7 @@ namespace ts.NavigationBar { } function nodeText(node: Node): string { - return node.getText(curSourceFile); + return cleanText(node.getText(curSourceFile)); } function navigationBarNodeKind(n: NavigationBarNode): SyntaxKind { @@ -434,13 +440,13 @@ namespace ts.NavigationBar { function getItemName(node: Node, name: Node | undefined): string { if (node.kind === SyntaxKind.ModuleDeclaration) { - return getModuleName(node); + return cleanText(getModuleName(node)); } if (name) { const text = nodeText(name); if (text.length > 0) { - return text; + return cleanText(text); } } @@ -636,11 +642,11 @@ namespace ts.NavigationBar { function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { const { parent } = node; if (node.name && getFullWidth(node.name) > 0) { - return declarationNameToString(node.name); + return cleanText(declarationNameToString(node.name)); } // See if it is a var initializer. If so, use the var name. else if (isVariableDeclaration(parent)) { - return declarationNameToString(parent.name); + return cleanText(declarationNameToString(parent.name)); } // See if it is of the form " = function(){...}". If so, use the text from the left-hand side. else if (isBinaryExpression(parent) && parent.operatorToken.kind === SyntaxKind.EqualsToken) { @@ -658,18 +664,15 @@ namespace ts.NavigationBar { return ""; } else if (isCallExpression(parent)) { - const name = getCalledExpressionName(parent.expression); + let name = getCalledExpressionName(parent.expression); if (name !== undefined) { - const args = mapDefined(parent.arguments, a => { - if (isStringLiteralLike(a)) { - const line = curSourceFile.getLineAndCharacterOfPosition(a.pos).line; - const endOfLine = getEndLinePosition(line, curSourceFile); + name = cleanText(name); - return a.getText(curSourceFile).substring(0, endOfLine - a.pos); - } + if (name.length > maxLength) { + return `${name} callback`; + } - return undefined; - }).join(", "); + const args = cleanText(mapDefined(parent.arguments, a => isStringLiteralLike(a) ? a.getText(curSourceFile) : undefined).join(", ")); return `${name}(${args}) callback`; } } @@ -700,4 +703,16 @@ namespace ts.NavigationBar { return false; } } + + function cleanText(text: string): string { + // Truncate to maximum amount of characters as we don't want to do a big replace operation. + text = text.length > maxLength ? text.substring(0, maxLength) + "..." : text; + + // Replaces ECMAScript line terminators and removes the trailing `\` from each line: + // \n - Line Feed + // \r - Carriage Return + // \u2028 - Line separator + // \u2029 - Paragraph separator + return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ''); + } } diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers1.ts similarity index 85% rename from tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers.ts rename to tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers1.ts index 793844ac970..e5b6c56bdcb 100644 --- a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers.ts +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers1.ts @@ -7,6 +7,10 @@ ////} ////declare module "MultilineMadness" {} //// +////declare module "Multiline\ +////Madness2" { +////} +//// ////interface Foo { //// "a1\\\r\nb"; //// "a2\ @@ -29,7 +33,12 @@ verify.navigationTree({ "kind": "script", "childItems": [ { - "text": "\"Multiline\\\nMadness\"", + "text": "\"MultilineMadness\"", + "kind": "module", + "kindModifiers": "declare" + }, + { + "text": "\"MultilineMadness2\"", "kind": "module", "kindModifiers": "declare" }, @@ -38,11 +47,6 @@ verify.navigationTree({ "kind": "module", "kindModifiers": "declare" }, - { - "text": "\"MultilineMadness\"", - "kind": "module", - "kindModifiers": "declare" - }, { "text": "Bar", "kind": "class", @@ -52,7 +56,7 @@ verify.navigationTree({ "kind": "property" }, { - "text": "'a2\\\n \\\n b'", + "text": "'a2 b'", "kind": "method" } ] @@ -66,7 +70,7 @@ verify.navigationTree({ "kind": "property" }, { - "text": "\"a2\\\n \\\n b\"", + "text": "\"a2 b\"", "kind": "method" } ] @@ -80,7 +84,12 @@ verify.navigationBar([ "kind": "script", "childItems": [ { - "text": "\"Multiline\\\nMadness\"", + "text": "\"MultilineMadness\"", + "kind": "module", + "kindModifiers": "declare" + }, + { + "text": "\"MultilineMadness2\"", "kind": "module", "kindModifiers": "declare" }, @@ -89,11 +98,6 @@ verify.navigationBar([ "kind": "module", "kindModifiers": "declare" }, - { - "text": "\"MultilineMadness\"", - "kind": "module", - "kindModifiers": "declare" - }, { "text": "Bar", "kind": "class" @@ -105,7 +109,13 @@ verify.navigationBar([ ] }, { - "text": "\"Multiline\\\nMadness\"", + "text": "\"MultilineMadness\"", + "kind": "module", + "kindModifiers": "declare", + "indent": 1 + }, + { + "text": "\"MultilineMadness2\"", "kind": "module", "kindModifiers": "declare", "indent": 1 @@ -116,12 +126,6 @@ verify.navigationBar([ "kindModifiers": "declare", "indent": 1 }, - { - "text": "\"MultilineMadness\"", - "kind": "module", - "kindModifiers": "declare", - "indent": 1 - }, { "text": "Bar", "kind": "class", @@ -131,7 +135,7 @@ verify.navigationBar([ "kind": "property" }, { - "text": "'a2\\\n \\\n b'", + "text": "'a2 b'", "kind": "method" } ], @@ -146,7 +150,7 @@ verify.navigationBar([ "kind": "property" }, { - "text": "\"a2\\\n \\\n b\"", + "text": "\"a2 b\"", "kind": "method" } ], diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts new file mode 100644 index 00000000000..8c3fe8b4c09 --- /dev/null +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts @@ -0,0 +1,85 @@ + +//// function f(p1: () => any, p2: string) { } +//// f(() => { }, `line1\ +//// line2\ +//// line3`); +//// +//// class c1 { +//// const a = ' ''line1\ +//// line2'; +//// } + +verify.navigationTree({ + "text": "", + "kind": "script", + "childItems": [ + { + "text": "c1", + "kind": "class", + "childItems": [ + { + "text": "a", + "kind": "property" + }, + { + "text": "'line1 line2'", + "kind": "property" + } + ] + }, + { + "text": "f", + "kind": "function" + }, + { + "text": "f(`line1line2line3`) callback", + "kind": "function" + } + ] +}); + +verify.navigationBar([ + { + "text": "", + "kind": "script", + "childItems": [ + { + "text": "c1", + "kind": "class" + }, + { + "text": "f", + "kind": "function" + }, + { + "text": "f(`line1line2line3`) callback", + "kind": "function" + } + ] + }, + { + "text": "c1", + "kind": "class", + "childItems": [ + { + "text": "a", + "kind": "property" + }, + { + "text": "'line1 line2'", + "kind": "property" + } + ], + "indent": 1 + }, + { + "text": "f", + "kind": "function", + "indent": 1 + }, + { + "text": "f(`line1line2line3`) callback", + "kind": "function", + "indent": 1 + } +]); diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts new file mode 100644 index 00000000000..f04ce528493 --- /dev/null +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers3.ts @@ -0,0 +1,40 @@ + +//// declare module 'MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters\ +//// MoreThanOneHundredAndFiftyCharacters' { } + +verify.navigationTree({ + "text": "", + "kind": "script", + "childItems": [ + { + "text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...", + "kind": "module", + "kindModifiers": "declare" + } + ] +}); + +verify.navigationBar([ + { + "text": "", + "kind": "script", + "childItems": [ + { + "text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...", + "kind": "module", + "kindModifiers": "declare" + } + ] + }, + { + "text": "'MoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharactersMoreThanOneHundredAndFiftyCharacter...", + "kind": "module", + "kindModifiers": "declare", + "indent": 1 + } +]); From 209ca0749faccfc7f3dd04a49acf5a83c78e1267 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 8 Aug 2019 14:29:03 -0700 Subject: [PATCH 03/13] Fixed lint issues --- src/services/navigationBar.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 6d85f76a830..8b7afce9e1c 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -15,8 +15,8 @@ namespace ts.NavigationBar { */ const whiteSpaceRegex = /\s+/g; - /** - * Maximum amount of characters to return + /** + * Maximum amount of characters to return * The amount was choosen arbitrarily. */ const maxLength = 150; @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ''); + return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ""); } } From 428e8d0cb16e9474fd554e77ebf60083972ac955 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Thu, 15 Aug 2019 13:49:11 -0700 Subject: [PATCH 04/13] Fixed navigationbar regex --- src/services/navigationBar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 8b7afce9e1c..57bf910efda 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\?(\r?\n|\u2028|\u2029)/g, ""); + return text.replace(/\\?(\r?\n|\r|\u2028|\u2029)/g, ""); } } From 00d37268e8fd1e52fd90f1616d8a21802dd83b0a Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Fri, 23 Aug 2019 12:55:10 -0700 Subject: [PATCH 05/13] Make triple-slash comment classification more restrictive It was overly permissive and ended up making a mess of C#-style comments: `/// Text` Now it checks the element name. Attribute names remain unchecked. --- src/services/classifier.ts | 12 ++++++++---- .../syntacticClassificationsTripleSlash17.ts | 7 +++++++ .../syntacticClassificationsTripleSlash18.ts | 7 +++++++ .../syntacticClassificationsTripleSlash4.ts | 3 +-- 4 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/syntacticClassificationsTripleSlash17.ts create mode 100644 tests/cases/fourslash/syntacticClassificationsTripleSlash18.ts diff --git a/src/services/classifier.ts b/src/services/classifier.ts index 1e1309a2eac..334417ef549 100644 --- a/src/services/classifier.ts +++ b/src/services/classifier.ts @@ -771,6 +771,14 @@ namespace ts { return false; } + // Limiting classification to exactly the elements and attributes + // defined in `ts.commentPragmas` would be excessive, but we can avoid + // some obvious false positives (e.g. in XML-like doc comments) by + // checking the element name. + if (!match[3] || !(commentPragmas as any)[match[3]]) { + return false; + } + let pos = start; pushCommentRange(pos, match[1].length); // /// @@ -779,10 +787,6 @@ namespace ts { pushClassification(pos, match[2].length, ClassificationType.punctuation); // < pos += match[2].length; - if (!match[3]) { - return true; - } - pushClassification(pos, match[3].length, ClassificationType.jsxSelfClosingTagName); // element name pos += match[3].length; diff --git a/tests/cases/fourslash/syntacticClassificationsTripleSlash17.ts b/tests/cases/fourslash/syntacticClassificationsTripleSlash17.ts new file mode 100644 index 00000000000..b95cdd6b6c0 --- /dev/null +++ b/tests/cases/fourslash/syntacticClassificationsTripleSlash17.ts @@ -0,0 +1,7 @@ +/// + +//// /// Text + +var c = classification; +verify.syntacticClassificationsAre( + c.comment("/// Text")); \ No newline at end of file diff --git a/tests/cases/fourslash/syntacticClassificationsTripleSlash18.ts b/tests/cases/fourslash/syntacticClassificationsTripleSlash18.ts new file mode 100644 index 00000000000..148b17f6e45 --- /dev/null +++ b/tests/cases/fourslash/syntacticClassificationsTripleSlash18.ts @@ -0,0 +1,7 @@ +/// + +//// /// Text + +var c = classification; +verify.syntacticClassificationsAre( + c.comment("/// Text")); \ No newline at end of file diff --git a/tests/cases/fourslash/syntacticClassificationsTripleSlash4.ts b/tests/cases/fourslash/syntacticClassificationsTripleSlash4.ts index e656c811377..d407bcee503 100644 --- a/tests/cases/fourslash/syntacticClassificationsTripleSlash4.ts +++ b/tests/cases/fourslash/syntacticClassificationsTripleSlash4.ts @@ -4,5 +4,4 @@ var c = classification; verify.syntacticClassificationsAre( - c.comment("/// "), - c.punctuation("<")); \ No newline at end of file + c.comment("/// <")); // Don't classify until we recognize the element name \ No newline at end of file From e3690c3a0712420de5e32c0fa5036657bfa2de22 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Fri, 23 Aug 2019 13:10:32 -0700 Subject: [PATCH 06/13] Use the in operator --- src/services/classifier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/classifier.ts b/src/services/classifier.ts index 334417ef549..5d9b4924bed 100644 --- a/src/services/classifier.ts +++ b/src/services/classifier.ts @@ -775,7 +775,7 @@ namespace ts { // defined in `ts.commentPragmas` would be excessive, but we can avoid // some obvious false positives (e.g. in XML-like doc comments) by // checking the element name. - if (!match[3] || !(commentPragmas as any)[match[3]]) { + if (!match[3] || !(match[3] in commentPragmas)) { return false; } From f76e3b59b2dc44c975df346a6fc1cb898ca9fe63 Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Fri, 23 Aug 2019 13:51:53 -0700 Subject: [PATCH 07/13] Make trailing slash required on cleanText regex --- src/services/navigationBar.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 57bf910efda..5f826a1a78f 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\?(\r?\n|\r|\u2028|\u2029)/g, ""); + return text.replace(/\\(\r?\n|\r|\u2028|\u2029)/g, ""); } } From a0c29fe4e50256a33d0a3f3fafbd47acc404433b Mon Sep 17 00:00:00 2001 From: Armando Aguirre Date: Fri, 23 Aug 2019 15:42:03 -0700 Subject: [PATCH 08/13] Added optional trailing slash regex --- src/services/navigationBar.ts | 2 +- ...gationBarItemsMultilineStringIdentifiers2.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 5f826a1a78f..57bf910efda 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -713,6 +713,6 @@ namespace ts.NavigationBar { // \r - Carriage Return // \u2028 - Line separator // \u2029 - Paragraph separator - return text.replace(/\\(\r?\n|\r|\u2028|\u2029)/g, ""); + return text.replace(/\\?(\r?\n|\r|\u2028|\u2029)/g, ""); } } diff --git a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts index 8c3fe8b4c09..0aef520785a 100644 --- a/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts +++ b/tests/cases/fourslash/navigationBarItemsMultilineStringIdentifiers2.ts @@ -8,6 +8,10 @@ //// const a = ' ''line1\ //// line2'; //// } +//// +//// f(() => { }, `unterminated backtick 1 +//// unterminated backtick 2 +//// unterminated backtick 3 verify.navigationTree({ "text": "", @@ -34,6 +38,10 @@ verify.navigationTree({ { "text": "f(`line1line2line3`) callback", "kind": "function" + }, + { + "text": "f(`unterminated backtick 1unterminated backtick 2unterminated backtick 3) callback", + "kind": "function" } ] }); @@ -54,6 +62,10 @@ verify.navigationBar([ { "text": "f(`line1line2line3`) callback", "kind": "function" + }, + { + "text": "f(`unterminated backtick 1unterminated backtick 2unterminated backtick 3) callback", + "kind": "function" } ] }, @@ -81,5 +93,10 @@ verify.navigationBar([ "text": "f(`line1line2line3`) callback", "kind": "function", "indent": 1 + }, + { + "text": "f(`unterminated backtick 1unterminated backtick 2unterminated backtick 3) callback", + "kind": "function", + "indent": 1 } ]); From b4417da646eb650713196faf5786103a0dab57b6 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 26 Aug 2019 08:46:41 -0700 Subject: [PATCH 09/13] Fix fourslash server (#33063) --- src/harness/client.ts | 8 ++++--- src/harness/fourslash.ts | 7 ++++++ src/harness/harnessLanguageService.ts | 34 +++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/harness/client.ts b/src/harness/client.ts index 4f8e84dcdaf..c41340d6eac 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -134,11 +134,13 @@ namespace ts.server { this.processRequest(CommandNames.Close, args); } - changeFile(fileName: string, start: number, end: number, insertString: string): void { + createChangeFileRequestArgs(fileName: string, start: number, end: number, insertString: string): protocol.ChangeRequestArgs { + return { ...this.createFileLocationRequestArgsWithEndLineAndOffset(fileName, start, end), insertString }; + } + + changeFile(fileName: string, args: protocol.ChangeRequestArgs): void { // clear the line map after an edit this.lineMaps.set(fileName, undefined!); // TODO: GH#18217 - - const args: protocol.ChangeRequestArgs = { ...this.createFileLocationRequestArgsWithEndLineAndOffset(fileName, start, end), insertString }; this.processRequest(CommandNames.Change, args); } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 942e860e0d8..81c83a07bd7 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -150,6 +150,7 @@ namespace FourSlash { private languageServiceAdapterHost: Harness.LanguageService.LanguageServiceAdapterHost; private languageService: ts.LanguageService; private cancellationToken: TestCancellationToken; + private assertTextConsistent: ((fileName: string) => void) | undefined; // The current caret position in the active file public currentCaretPosition = 0; @@ -280,6 +281,9 @@ namespace FourSlash { const languageServiceAdapter = this.getLanguageServiceAdapter(testType, this.cancellationToken, compilationOptions); this.languageServiceAdapterHost = languageServiceAdapter.getHost(); this.languageService = memoWrap(languageServiceAdapter.getLanguageService(), this); // Wrap the LS to cache some expensive operations certain tests call repeatedly + if (this.testType === FourSlashTestType.Server) { + this.assertTextConsistent = fileName => (languageServiceAdapter as Harness.LanguageService.ServerLanguageServiceAdapter).assertTextConsistent(fileName); + } if (startResolveFileRef) { // Add the entry-point file itself into the languageServiceShimHost @@ -1867,6 +1871,9 @@ namespace FourSlash { private editScriptAndUpdateMarkers(fileName: string, editStart: number, editEnd: number, newText: string) { this.languageServiceAdapterHost.editScript(fileName, editStart, editEnd, newText); + if (this.assertTextConsistent) { + this.assertTextConsistent(fileName); + } for (const marker of this.testData.markers) { if (marker.fileName === fileName) { marker.position = updatePosition(marker.position, editStart, editEnd, newText); diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 03ccbb5d6de..2b0c4c162fb 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -661,8 +661,9 @@ namespace Harness.LanguageService { } editScript(fileName: string, start: number, end: number, newText: string) { + const changeArgs = this.client.createChangeFileRequestArgs(fileName, start, end, newText); super.editScript(fileName, start, end, newText); - this.client.changeFile(fileName, start, end, newText); + this.client.changeFile(fileName, changeArgs); } } @@ -719,8 +720,8 @@ namespace Harness.LanguageService { return this.host.getCurrentDirectory(); } - getDirectories(): string[] { - return []; + getDirectories(path: string): string[] { + return this.host.getDirectories(path); } getEnvironmentVariable(name: string): string { @@ -890,9 +891,16 @@ namespace Harness.LanguageService { } } + class FourslashSession extends ts.server.Session { + getText(fileName: string) { + return ts.getSnapshotText(this.projectService.getDefaultProjectForFile(ts.server.toNormalizedPath(fileName), /*ensureProject*/ true)!.getScriptSnapshot(fileName)!); + } + } + export class ServerLanguageServiceAdapter implements LanguageServiceAdapter { private host: SessionClientHost; private client: ts.server.SessionClient; + private server: FourslashSession; constructor(cancellationToken?: ts.HostCancellationToken, options?: ts.CompilerOptions) { // This is the main host that tests use to direct tests const clientHost = new SessionClientHost(cancellationToken, options); @@ -912,11 +920,12 @@ namespace Harness.LanguageService { logger: serverHost, canUseEvents: true }; - const server = new ts.server.Session(opts); + this.server = new FourslashSession(opts); + // Fake the connection between the client and the server serverHost.writeMessage = client.onMessage.bind(client); - clientHost.writeMessage = server.onMessage.bind(server); + clientHost.writeMessage = this.server.onMessage.bind(this.server); // Wire the client to the host to get notifications when a file is open // or edited. @@ -930,5 +939,20 @@ namespace Harness.LanguageService { getLanguageService(): ts.LanguageService { return this.client; } getClassifier(): ts.Classifier { throw new Error("getClassifier is not available using the server interface."); } getPreProcessedFileInfo(): ts.PreProcessedFileInfo { throw new Error("getPreProcessedFileInfo is not available using the server interface."); } + assertTextConsistent(fileName: string) { + const serverText = this.server.getText(fileName); + const clientText = this.host.readFile(fileName); + ts.Debug.assert(serverText === clientText, [ + "Server and client text are inconsistent.", + "", + "\x1b[1mServer\x1b[0m\x1b[31m:", + serverText, + "", + "\x1b[1mClient\x1b[0m\x1b[31m:", + clientText, + "", + "This probably means something is wrong with the fourslash infrastructure, not with the test." + ].join(ts.sys.newLine)); + } } } From 21f192367adf0f8ecaf5dc9c8ee972f1fd292d1d Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 26 Aug 2019 10:42:17 -0700 Subject: [PATCH 10/13] Fix infer from usage prop assignment (#33088) * Add test case * Fix infer from usage property assignment Property assignment and shorthand property assignment were incorrectly treated differently; both have ObjectLiteralExpression as a parent, but the code previously assumed that property assignments had ObjectLiteralExpression as parent.parent. Also make fourslash directives case insensitive and less whitespace sensitive. * Add "incorrect 3-slash" error to fourslash parsing. --- src/harness/fourslash.ts | 20 ++++++++++++------- src/services/codefixes/inferFromUsage.ts | 10 +++------- .../codeFixInferFromUsageJSDestructuring.ts | 18 +++++++++++++++++ .../codeFixInferFromUsageJSXElement.ts | 9 +-------- 4 files changed, 35 insertions(+), 22 deletions(-) create mode 100644 tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 81c83a07bd7..ae16159bc9d 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -80,11 +80,11 @@ namespace FourSlash { // To add additional option, add property into the testOptMetadataNames, refer the property in either globalMetadataNames or fileMetadataNames // Add cases into convertGlobalOptionsToCompilationsSettings function for the compiler to acknowledge such option from meta data const enum MetadataOptionNames { - baselineFile = "BaselineFile", - emitThisFile = "emitThisFile", // This flag is used for testing getEmitOutput feature. It allows test-cases to indicate what file to be output in multiple files project - fileName = "Filename", - resolveReference = "ResolveReference", // This flag is used to specify entry file for resolve file references. The flag is only allow once per test file - symlink = "Symlink", + baselineFile = "baselinefile", + emitThisFile = "emitthisfile", // This flag is used for testing getEmitOutput feature. It allows test-cases to indicate what file to be output in multiple files project + fileName = "filename", + resolveReference = "resolvereference", // This flag is used to specify entry file for resolve file references. The flag is only allow once per test file + symlink = "symlink", } // List of allowed metadata names @@ -3281,7 +3281,7 @@ ${code} function parseTestData(basePath: string, contents: string, fileName: string): FourSlashData { // Regex for parsing options in the format "@Alpha: Value of any sort" - const optionRegex = /^\s*@(\w+): (.*)\s*/; + const optionRegex = /^\s*@(\w+):\s*(.*)\s*/; // List of all the subfiles we've parsed out const files: FourSlashFile[] = []; @@ -3293,6 +3293,7 @@ ${code} // Note: IE JS engine incorrectly handles consecutive delimiters here when using RegExp split, so // we have to string-based splitting instead and try to figure out the delimiting chars const lines = contents.split("\n"); + let i = 0; const markerPositions = ts.createMap(); const markers: Marker[] = []; @@ -3321,6 +3322,7 @@ ${code} } for (let line of lines) { + i++; if (line.length > 0 && line.charAt(line.length - 1) === "\r") { line = line.substr(0, line.length - 1); } @@ -3329,11 +3331,15 @@ ${code} const text = line.substr(4); currentFileContent = currentFileContent === undefined ? text : currentFileContent + "\n" + text; } + else if (line.substr(0, 3) === "///" && currentFileContent !== undefined) { + throw new Error("Three-slash line in the middle of four-slash region at line " + i); + } else if (line.substr(0, 2) === "//") { // Comment line, check for global/file @options and record them const match = optionRegex.exec(line.substr(2)); if (match) { - const [key, value] = match.slice(1); + const key = match[1].toLowerCase(); + const value = match[2]; if (!ts.contains(fileMetadataNames, key)) { // Check if the match is already existed in the global options if (globalOptions[key] !== undefined) { diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 82a3a9d229b..396d7f44fa8 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -717,13 +717,9 @@ namespace ts.codefix { } function inferTypeFromPropertyAssignment(assignment: PropertyAssignment | ShorthandPropertyAssignment, checker: TypeChecker, usageContext: UsageContext) { - const objectLiteral = isShorthandPropertyAssignment(assignment) ? - assignment.parent : - assignment.parent.parent; - const nodeWithRealType = isVariableDeclaration(objectLiteral.parent) ? - objectLiteral.parent : - objectLiteral; - + const nodeWithRealType = isVariableDeclaration(assignment.parent.parent) ? + assignment.parent.parent : + assignment.parent; addCandidateThisType(usageContext, checker.getTypeAtLocation(nodeWithRealType)); } diff --git a/tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts b/tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts new file mode 100644 index 00000000000..fb394c3507a --- /dev/null +++ b/tests/cases/fourslash/codeFixInferFromUsageJSDestructuring.ts @@ -0,0 +1,18 @@ +/// +// @allowJs: true +// @checkJs: true +// @noEmit: true +// @filename:destruct.js +//// function [|formatter|](message) { +//// const { type } = false ? { type: message } : message; +//// } +verify.codeFix({ + description: "Infer parameter types from usage", + index: 0, + newFileContent: `/** + * @param {{ type: any; }} message + */ +function formatter(message) { + const { type } = false ? { type: message } : message; +}` +}); diff --git a/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts b/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts index 250673d91fc..3200fffafb8 100644 --- a/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts +++ b/tests/cases/fourslash/codeFixInferFromUsageJSXElement.ts @@ -5,9 +5,6 @@ // @module: es2015 // @moduleResolution: node -// @Filename: /node_modules/@types/react/index.d.ts -////export = React; -////export as namespace React; ////declare namespace React { //// export class Component { render(): JSX.Element | null; } ////} @@ -16,10 +13,6 @@ //// interface Element {} //// } ////} - - -// @filename: a.tsx -//// import React from 'react'; //// //// export default function Component([|props |]) { //// if (props.isLoading) { @@ -37,4 +30,4 @@ //// } -verify.rangeAfterCodeFix("props: { isLoading: any; update: (arg0: any) => void; }",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0); \ No newline at end of file +verify.rangeAfterCodeFix("props: { isLoading: any; update: (arg0: any) => void; }",/*includeWhiteSpace*/ undefined, /*errorCode*/ undefined, 0); From e9073a863d1d99d292a80028a71fe008dbe17808 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 26 Aug 2019 13:20:42 -0700 Subject: [PATCH 11/13] Improve names in infer-from-usage (#33090) Basically, drop "Context" from all names, because it just indicates that it's an implementation of the State monad. --- src/services/codefixes/inferFromUsage.ts | 238 +++++++++++------------ 1 file changed, 119 insertions(+), 119 deletions(-) diff --git a/src/services/codefixes/inferFromUsage.ts b/src/services/codefixes/inferFromUsage.ts index 396d7f44fa8..417ed5c625c 100644 --- a/src/services/codefixes/inferFromUsage.ts +++ b/src/services/codefixes/inferFromUsage.ts @@ -359,7 +359,7 @@ namespace ts.codefix { const references = getReferences(token, program, cancellationToken); const checker = program.getTypeChecker(); const types = InferFromReference.inferTypesFromReferences(references, checker, cancellationToken); - return InferFromReference.unifyFromContext(types, checker); + return InferFromReference.unifyFromUsage(types, checker); } function inferFunctionReferencesFromUsage(containingFunction: FunctionLike, sourceFile: SourceFile, program: Program, cancellationToken: CancellationToken): ReadonlyArray | undefined { @@ -395,33 +395,33 @@ namespace ts.codefix { } namespace InferFromReference { - interface CallContext { + interface CallUsage { argumentTypes: Type[]; - returnType: UsageContext; + returnType: Usage; } - interface UsageContext { + interface Usage { isNumber?: boolean; isString?: boolean; /** Used ambiguously, eg x + ___ or object[___]; results in string | number if no other evidence exists */ isNumberOrString?: boolean; candidateTypes?: Type[]; - properties?: UnderscoreEscapedMap; - callContexts?: CallContext[]; - constructContexts?: CallContext[]; - numberIndexContext?: UsageContext; - stringIndexContext?: UsageContext; + properties?: UnderscoreEscapedMap; + calls?: CallUsage[]; + constructs?: CallUsage[]; + numberIndex?: Usage; + stringIndex?: Usage; candidateThisTypes?: Type[]; } export function inferTypesFromReferences(references: ReadonlyArray, checker: TypeChecker, cancellationToken: CancellationToken): Type[] { - const usageContext: UsageContext = {}; + const usage: Usage = {}; for (const reference of references) { cancellationToken.throwIfCancellationRequested(); - inferTypeFromContext(reference, checker, usageContext); + calculateUsageOfNode(reference, checker, usage); } - return inferFromContext(usageContext, checker); + return inferFromUsage(usage, checker); } export function inferTypeForParametersFromReferences(references: ReadonlyArray | undefined, declaration: FunctionLike, program: Program, cancellationToken: CancellationToken): ParameterInference[] | undefined { @@ -430,35 +430,35 @@ namespace ts.codefix { } const checker = program.getTypeChecker(); - const usageContext: UsageContext = {}; + const usage: Usage = {}; for (const reference of references) { cancellationToken.throwIfCancellationRequested(); - inferTypeFromContext(reference, checker, usageContext); + calculateUsageOfNode(reference, checker, usage); } - const callContexts = [...usageContext.constructContexts || [], ...usageContext.callContexts || []]; + const calls = [...usage.constructs || [], ...usage.calls || []]; return declaration.parameters.map((parameter, parameterIndex): ParameterInference => { const types = []; const isRest = isRestParameter(parameter); let isOptional = false; - for (const callContext of callContexts) { - if (callContext.argumentTypes.length <= parameterIndex) { + for (const call of calls) { + if (call.argumentTypes.length <= parameterIndex) { isOptional = isInJSFile(declaration); types.push(checker.getUndefinedType()); } else if (isRest) { - for (let i = parameterIndex; i < callContext.argumentTypes.length; i++) { - types.push(checker.getBaseTypeOfLiteralType(callContext.argumentTypes[i])); + for (let i = parameterIndex; i < call.argumentTypes.length; i++) { + types.push(checker.getBaseTypeOfLiteralType(call.argumentTypes[i])); } } else { - types.push(checker.getBaseTypeOfLiteralType(callContext.argumentTypes[parameterIndex])); + types.push(checker.getBaseTypeOfLiteralType(call.argumentTypes[parameterIndex])); } } if (isIdentifier(parameter.name)) { const inferred = inferTypesFromReferences(getReferences(parameter.name, program, cancellationToken), checker, cancellationToken); types.push(...(isRest ? mapDefined(inferred, checker.getElementTypeOfArrayType) : inferred)); } - const type = unifyFromContext(types, checker); + const type = unifyFromUsage(types, checker); return { type: isRest ? checker.createArrayType(type) : type, isOptional: isOptional && !isRest, @@ -473,89 +473,89 @@ namespace ts.codefix { } const checker = program.getTypeChecker(); - const usageContext: UsageContext = {}; + const usage: Usage = {}; for (const reference of references) { cancellationToken.throwIfCancellationRequested(); - inferTypeFromContext(reference, checker, usageContext); + calculateUsageOfNode(reference, checker, usage); } - return unifyFromContext(usageContext.candidateThisTypes || emptyArray, checker); + return unifyFromUsage(usage.candidateThisTypes || emptyArray, checker); } - function inferTypeFromContext(node: Expression, checker: TypeChecker, usageContext: UsageContext): void { + function calculateUsageOfNode(node: Expression, checker: TypeChecker, usage: Usage): void { while (isRightSideOfQualifiedNameOrPropertyAccess(node)) { node = node.parent; } switch (node.parent.kind) { case SyntaxKind.PostfixUnaryExpression: - usageContext.isNumber = true; + usage.isNumber = true; break; case SyntaxKind.PrefixUnaryExpression: - inferTypeFromPrefixUnaryExpressionContext(node.parent, usageContext); + inferTypeFromPrefixUnaryExpression(node.parent, usage); break; case SyntaxKind.BinaryExpression: - inferTypeFromBinaryExpressionContext(node, node.parent, checker, usageContext); + inferTypeFromBinaryExpression(node, node.parent, checker, usage); break; case SyntaxKind.CaseClause: case SyntaxKind.DefaultClause: - inferTypeFromSwitchStatementLabelContext(node.parent, checker, usageContext); + inferTypeFromSwitchStatementLabel(node.parent, checker, usage); break; case SyntaxKind.CallExpression: case SyntaxKind.NewExpression: if ((node.parent).expression === node) { - inferTypeFromCallExpressionContext(node.parent, checker, usageContext); + inferTypeFromCallExpression(node.parent, checker, usage); } else { - inferTypeFromContextualType(node, checker, usageContext); + inferTypeFromContextualType(node, checker, usage); } break; case SyntaxKind.PropertyAccessExpression: - inferTypeFromPropertyAccessExpressionContext(node.parent, checker, usageContext); + inferTypeFromPropertyAccessExpression(node.parent, checker, usage); break; case SyntaxKind.ElementAccessExpression: - inferTypeFromPropertyElementExpressionContext(node.parent, node, checker, usageContext); + inferTypeFromPropertyElementExpression(node.parent, node, checker, usage); break; case SyntaxKind.PropertyAssignment: case SyntaxKind.ShorthandPropertyAssignment: - inferTypeFromPropertyAssignment(node.parent, checker, usageContext); + inferTypeFromPropertyAssignment(node.parent, checker, usage); break; case SyntaxKind.PropertyDeclaration: - inferTypeFromPropertyDeclaration(node.parent, checker, usageContext); + inferTypeFromPropertyDeclaration(node.parent, checker, usage); break; case SyntaxKind.VariableDeclaration: { const { name, initializer } = node.parent as VariableDeclaration; if (node === name) { if (initializer) { // This can happen for `let x = null;` which still has an implicit-any error. - addCandidateType(usageContext, checker.getTypeAtLocation(initializer)); + addCandidateType(usage, checker.getTypeAtLocation(initializer)); } break; } } // falls through default: - return inferTypeFromContextualType(node, checker, usageContext); + return inferTypeFromContextualType(node, checker, usage); } } - function inferTypeFromContextualType(node: Expression, checker: TypeChecker, usageContext: UsageContext): void { + function inferTypeFromContextualType(node: Expression, checker: TypeChecker, usage: Usage): void { if (isExpressionNode(node)) { - addCandidateType(usageContext, checker.getContextualType(node)); + addCandidateType(usage, checker.getContextualType(node)); } } - function inferTypeFromPrefixUnaryExpressionContext(node: PrefixUnaryExpression, usageContext: UsageContext): void { + function inferTypeFromPrefixUnaryExpression(node: PrefixUnaryExpression, usage: Usage): void { switch (node.operator) { case SyntaxKind.PlusPlusToken: case SyntaxKind.MinusMinusToken: case SyntaxKind.MinusToken: case SyntaxKind.TildeToken: - usageContext.isNumber = true; + usage.isNumber = true; break; case SyntaxKind.PlusToken: - usageContext.isNumberOrString = true; + usage.isNumberOrString = true; break; // case SyntaxKind.ExclamationToken: @@ -563,7 +563,7 @@ namespace ts.codefix { } } - function inferTypeFromBinaryExpressionContext(node: Expression, parent: BinaryExpression, checker: TypeChecker, usageContext: UsageContext): void { + function inferTypeFromBinaryExpression(node: Expression, parent: BinaryExpression, checker: TypeChecker, usage: Usage): void { switch (parent.operatorToken.kind) { // ExponentiationOperator case SyntaxKind.AsteriskAsteriskToken: @@ -606,10 +606,10 @@ namespace ts.codefix { case SyntaxKind.GreaterThanEqualsToken: const operandType = checker.getTypeAtLocation(parent.left === node ? parent.right : parent.left); if (operandType.flags & TypeFlags.EnumLike) { - addCandidateType(usageContext, operandType); + addCandidateType(usage, operandType); } else { - usageContext.isNumber = true; + usage.isNumber = true; } break; @@ -617,16 +617,16 @@ namespace ts.codefix { case SyntaxKind.PlusToken: const otherOperandType = checker.getTypeAtLocation(parent.left === node ? parent.right : parent.left); if (otherOperandType.flags & TypeFlags.EnumLike) { - addCandidateType(usageContext, otherOperandType); + addCandidateType(usage, otherOperandType); } else if (otherOperandType.flags & TypeFlags.NumberLike) { - usageContext.isNumber = true; + usage.isNumber = true; } else if (otherOperandType.flags & TypeFlags.StringLike) { - usageContext.isString = true; + usage.isString = true; } else { - usageContext.isNumberOrString = true; + usage.isNumberOrString = true; } break; @@ -636,12 +636,12 @@ namespace ts.codefix { case SyntaxKind.EqualsEqualsEqualsToken: case SyntaxKind.ExclamationEqualsEqualsToken: case SyntaxKind.ExclamationEqualsToken: - addCandidateType(usageContext, checker.getTypeAtLocation(parent.left === node ? parent.right : parent.left)); + addCandidateType(usage, checker.getTypeAtLocation(parent.left === node ? parent.right : parent.left)); break; case SyntaxKind.InKeyword: if (node === parent.left) { - usageContext.isString = true; + usage.isString = true; } break; @@ -651,7 +651,7 @@ namespace ts.codefix { (node.parent.parent.kind === SyntaxKind.VariableDeclaration || isAssignmentExpression(node.parent.parent, /*excludeCompoundAssignment*/ true))) { // var x = x || {}; // TODO: use getFalsyflagsOfType - addCandidateType(usageContext, checker.getTypeAtLocation(parent.right)); + addCandidateType(usage, checker.getTypeAtLocation(parent.right)); } break; @@ -663,68 +663,68 @@ namespace ts.codefix { } } - function inferTypeFromSwitchStatementLabelContext(parent: CaseOrDefaultClause, checker: TypeChecker, usageContext: UsageContext): void { - addCandidateType(usageContext, checker.getTypeAtLocation(parent.parent.parent.expression)); + function inferTypeFromSwitchStatementLabel(parent: CaseOrDefaultClause, checker: TypeChecker, usage: Usage): void { + addCandidateType(usage, checker.getTypeAtLocation(parent.parent.parent.expression)); } - function inferTypeFromCallExpressionContext(parent: CallExpression | NewExpression, checker: TypeChecker, usageContext: UsageContext): void { - const callContext: CallContext = { + function inferTypeFromCallExpression(parent: CallExpression | NewExpression, checker: TypeChecker, usage: Usage): void { + const call: CallUsage = { argumentTypes: [], returnType: {} }; if (parent.arguments) { for (const argument of parent.arguments) { - callContext.argumentTypes.push(checker.getTypeAtLocation(argument)); + call.argumentTypes.push(checker.getTypeAtLocation(argument)); } } - inferTypeFromContext(parent, checker, callContext.returnType); + calculateUsageOfNode(parent, checker, call.returnType); if (parent.kind === SyntaxKind.CallExpression) { - (usageContext.callContexts || (usageContext.callContexts = [])).push(callContext); + (usage.calls || (usage.calls = [])).push(call); } else { - (usageContext.constructContexts || (usageContext.constructContexts = [])).push(callContext); + (usage.constructs || (usage.constructs = [])).push(call); } } - function inferTypeFromPropertyAccessExpressionContext(parent: PropertyAccessExpression, checker: TypeChecker, usageContext: UsageContext): void { + function inferTypeFromPropertyAccessExpression(parent: PropertyAccessExpression, checker: TypeChecker, usage: Usage): void { const name = escapeLeadingUnderscores(parent.name.text); - if (!usageContext.properties) { - usageContext.properties = createUnderscoreEscapedMap(); + if (!usage.properties) { + usage.properties = createUnderscoreEscapedMap(); } - const propertyUsageContext = usageContext.properties.get(name) || { }; - inferTypeFromContext(parent, checker, propertyUsageContext); - usageContext.properties.set(name, propertyUsageContext); + const propertyUsage = usage.properties.get(name) || { }; + calculateUsageOfNode(parent, checker, propertyUsage); + usage.properties.set(name, propertyUsage); } - function inferTypeFromPropertyElementExpressionContext(parent: ElementAccessExpression, node: Expression, checker: TypeChecker, usageContext: UsageContext): void { + function inferTypeFromPropertyElementExpression(parent: ElementAccessExpression, node: Expression, checker: TypeChecker, usage: Usage): void { if (node === parent.argumentExpression) { - usageContext.isNumberOrString = true; + usage.isNumberOrString = true; return; } else { const indexType = checker.getTypeAtLocation(parent.argumentExpression); - const indexUsageContext = {}; - inferTypeFromContext(parent, checker, indexUsageContext); + const indexUsage = {}; + calculateUsageOfNode(parent, checker, indexUsage); if (indexType.flags & TypeFlags.NumberLike) { - usageContext.numberIndexContext = indexUsageContext; + usage.numberIndex = indexUsage; } else { - usageContext.stringIndexContext = indexUsageContext; + usage.stringIndex = indexUsage; } } } - function inferTypeFromPropertyAssignment(assignment: PropertyAssignment | ShorthandPropertyAssignment, checker: TypeChecker, usageContext: UsageContext) { + function inferTypeFromPropertyAssignment(assignment: PropertyAssignment | ShorthandPropertyAssignment, checker: TypeChecker, usage: Usage) { const nodeWithRealType = isVariableDeclaration(assignment.parent.parent) ? assignment.parent.parent : assignment.parent; - addCandidateThisType(usageContext, checker.getTypeAtLocation(nodeWithRealType)); + addCandidateThisType(usage, checker.getTypeAtLocation(nodeWithRealType)); } - function inferTypeFromPropertyDeclaration(declaration: PropertyDeclaration, checker: TypeChecker, usageContext: UsageContext) { - addCandidateThisType(usageContext, checker.getTypeAtLocation(declaration.parent)); + function inferTypeFromPropertyDeclaration(declaration: PropertyDeclaration, checker: TypeChecker, usage: Usage) { + addCandidateThisType(usage, checker.getTypeAtLocation(declaration.parent)); } interface Priority { @@ -745,7 +745,7 @@ namespace ts.codefix { return inferences.filter(i => toRemove.every(f => !f(i))); } - export function unifyFromContext(inferences: ReadonlyArray, checker: TypeChecker, fallback = checker.getAnyType()): Type { + export function unifyFromUsage(inferences: ReadonlyArray, checker: TypeChecker, fallback = checker.getAnyType()): Type { if (!inferences.length) return fallback; // 1. string or number individually override string | number @@ -815,82 +815,82 @@ namespace ts.codefix { numberIndices.length ? checker.createIndexInfo(checker.getUnionType(numberIndices), numberIndexReadonly) : undefined); } - function inferFromContext(usageContext: UsageContext, checker: TypeChecker) { + function inferFromUsage(usage: Usage, checker: TypeChecker) { const types = []; - if (usageContext.isNumber) { + if (usage.isNumber) { types.push(checker.getNumberType()); } - if (usageContext.isString) { + if (usage.isString) { types.push(checker.getStringType()); } - if (usageContext.isNumberOrString) { + if (usage.isNumberOrString) { types.push(checker.getUnionType([checker.getStringType(), checker.getNumberType()])); } - types.push(...(usageContext.candidateTypes || []).map(t => checker.getBaseTypeOfLiteralType(t))); + types.push(...(usage.candidateTypes || []).map(t => checker.getBaseTypeOfLiteralType(t))); - if (usageContext.properties && hasCallContext(usageContext.properties.get("then" as __String))) { - const paramType = getParameterTypeFromCallContexts(0, usageContext.properties.get("then" as __String)!.callContexts!, /*isRestParameter*/ false, checker)!; // TODO: GH#18217 + if (usage.properties && hasCalls(usage.properties.get("then" as __String))) { + const paramType = getParameterTypeFromCalls(0, usage.properties.get("then" as __String)!.calls!, /*isRestParameter*/ false, checker)!; // TODO: GH#18217 const types = paramType.getCallSignatures().map(c => c.getReturnType()); types.push(checker.createPromiseType(types.length ? checker.getUnionType(types, UnionReduction.Subtype) : checker.getAnyType())); } - else if (usageContext.properties && hasCallContext(usageContext.properties.get("push" as __String))) { - types.push(checker.createArrayType(getParameterTypeFromCallContexts(0, usageContext.properties.get("push" as __String)!.callContexts!, /*isRestParameter*/ false, checker)!)); + else if (usage.properties && hasCalls(usage.properties.get("push" as __String))) { + types.push(checker.createArrayType(getParameterTypeFromCalls(0, usage.properties.get("push" as __String)!.calls!, /*isRestParameter*/ false, checker)!)); } - if (usageContext.numberIndexContext) { - types.push(checker.createArrayType(recur(usageContext.numberIndexContext))); + if (usage.numberIndex) { + types.push(checker.createArrayType(recur(usage.numberIndex))); } - else if (usageContext.properties || usageContext.callContexts || usageContext.constructContexts || usageContext.stringIndexContext) { + else if (usage.properties || usage.calls || usage.constructs || usage.stringIndex) { const members = createUnderscoreEscapedMap(); const callSignatures: Signature[] = []; const constructSignatures: Signature[] = []; let stringIndexInfo: IndexInfo | undefined; - if (usageContext.properties) { - usageContext.properties.forEach((context, name) => { + if (usage.properties) { + usage.properties.forEach((u, name) => { const symbol = checker.createSymbol(SymbolFlags.Property, name); - symbol.type = recur(context); + symbol.type = recur(u); members.set(name, symbol); }); } - if (usageContext.callContexts) { - for (const callContext of usageContext.callContexts) { - callSignatures.push(getSignatureFromCallContext(callContext, checker)); + if (usage.calls) { + for (const call of usage.calls) { + callSignatures.push(getSignatureFromCall(call, checker)); } } - if (usageContext.constructContexts) { - for (const constructContext of usageContext.constructContexts) { - constructSignatures.push(getSignatureFromCallContext(constructContext, checker)); + if (usage.constructs) { + for (const construct of usage.constructs) { + constructSignatures.push(getSignatureFromCall(construct, checker)); } } - if (usageContext.stringIndexContext) { - stringIndexInfo = checker.createIndexInfo(recur(usageContext.stringIndexContext), /*isReadonly*/ false); + if (usage.stringIndex) { + stringIndexInfo = checker.createIndexInfo(recur(usage.stringIndex), /*isReadonly*/ false); } types.push(checker.createAnonymousType(/*symbol*/ undefined!, members, callSignatures, constructSignatures, stringIndexInfo, /*numberIndexInfo*/ undefined)); // TODO: GH#18217 } return types; - function recur(innerContext: UsageContext): Type { - return unifyFromContext(inferFromContext(innerContext, checker), checker); + function recur(innerUsage: Usage): Type { + return unifyFromUsage(inferFromUsage(innerUsage, checker), checker); } } - function getParameterTypeFromCallContexts(parameterIndex: number, callContexts: CallContext[], isRestParameter: boolean, checker: TypeChecker) { + function getParameterTypeFromCalls(parameterIndex: number, calls: CallUsage[], isRestParameter: boolean, checker: TypeChecker) { let types: Type[] = []; - if (callContexts) { - for (const callContext of callContexts) { - if (callContext.argumentTypes.length > parameterIndex) { + if (calls) { + for (const call of calls) { + if (call.argumentTypes.length > parameterIndex) { if (isRestParameter) { - types = concatenate(types, map(callContext.argumentTypes.slice(parameterIndex), a => checker.getBaseTypeOfLiteralType(a))); + types = concatenate(types, map(call.argumentTypes.slice(parameterIndex), a => checker.getBaseTypeOfLiteralType(a))); } else { - types.push(checker.getBaseTypeOfLiteralType(callContext.argumentTypes[parameterIndex])); + types.push(checker.getBaseTypeOfLiteralType(call.argumentTypes[parameterIndex])); } } } @@ -903,32 +903,32 @@ namespace ts.codefix { return undefined; } - function getSignatureFromCallContext(callContext: CallContext, checker: TypeChecker): Signature { + function getSignatureFromCall(call: CallUsage, checker: TypeChecker): Signature { const parameters: Symbol[] = []; - for (let i = 0; i < callContext.argumentTypes.length; i++) { + for (let i = 0; i < call.argumentTypes.length; i++) { const symbol = checker.createSymbol(SymbolFlags.FunctionScopedVariable, escapeLeadingUnderscores(`arg${i}`)); - symbol.type = checker.getWidenedType(checker.getBaseTypeOfLiteralType(callContext.argumentTypes[i])); + symbol.type = checker.getWidenedType(checker.getBaseTypeOfLiteralType(call.argumentTypes[i])); parameters.push(symbol); } - const returnType = unifyFromContext(inferFromContext(callContext.returnType, checker), checker, checker.getVoidType()); + const returnType = unifyFromUsage(inferFromUsage(call.returnType, checker), checker, checker.getVoidType()); // TODO: GH#18217 - return checker.createSignature(/*declaration*/ undefined!, /*typeParameters*/ undefined, /*thisParameter*/ undefined, parameters, returnType, /*typePredicate*/ undefined, callContext.argumentTypes.length, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); + return checker.createSignature(/*declaration*/ undefined!, /*typeParameters*/ undefined, /*thisParameter*/ undefined, parameters, returnType, /*typePredicate*/ undefined, call.argumentTypes.length, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false); } - function addCandidateType(context: UsageContext, type: Type | undefined) { + function addCandidateType(usage: Usage, type: Type | undefined) { if (type && !(type.flags & TypeFlags.Any) && !(type.flags & TypeFlags.Never)) { - (context.candidateTypes || (context.candidateTypes = [])).push(type); + (usage.candidateTypes || (usage.candidateTypes = [])).push(type); } } - function addCandidateThisType(context: UsageContext, type: Type | undefined) { + function addCandidateThisType(usage: Usage, type: Type | undefined) { if (type && !(type.flags & TypeFlags.Any) && !(type.flags & TypeFlags.Never)) { - (context.candidateThisTypes || (context.candidateThisTypes = [])).push(type); + (usage.candidateThisTypes || (usage.candidateThisTypes = [])).push(type); } } - function hasCallContext(usageContext: UsageContext | undefined): boolean { - return !!usageContext && !!usageContext.callContexts; + function hasCalls(usage: Usage | undefined): boolean { + return !!usage && !!usage.calls; } } } From 9942c6052fc56e050414198a5aaadf2a96665f6d Mon Sep 17 00:00:00 2001 From: Wenlu Wang Date: Tue, 27 Aug 2019 06:26:25 +0800 Subject: [PATCH 12/13] add completion for promise context (#32101) * add completion for promise context * check insert text inside add symbol helper * fix incorrect branch * avoid completions with includeCompletionsWithInsertText perferences * avoid useless parameter --- src/services/completions.ts | 45 +++++++++++++++---- .../fourslash/completionOfAwaitPromise1.ts | 17 +++++++ .../fourslash/completionOfAwaitPromise2.ts | 18 ++++++++ .../fourslash/completionOfAwaitPromise3.ts | 18 ++++++++ .../fourslash/completionOfAwaitPromise4.ts | 15 +++++++ .../fourslash/completionOfAwaitPromise5.ts | 18 ++++++++ .../fourslash/completionOfAwaitPromise6.ts | 17 +++++++ 7 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/completionOfAwaitPromise1.ts create mode 100644 tests/cases/fourslash/completionOfAwaitPromise2.ts create mode 100644 tests/cases/fourslash/completionOfAwaitPromise3.ts create mode 100644 tests/cases/fourslash/completionOfAwaitPromise4.ts create mode 100644 tests/cases/fourslash/completionOfAwaitPromise5.ts create mode 100644 tests/cases/fourslash/completionOfAwaitPromise6.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index c7ea7b5a40e..f574bb2cab2 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -9,8 +9,8 @@ namespace ts.Completions { } export type Log = (message: string) => void; - const enum SymbolOriginInfoKind { ThisType, SymbolMemberNoExport, SymbolMemberExport, Export } - type SymbolOriginInfo = { kind: SymbolOriginInfoKind.ThisType } | { kind: SymbolOriginInfoKind.SymbolMemberNoExport } | SymbolOriginInfoExport; + const enum SymbolOriginInfoKind { ThisType, SymbolMemberNoExport, SymbolMemberExport, Export, Promise } + type SymbolOriginInfo = { kind: SymbolOriginInfoKind.ThisType } | { kind: SymbolOriginInfoKind.Promise } | { kind: SymbolOriginInfoKind.SymbolMemberNoExport } | SymbolOriginInfoExport; interface SymbolOriginInfoExport { kind: SymbolOriginInfoKind.SymbolMemberExport | SymbolOriginInfoKind.Export; moduleSymbol: Symbol; @@ -22,6 +22,9 @@ namespace ts.Completions { function originIsExport(origin: SymbolOriginInfo): origin is SymbolOriginInfoExport { return origin.kind === SymbolOriginInfoKind.SymbolMemberExport || origin.kind === SymbolOriginInfoKind.Export; } + function originIsPromise(origin: SymbolOriginInfo): boolean { + return origin.kind === SymbolOriginInfoKind.Promise; + } /** * Map from symbol id -> SymbolOriginInfo. @@ -264,6 +267,12 @@ namespace ts.Completions { replacementSpan = createTextSpanFromNode(isJsxInitializer, sourceFile); } } + if (origin && originIsPromise(origin) && propertyAccessToConvert) { + if (insertText === undefined) insertText = name; + const awaitText = `(await ${propertyAccessToConvert.expression.getText()})`; + insertText = needsConvertPropertyAccess ? `${awaitText}${insertText}` : `${awaitText}.${insertText}`; + replacementSpan = createTextSpanFromBounds(propertyAccessToConvert.getStart(sourceFile), propertyAccessToConvert.end); + } if (insertText !== undefined && !preferences.includeCompletionsWithInsertText) { return undefined; @@ -313,7 +322,7 @@ namespace ts.Completions { log: Log, kind: CompletionKind, preferences: UserPreferences, - propertyAccessToConvert?: PropertyAccessExpression | undefined, + propertyAccessToConvert?: PropertyAccessExpression, isJsxInitializer?: IsJsxInitializer, recommendedCompletion?: Symbol, symbolToOriginInfoMap?: SymbolOriginInfoMap, @@ -984,7 +993,7 @@ namespace ts.Completions { if (!isTypeLocation && symbol.declarations && symbol.declarations.some(d => d.kind !== SyntaxKind.SourceFile && d.kind !== SyntaxKind.ModuleDeclaration && d.kind !== SyntaxKind.EnumDeclaration)) { - addTypeProperties(typeChecker.getTypeOfSymbolAtLocation(symbol, node)); + addTypeProperties(typeChecker.getTypeOfSymbolAtLocation(symbol, node), !!(node.flags & NodeFlags.AwaitContext)); } return; @@ -999,13 +1008,14 @@ namespace ts.Completions { } if (!isTypeLocation) { - addTypeProperties(typeChecker.getTypeAtLocation(node)); + addTypeProperties(typeChecker.getTypeAtLocation(node), !!(node.flags & NodeFlags.AwaitContext)); } } - function addTypeProperties(type: Type): void { + function addTypeProperties(type: Type, insertAwait?: boolean): void { isNewIdentifierLocation = !!type.getStringIndexType(); + const propertyAccess = node.kind === SyntaxKind.ImportType ? node : node.parent; if (isUncheckedFile) { // In javascript files, for union types, we don't just get the members that // the individual types have in common, we also include all the members that @@ -1016,14 +1026,25 @@ namespace ts.Completions { } else { for (const symbol of type.getApparentProperties()) { - if (typeChecker.isValidPropertyAccessForCompletions(node.kind === SyntaxKind.ImportType ? node : node.parent, type, symbol)) { + if (typeChecker.isValidPropertyAccessForCompletions(propertyAccess, type, symbol)) { addPropertySymbol(symbol); } } } + + if (insertAwait && preferences.includeCompletionsWithInsertText) { + const promiseType = typeChecker.getPromisedTypeOfPromise(type); + if (promiseType) { + for (const symbol of promiseType.getApparentProperties()) { + if (typeChecker.isValidPropertyAccessForCompletions(propertyAccess, promiseType, symbol)) { + addPropertySymbol(symbol, /* insertAwait */ true); + } + } + } + } } - function addPropertySymbol(symbol: Symbol) { + function addPropertySymbol(symbol: Symbol, insertAwait?: boolean) { // For a computed property with an accessible name like `Symbol.iterator`, // we'll add a completion for the *name* `Symbol` instead of for the property. // If this is e.g. [Symbol.iterator], add a completion for `Symbol`. @@ -1040,12 +1061,20 @@ namespace ts.Completions { !moduleSymbol || !isExternalModuleSymbol(moduleSymbol) ? { kind: SymbolOriginInfoKind.SymbolMemberNoExport } : { kind: SymbolOriginInfoKind.SymbolMemberExport, moduleSymbol, isDefaultExport: false }; } else if (preferences.includeCompletionsWithInsertText) { + addPromiseSymbolOriginInfo(symbol); symbols.push(symbol); } } else { + addPromiseSymbolOriginInfo(symbol); symbols.push(symbol); } + + function addPromiseSymbolOriginInfo (symbol: Symbol) { + if (insertAwait && preferences.includeCompletionsWithInsertText && !symbolToOriginInfoMap[getSymbolId(symbol)]) { + symbolToOriginInfoMap[getSymbolId(symbol)] = { kind: SymbolOriginInfoKind.Promise }; + } + } } /** Given 'a.b.c', returns 'a'. */ diff --git a/tests/cases/fourslash/completionOfAwaitPromise1.ts b/tests/cases/fourslash/completionOfAwaitPromise1.ts new file mode 100644 index 00000000000..0c72b120211 --- /dev/null +++ b/tests/cases/fourslash/completionOfAwaitPromise1.ts @@ -0,0 +1,17 @@ +/// + +//// async function foo(x: Promise) { +//// [|x./**/|] +//// } + +const replacementSpan = test.ranges()[0] +verify.completions({ + marker: "", + includes: [ + "then", + { name: "trim", insertText: '(await x).trim', replacementSpan }, + ], + preferences: { + includeInsertTextCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionOfAwaitPromise2.ts b/tests/cases/fourslash/completionOfAwaitPromise2.ts new file mode 100644 index 00000000000..fab4ff9020a --- /dev/null +++ b/tests/cases/fourslash/completionOfAwaitPromise2.ts @@ -0,0 +1,18 @@ +/// + +//// interface Foo { foo: string } +//// async function foo(x: Promise) { +//// [|x./**/|] +//// } + +const replacementSpan = test.ranges()[0] +verify.completions({ + marker: "", + includes: [ + "then", + { name: "foo", insertText: '(await x).foo', replacementSpan }, + ], + preferences: { + includeInsertTextCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionOfAwaitPromise3.ts b/tests/cases/fourslash/completionOfAwaitPromise3.ts new file mode 100644 index 00000000000..aec39eec6ac --- /dev/null +++ b/tests/cases/fourslash/completionOfAwaitPromise3.ts @@ -0,0 +1,18 @@ +/// + +//// interface Foo { ["foo-foo"]: string } +//// async function foo(x: Promise) { +//// [|x./**/|] +//// } + +const replacementSpan = test.ranges()[0] +verify.completions({ + marker: "", + includes: [ + "then", + { name: "foo-foo", insertText: '(await x)["foo-foo"]', replacementSpan, }, + ], + preferences: { + includeInsertTextCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionOfAwaitPromise4.ts b/tests/cases/fourslash/completionOfAwaitPromise4.ts new file mode 100644 index 00000000000..702efd7090e --- /dev/null +++ b/tests/cases/fourslash/completionOfAwaitPromise4.ts @@ -0,0 +1,15 @@ +/// + +//// function foo(x: Promise) { +//// [|x./**/|] +//// } + +const replacementSpan = test.ranges()[0] +verify.completions({ + marker: "", + includes: ["then"], + excludes: ["trim"], + preferences: { + includeInsertTextCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionOfAwaitPromise5.ts b/tests/cases/fourslash/completionOfAwaitPromise5.ts new file mode 100644 index 00000000000..87a5523fe23 --- /dev/null +++ b/tests/cases/fourslash/completionOfAwaitPromise5.ts @@ -0,0 +1,18 @@ +/// + +//// interface Foo { foo: string } +//// async function foo(x: (a: number) => Promise) { +//// [|x(1)./**/|] +//// } + +const replacementSpan = test.ranges()[0] +verify.completions({ + marker: "", + includes: [ + "then", + { name: "foo", insertText: '(await x(1)).foo', replacementSpan }, + ], + preferences: { + includeInsertTextCompletions: true, + }, +}); diff --git a/tests/cases/fourslash/completionOfAwaitPromise6.ts b/tests/cases/fourslash/completionOfAwaitPromise6.ts new file mode 100644 index 00000000000..c09cffdca09 --- /dev/null +++ b/tests/cases/fourslash/completionOfAwaitPromise6.ts @@ -0,0 +1,17 @@ +/// + +//// async function foo(x: Promise) { +//// [|x./**/|] +//// } + +const replacementSpan = test.ranges()[0] +verify.completions({ + marker: "", + exact: [ + "then", + "catch" + ], + preferences: { + includeInsertTextCompletions: false, + }, +}); From 111b73acf9bc3ff79b5c5185d75585c7c824aad7 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Mon, 26 Aug 2019 15:34:44 -0700 Subject: [PATCH 13/13] Remove unnecessary tslint-ignore (#33091) --- src/compiler/emitter.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 474d0e67932..a47cfcb3c19 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -411,11 +411,7 @@ namespace ts { } ); if (emitOnlyDtsFiles && declarationTransform.transformed[0].kind === SyntaxKind.SourceFile) { - // Improved narrowing in master/3.6 makes this cast unnecessary, triggering a lint rule. - // But at the same time, the LKG (3.5) necessitates it because it doesn’t narrow. - // Once the LKG is updated to 3.6, this comment, the cast to `SourceFile`, and the - // tslint directive can be all be removed. - const sourceFile = declarationTransform.transformed[0] as SourceFile; // tslint:disable-line + const sourceFile = declarationTransform.transformed[0]; exportedModulesFromDeclarationEmit = sourceFile.exportedModulesFromDeclarationEmit; } }