Skip to content

Commit

Permalink
Improve diagnostics deduplication 2 (#58318)
Browse files Browse the repository at this point in the history
  • Loading branch information
gabritto committed Apr 29, 2024
1 parent 3358157 commit d2ad3ca
Show file tree
Hide file tree
Showing 15 changed files with 310 additions and 27 deletions.
22 changes: 20 additions & 2 deletions src/compiler/checker.ts
Expand Up @@ -251,6 +251,7 @@ import {
getAssignmentDeclarationKind,
getAssignmentDeclarationPropertyAccessKind,
getAssignmentTargetKind,
getCanonicalDiagnostic,
getCheckFlags,
getClassExtendsHeritageElement,
getClassLikeDeclarationOfSymbol,
Expand Down Expand Up @@ -3118,10 +3119,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (suggestion) {
const suggestionName = symbolToString(suggestion);
const isUncheckedJS = isUncheckedJSSuggestion(errorLocation, suggestion, /*excludeClasses*/ false);
const message = meaning === SymbolFlags.Namespace || nameArg && typeof nameArg !== "string" && nodeIsSynthesized(nameArg) ? Diagnostics.Cannot_find_namespace_0_Did_you_mean_1
const message = meaning === SymbolFlags.Namespace ||
nameArg && typeof nameArg !== "string" && nodeIsSynthesized(nameArg) ?
Diagnostics.Cannot_find_namespace_0_Did_you_mean_1
: isUncheckedJS ? Diagnostics.Could_not_find_name_0_Did_you_mean_1
: Diagnostics.Cannot_find_name_0_Did_you_mean_1;
const diagnostic = createError(errorLocation, message, diagnosticName(nameArg), suggestionName);
diagnostic.canonicalHead = getCanonicalDiagnostic(nameNotFoundMessage, diagnosticName(nameArg));
addErrorOrSuggestion(!isUncheckedJS, diagnostic);
if (suggestion.valueDeclaration) {
addRelatedInfo(
Expand Down Expand Up @@ -21697,9 +21701,23 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
else {
errorInfo = elaborateNeverIntersection(errorInfo, originalTarget);
}
// Used by, eg, missing property checking to replace the top-level message with a more informative one.
if (!headMessage && maybeSuppress) {
// We suppress a call to `reportRelationError` or not depending on the state of the type checker, so
// we call `reportRelationError` here and then undo its effects to figure out what would be the diagnostic
// if we hadn't supress it, and save that as a canonical diagnostic for deduplication purposes.
const savedErrorState = captureErrorCalculationState();
reportRelationError(headMessage, source, target);
let canonical;
if (errorInfo && errorInfo !== savedErrorState.errorInfo) {
canonical = { code: errorInfo.code, messageText: errorInfo.messageText };
}
resetErrorInfo(savedErrorState);
if (canonical && errorInfo) {
errorInfo.canonicalHead = canonical;
}

lastSkippedInfo = [source, target];
// Used by, eg, missing property checking to replace the top-level message with a more informative one
return;
}
reportRelationError(headMessage, source, target);
Expand Down
17 changes: 17 additions & 0 deletions src/compiler/types.ts
Expand Up @@ -7069,6 +7069,8 @@ export interface DiagnosticMessageChain {
next?: DiagnosticMessageChain[];
/** @internal */
repopulateInfo?: () => RepopulateDiagnosticChainInfo;
/** @internal */
canonicalHead?: CanonicalDiagnostic;
}

export interface Diagnostic extends DiagnosticRelatedInformation {
Expand All @@ -7079,6 +7081,21 @@ export interface Diagnostic extends DiagnosticRelatedInformation {
source?: string;
relatedInformation?: DiagnosticRelatedInformation[];
/** @internal */ skippedOn?: keyof CompilerOptions;
/**
* @internal
* Used for deduplication and comparison.
* Whenever it is possible for two diagnostics that report the same problem to be produced with
* different messages (e.g. "Cannot find name 'foo'" vs "Cannot find name 'foo'. Did you mean 'bar'?"),
* this property can be set to a canonical message,
* so that those two diagnostics are appropriately considered to be the same.
*/
canonicalHead?: CanonicalDiagnostic;
}

/** @internal */
export interface CanonicalDiagnostic {
code: number;
messageText: string;
}

/** @internal */
Expand Down
67 changes: 52 additions & 15 deletions src/compiler/utilities.ts
Expand Up @@ -41,6 +41,7 @@ import {
canHaveLocals,
canHaveModifiers,
type CanHaveModuleSpecifier,
CanonicalDiagnostic,
CaseBlock,
CaseClause,
CaseOrDefaultClause,
Expand Down Expand Up @@ -2178,6 +2179,7 @@ export function createFileDiagnosticFromMessageChain(file: SourceFile, start: nu
category: messageChain.category,
messageText: messageChain.next ? messageChain : messageChain.messageText,
relatedInformation,
canonicalHead: messageChain.canonicalHead,
};
}

Expand Down Expand Up @@ -2216,6 +2218,14 @@ export function createDiagnosticForRange(sourceFile: SourceFile, range: TextRang
};
}

/** @internal */
export function getCanonicalDiagnostic(message: DiagnosticMessage, ...args: string[]): CanonicalDiagnostic {
return {
code: message.code,
messageText: formatMessage(message, ...args),
};
}

/** @internal */
export function getSpanOfTokenAtPosition(sourceFile: SourceFile, pos: number): TextSpan {
const scanner = createScanner(sourceFile.languageVersion, /*skipTrivia*/ true, sourceFile.languageVariant, sourceFile.text, /*onError*/ undefined, pos);
Expand Down Expand Up @@ -8540,11 +8550,13 @@ export function compareDiagnostics(d1: Diagnostic, d2: Diagnostic): Comparison {

/** @internal */
export function compareDiagnosticsSkipRelatedInformation(d1: Diagnostic, d2: Diagnostic): Comparison {
const code1 = getDiagnosticCode(d1);
const code2 = getDiagnosticCode(d2);
return compareStringsCaseSensitive(getDiagnosticFilePath(d1), getDiagnosticFilePath(d2)) ||
compareValues(d1.start, d2.start) ||
compareValues(d1.length, d2.length) ||
compareValues(d1.code, d2.code) ||
compareMessageText(d1.messageText, d2.messageText) ||
compareValues(code1, code2) ||
compareMessageText(d1, d2) ||
Comparison.EqualTo;
}

Expand All @@ -8566,25 +8578,38 @@ function compareRelatedInformation(d1: Diagnostic, d2: Diagnostic): Comparison {
// An diagnostic message with more elaboration should be considered *less than* a diagnostic message
// with less elaboration that is otherwise similar.
function compareMessageText(
t1: string | Pick<DiagnosticMessageChain, "messageText" | "next">,
t2: string | Pick<DiagnosticMessageChain, "messageText" | "next">,
d1: Diagnostic,
d2: Diagnostic,
): Comparison {
if (typeof t1 === "string" && typeof t2 === "string") {
return compareStringsCaseSensitive(t1, t2);
let headMsg1 = getDiagnosticMessage(d1);
let headMsg2 = getDiagnosticMessage(d2);
if (typeof headMsg1 !== "string") {
headMsg1 = headMsg1.messageText;
}

if (typeof t1 === "string") {
t1 = { messageText: t1 };
if (typeof headMsg2 !== "string") {
headMsg2 = headMsg2.messageText;
}
if (typeof t2 === "string") {
t2 = { messageText: t2 };
const chain1 = typeof d1.messageText !== "string" ? d1.messageText.next : undefined;
const chain2 = typeof d2.messageText !== "string" ? d2.messageText.next : undefined;

let res = compareStringsCaseSensitive(headMsg1, headMsg2);
if (res) {
return res;
}
const res = compareStringsCaseSensitive(t1.messageText, t2.messageText);

res = compareMessageChain(chain1, chain2);
if (res) {
return res;
}

return compareMessageChain(t1.next, t2.next);
if (d1.canonicalHead && !d2.canonicalHead) {
return Comparison.LessThan;
}
if (d2.canonicalHead && !d1.canonicalHead) {
return Comparison.GreaterThan;
}

return Comparison.EqualTo;
}

// First compare by size of the message chain,
Expand Down Expand Up @@ -8659,11 +8684,23 @@ function compareMessageChainContent(

/** @internal */
export function diagnosticsEqualityComparer(d1: Diagnostic, d2: Diagnostic): boolean {
const code1 = getDiagnosticCode(d1);
const code2 = getDiagnosticCode(d2);
const msg1 = getDiagnosticMessage(d1);
const msg2 = getDiagnosticMessage(d2);
return compareStringsCaseSensitive(getDiagnosticFilePath(d1), getDiagnosticFilePath(d2)) === Comparison.EqualTo &&
compareValues(d1.start, d2.start) === Comparison.EqualTo &&
compareValues(d1.length, d2.length) === Comparison.EqualTo &&
compareValues(d1.code, d2.code) === Comparison.EqualTo &&
messageTextEqualityComparer(d1.messageText, d2.messageText);
compareValues(code1, code2) === Comparison.EqualTo &&
messageTextEqualityComparer(msg1, msg2);
}

function getDiagnosticCode(d: Diagnostic): number {
return d.canonicalHead?.code || d.code;
}

function getDiagnosticMessage(d: Diagnostic): string | DiagnosticMessageChain {
return d.canonicalHead?.messageText || d.messageText;
}

function messageTextEqualityComparer(m1: string | DiagnosticMessageChain, m2: string | DiagnosticMessageChain): boolean {
Expand Down
22 changes: 22 additions & 0 deletions tests/baselines/reference/duplicateErrorAssignability.errors.txt
@@ -0,0 +1,22 @@
duplicateErrorAssignability.ts(10,11): error TS2741: Property 'x' is missing in type 'B' but required in type 'A'.
duplicateErrorAssignability.ts(12,5): error TS2538: Type 'B' cannot be used as an index type.


==== duplicateErrorAssignability.ts (2 errors) ====
interface A {
x: number;
}
interface B {
y: string;
}

declare let b: B;
declare let a: A;
const x = a = b;
~
!!! error TS2741: Property 'x' is missing in type 'B' but required in type 'A'.
!!! related TS2728 duplicateErrorAssignability.ts:2:5: 'x' is declared here.
let obj: { 3: string } = { 3: "three" };
obj[x];
~
!!! error TS2538: Type 'B' cannot be used as an index type.
21 changes: 21 additions & 0 deletions tests/baselines/reference/duplicateErrorAssignability.js
@@ -0,0 +1,21 @@
//// [tests/cases/compiler/duplicateErrorAssignability.ts] ////

//// [duplicateErrorAssignability.ts]
interface A {
x: number;
}
interface B {
y: string;
}

declare let b: B;
declare let a: A;
const x = a = b;
let obj: { 3: string } = { 3: "three" };
obj[x];

//// [duplicateErrorAssignability.js]
"use strict";
var x = a = b;
var obj = { 3: "three" };
obj[x];
38 changes: 38 additions & 0 deletions tests/baselines/reference/duplicateErrorAssignability.symbols
@@ -0,0 +1,38 @@
//// [tests/cases/compiler/duplicateErrorAssignability.ts] ////

=== duplicateErrorAssignability.ts ===
interface A {
>A : Symbol(A, Decl(duplicateErrorAssignability.ts, 0, 0))

x: number;
>x : Symbol(A.x, Decl(duplicateErrorAssignability.ts, 0, 13))
}
interface B {
>B : Symbol(B, Decl(duplicateErrorAssignability.ts, 2, 1))

y: string;
>y : Symbol(B.y, Decl(duplicateErrorAssignability.ts, 3, 13))
}

declare let b: B;
>b : Symbol(b, Decl(duplicateErrorAssignability.ts, 7, 11))
>B : Symbol(B, Decl(duplicateErrorAssignability.ts, 2, 1))

declare let a: A;
>a : Symbol(a, Decl(duplicateErrorAssignability.ts, 8, 11))
>A : Symbol(A, Decl(duplicateErrorAssignability.ts, 0, 0))

const x = a = b;
>x : Symbol(x, Decl(duplicateErrorAssignability.ts, 9, 5))
>a : Symbol(a, Decl(duplicateErrorAssignability.ts, 8, 11))
>b : Symbol(b, Decl(duplicateErrorAssignability.ts, 7, 11))

let obj: { 3: string } = { 3: "three" };
>obj : Symbol(obj, Decl(duplicateErrorAssignability.ts, 10, 3))
>3 : Symbol(3, Decl(duplicateErrorAssignability.ts, 10, 10))
>3 : Symbol(3, Decl(duplicateErrorAssignability.ts, 10, 26))

obj[x];
>obj : Symbol(obj, Decl(duplicateErrorAssignability.ts, 10, 3))
>x : Symbol(x, Decl(duplicateErrorAssignability.ts, 9, 5))

52 changes: 52 additions & 0 deletions tests/baselines/reference/duplicateErrorAssignability.types
@@ -0,0 +1,52 @@
//// [tests/cases/compiler/duplicateErrorAssignability.ts] ////

=== duplicateErrorAssignability.ts ===
interface A {
x: number;
>x : number
> : ^^^^^^
}
interface B {
y: string;
>y : string
> : ^^^^^^
}

declare let b: B;
>b : B
> : ^

declare let a: A;
>a : A
> : ^

const x = a = b;
>x : B
> : ^
>a = b : B
> : ^
>a : A
> : ^
>b : B
> : ^

let obj: { 3: string } = { 3: "three" };
>obj : { 3: string; }
> : ^^^^^ ^^^
>3 : string
> : ^^^^^^
>{ 3: "three" } : { 3: string; }
> : ^^^^^^^^^^^^^^
>3 : string
> : ^^^^^^
>"three" : "three"
> : ^^^^^^^

obj[x];
>obj[x] : any
> : ^^^
>obj : { 3: string; }
> : ^^^^^^^^^^^^^^
>x : B
> : ^

11 changes: 11 additions & 0 deletions tests/baselines/reference/duplicateErrorNameNotFound.errors.txt
@@ -0,0 +1,11 @@
duplicateErrorNameNotFound.ts(4,5): error TS2552: Cannot find name 'RoomInterface'. Did you mean 'RoomInterfae'?


==== duplicateErrorNameNotFound.ts (1 errors) ====
type RoomInterfae = {};

export type {
RoomInterface
~~~~~~~~~~~~~
!!! error TS2552: Cannot find name 'RoomInterface'. Did you mean 'RoomInterfae'?
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/duplicateErrorNameNotFound.js
@@ -0,0 +1,12 @@
//// [tests/cases/compiler/duplicateErrorNameNotFound.ts] ////

//// [duplicateErrorNameNotFound.ts]
type RoomInterfae = {};

export type {
RoomInterface
}

//// [duplicateErrorNameNotFound.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
10 changes: 10 additions & 0 deletions tests/baselines/reference/duplicateErrorNameNotFound.symbols
@@ -0,0 +1,10 @@
//// [tests/cases/compiler/duplicateErrorNameNotFound.ts] ////

=== duplicateErrorNameNotFound.ts ===
type RoomInterfae = {};
>RoomInterfae : Symbol(RoomInterfae, Decl(duplicateErrorNameNotFound.ts, 0, 0))

export type {
RoomInterface
>RoomInterface : Symbol(RoomInterface, Decl(duplicateErrorNameNotFound.ts, 2, 13))
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/duplicateErrorNameNotFound.types
@@ -0,0 +1,12 @@
//// [tests/cases/compiler/duplicateErrorNameNotFound.ts] ////

=== duplicateErrorNameNotFound.ts ===
type RoomInterfae = {};
>RoomInterfae : RoomInterfae
> : ^^^^^^^^^^^^

export type {
RoomInterface
>RoomInterface : any
> : ^^^
}
Expand Up @@ -4,7 +4,7 @@
Assignability cache: 5,000
Type Count: 10,000
Instantiation count: 250,000
Symbol count: 100,000
Symbol count: 250,000

=== mappedTypeRecursiveInference.ts ===
interface A { a: A }
Expand Down

0 comments on commit d2ad3ca

Please sign in to comment.