From ba9d8e2e81736854734f07f4441100f29f603bcd Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Thu, 27 Jun 2019 16:30:35 -0700 Subject: [PATCH] Switch DiagnosticMessageChain to be a tree --- src/compiler/builder.ts | 26 ++--------- src/compiler/checker.ts | 5 ++- src/compiler/program.ts | 40 ++++++++--------- src/compiler/types.ts | 2 +- src/compiler/utilities.ts | 91 ++++++++++++++++++++++++++++----------- src/harness/fourslash.ts | 19 +++++--- 6 files changed, 104 insertions(+), 79 deletions(-) diff --git a/src/compiler/builder.ts b/src/compiler/builder.ts index d4c2132d36b..e8224e22866 100644 --- a/src/compiler/builder.ts +++ b/src/compiler/builder.ts @@ -20,7 +20,7 @@ namespace ts { messageText: string; category: DiagnosticCategory; code: number; - next?: ReusableDiagnosticMessageChain; + next?: ReusableDiagnosticMessageChain[]; } export interface ReusableBuilderProgramState extends ReusableBuilderState { @@ -263,20 +263,10 @@ namespace ts { } function convertToDiagnosticRelatedInformation(diagnostic: ReusableDiagnosticRelatedInformation, newProgram: Program): DiagnosticRelatedInformation { - const { file, messageText } = diagnostic; + const { file } = diagnostic; return { ...diagnostic, file: file && newProgram.getSourceFileByPath(file), - messageText: messageText === undefined || isString(messageText) ? - messageText : - convertToDiagnosticMessageChain(messageText, newProgram) - }; - } - - function convertToDiagnosticMessageChain(diagnostic: ReusableDiagnosticMessageChain, newProgram: Program): DiagnosticMessageChain { - return { - ...diagnostic, - next: diagnostic.next && convertToDiagnosticMessageChain(diagnostic.next, newProgram) }; } @@ -685,20 +675,10 @@ namespace ts { } function convertToReusableDiagnosticRelatedInformation(diagnostic: DiagnosticRelatedInformation): ReusableDiagnosticRelatedInformation { - const { file, messageText } = diagnostic; + const { file } = diagnostic; return { ...diagnostic, file: file && file.path, - messageText: messageText === undefined || isString(messageText) ? - messageText : - convertToReusableDiagnosticMessageChain(messageText) - }; - } - - function convertToReusableDiagnosticMessageChain(diagnostic: DiagnosticMessageChain): ReusableDiagnosticMessageChain { - return { - ...diagnostic, - next: diagnostic.next && convertToReusableDiagnosticMessageChain(diagnostic.next) }; } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f38bcb88e1d..75e477f55da 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12460,7 +12460,8 @@ namespace ts { if (containingMessageChain) { const chain = containingMessageChain(); if (chain) { - errorInfo = concatenateDiagnosticMessageChains(chain, errorInfo); + concatenateDiagnosticMessageChains(chain, errorInfo); + errorInfo = chain; } } @@ -21655,7 +21656,7 @@ namespace ts { } const related = max > 1 ? allDiagnostics[minIndex] : flatten(allDiagnostics); - diagnostics.add(createDiagnosticForNodeFromMessageChain(node, chainDiagnosticMessages(/*details*/ undefined, Diagnostics.No_overload_matches_this_call), related)); + diagnostics.add(createDiagnosticForNodeFromMessageChain(node, chainDiagnosticMessages(undefined, Diagnostics.No_overload_matches_this_call), related)); } } else if (candidateForArgumentArityError) { diff --git a/src/compiler/program.ts b/src/compiler/program.ts index f5ede694f8c..b409370d018 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -502,30 +502,30 @@ namespace ts { return output; } - export function flattenDiagnosticMessageText(messageText: string | DiagnosticMessageChain | undefined, newLine: string): string { - if (isString(messageText)) { - return messageText; + export function flattenDiagnosticMessageText(diag: string | DiagnosticMessageChain | undefined, newLine: string, indent = 0): string { + if (isString(diag)) { + return diag; } - else { - let diagnosticChain = messageText; - let result = ""; + else if (diag === undefined) { + return ""; + } + let result = ""; + if (indent) { + result += newLine; - let indent = 0; - while (diagnosticChain) { - if (indent) { - result += newLine; - - for (let i = 0; i < indent; i++) { - result += " "; - } - } - result += diagnosticChain.messageText; - indent++; - diagnosticChain = diagnosticChain.next; + for (let i = 0; i < indent; i++) { + result += " "; } - - return result; } + result += diag.messageText; + indent++; + if (diag.next) { + // TODO: Should be possible to optimise the common, non-tree case + for (const kid of diag.next) { + result += flattenDiagnosticMessageText(kid, newLine, indent); + } + } + return result; } /* @internal */ diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 4442f339d83..86b5174278b 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4561,7 +4561,7 @@ namespace ts { messageText: string; category: DiagnosticCategory; code: number; - next?: DiagnosticMessageChain; + next?: DiagnosticMessageChain[]; } export interface Diagnostic extends DiagnosticRelatedInformation { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index ae230d75154..4d7eaf65a24 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -7112,8 +7112,8 @@ namespace ts { }; } - export function chainDiagnosticMessages(details: DiagnosticMessageChain | undefined, message: DiagnosticMessage, ...args: (string | number | undefined)[]): DiagnosticMessageChain; - export function chainDiagnosticMessages(details: DiagnosticMessageChain | undefined, message: DiagnosticMessage): DiagnosticMessageChain { + export function chainDiagnosticMessages(details: DiagnosticMessageChain | DiagnosticMessageChain[] | undefined, message: DiagnosticMessage, ...args: (string | number | undefined)[]): DiagnosticMessageChain; + export function chainDiagnosticMessages(details: DiagnosticMessageChain | DiagnosticMessageChain[] | undefined, message: DiagnosticMessage): DiagnosticMessageChain { let text = getLocaleSpecificMessage(message); if (arguments.length > 2) { @@ -7125,18 +7125,17 @@ namespace ts { category: message.category, code: message.code, - next: details + next: details === undefined || Array.isArray(details) ? details : [details] }; } - export function concatenateDiagnosticMessageChains(headChain: DiagnosticMessageChain, tailChain: DiagnosticMessageChain): DiagnosticMessageChain { + export function concatenateDiagnosticMessageChains(headChain: DiagnosticMessageChain, tailChain: DiagnosticMessageChain): void { let lastChain = headChain; while (lastChain.next) { - lastChain = lastChain.next; + lastChain = lastChain.next[0]; } - lastChain.next = tailChain; - return headChain; + lastChain.next = [tailChain]; } function getDiagnosticFilePath(diagnostic: Diagnostic): string | undefined { @@ -7171,30 +7170,70 @@ namespace ts { return d1.relatedInformation ? Comparison.LessThan : Comparison.GreaterThan; } - function compareMessageText(t1: string | DiagnosticMessageChain, t2: string | DiagnosticMessageChain): Comparison { - let text1: string | DiagnosticMessageChain | undefined = t1; - let text2: string | DiagnosticMessageChain | undefined = t2; - while (text1 && text2) { - // We still have both chains. - const string1 = isString(text1) ? text1 : text1.messageText; - const string2 = isString(text2) ? text2 : text2.messageText; + // function compareMessageText(t1: string | DiagnosticMessageChain[], t2: string | DiagnosticMessageChain[]): Comparison { + // if (typeof t1 === 'string' && typeof t2 === 'string') { + // return compareStringsCaseSensitive(t1, t2) + // } + // else if (Array.isArray(t1) && Array.isArray(t2)) { + // if (t1.length < t2.length) { + // return Comparison.LessThan; + // } + // else if (t1.length > t2.length) { + // return Comparison.GreaterThan; + // } + // else { + // for (let i = 0; i < t1.length; i++) { + // t1[i].messageText + // const res = cmps(t1[i], t2[i]); + // if (res) { + // return res; + // } + // } + // return Comparison.EqualTo; + // } + // } + // else if (typeof t1 === 'string') { + // return Comparison.LessThan; + // } + // else { + // return Comparison.GreaterThan; + // } + // } - const res = compareStringsCaseSensitive(string1, string2); + function compareMessageText(t1: string | DiagnosticMessageChain, t2: string | DiagnosticMessageChain): Comparison { + if (typeof t1 === 'string' && typeof t2 === 'string') { + return compareStringsCaseSensitive(t1, t2); + } + else if (typeof t1 === 'string') { + return Comparison.LessThan; + } + else if (typeof t2 === 'string') { + return Comparison.GreaterThan; + } + let res = compareStringsCaseSensitive(t1.messageText, t2.messageText); + if (res) { + return res; + } + if (!t1.next && !t2.next) { + return Comparison.EqualTo; + } + if (!t1.next) { + return Comparison.LessThan; + } + if (!t2.next) { + return Comparison.GreaterThan; + } + res = compareValues(t1.next.length, t2.next.length); + if (res) { + return res; + } + for (let i = 0; i < t1.next.length; i++) { + res = compareMessageText(t1.next[i], t2.next[i]) if (res) { return res; } - - text1 = isString(text1) ? undefined : text1.next; - text2 = isString(text2) ? undefined : text2.next; } - - if (!text1 && !text2) { - // if the chains are done, then these messages are the same. - return Comparison.EqualTo; - } - - // We still have one chain remaining. The shorter chain should come first. - return text1 ? Comparison.GreaterThan : Comparison.LessThan; + return Comparison.EqualTo; } export function getEmitScriptTarget(compilerOptions: CompilerOptions) { diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index cbe4c3336d7..84fb7f390ba 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1482,13 +1482,7 @@ Actual: ${stringify(fullActual)}`); const diagnostics = ts.getPreEmitDiagnostics(this.languageService.getProgram()!); // TODO: GH#18217 for (const diagnostic of diagnostics) { if (!ts.isString(diagnostic.messageText)) { - let chainedMessage: ts.DiagnosticMessageChain | undefined = diagnostic.messageText; - let indentation = " "; - while (chainedMessage) { - resultString += indentation + chainedMessage.messageText + Harness.IO.newLine(); - chainedMessage = chainedMessage.next; - indentation = indentation + " "; - } + resultString += this.flattenChainedMessage(diagnostic.messageText); } else { resultString += " " + diagnostic.messageText + Harness.IO.newLine(); @@ -1506,6 +1500,17 @@ Actual: ${stringify(fullActual)}`); Harness.Baseline.runBaseline(ts.Debug.assertDefined(this.testData.globalOptions[MetadataOptionNames.baselineFile]), resultString); } + private flattenChainedMessage(diag: ts.DiagnosticMessageChain, indent = " ") { + let result = ""; + result += indent + diag.messageText + Harness.IO.newLine(); + if (diag.next) { + for (const kid of diag.next) { + result += this.flattenChainedMessage(kid, indent + " "); + } + } + return result; + } + public baselineQuickInfo() { const baselineFile = this.getBaselineFileName(); Harness.Baseline.runBaseline(