Switch DiagnosticMessageChain to be a tree

sandersn · sandersn · commit ba9d8e2e8173 · 2019-06-27T16:30:35.000-07:00
diff --git 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
@@ -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
@@ -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 = "";
-
-            let indent = 0;
-            while (diagnosticChain) {
-                if (indent) {
-                    result += newLine;
+        else if (diag === undefined) {
+            return "";
+        }
+        let result = "";
+        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 += "  ";
+            }
+        }
+        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;
         }
+        return result;
     }
 
     /* @internal */
diff --git 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
@@ -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 {
+    //     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;
+    //     }
+    // }
+
     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;
-
-            const res = compareStringsCaseSensitive(string1, string2);
+        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
@@ -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(

Original file line number	Diff line number	Diff line change
`@@ -12460,7 +12460,8 @@ namespace ts {`
`12460`	`12460`	`if (containingMessageChain) {`
`12461`	`12461`	`const chain = containingMessageChain();`
`12462`	`12462`	`if (chain) {`
`12463`		`- errorInfo = concatenateDiagnosticMessageChains(chain, errorInfo);`
	`12463`	`+ concatenateDiagnosticMessageChains(chain, errorInfo);`
	`12464`	`+ errorInfo = chain;`
`12464`	`12465`	`}`
`12465`	`12466`	`}`
`12466`	`12467`
`@@ -21655,7 +21656,7 @@ namespace ts {`
`21655`	`21656`	`}`
`21656`	`21657`
`21657`	`21658`	`const related = max > 1 ? allDiagnostics[minIndex] : flatten(allDiagnostics);`
`21658`		`- diagnostics.add(createDiagnosticForNodeFromMessageChain(node, chainDiagnosticMessages(/details/ undefined, Diagnostics.No_overload_matches_this_call), related));`
	`21659`	`+ diagnostics.add(createDiagnosticForNodeFromMessageChain(node, chainDiagnosticMessages(undefined, Diagnostics.No_overload_matches_this_call), related));`
`21659`	`21660`	`}`
`21660`	`21661`	`}`
`21661`	`21662`	`else if (candidateForArgumentArityError) {`
Original file line number	Diff line number	Diff line change
`@@ -4561,7 +4561,7 @@ namespace ts {`
`4561`	`4561`	`messageText: string;`
`4562`	`4562`	`category: DiagnosticCategory;`
`4563`	`4563`	`code: number;`
`4564`		`- next?: DiagnosticMessageChain;`
	`4564`	`+ next?: DiagnosticMessageChain[];`
`4565`	`4565`	`}`
`4566`	`4566`
`4567`	`4567`	`export interface Diagnostic extends DiagnosticRelatedInformation {`