Skip to content

Preserve error elaboration for indexed access type assignments#3220

Open
WhiteMinds wants to merge 1 commit intomicrosoft:mainfrom
WhiteMinds:fix/improve-indexed-access-error-elaboration
Open

Preserve error elaboration for indexed access type assignments#3220
WhiteMinds wants to merge 1 commit intomicrosoft:mainfrom
WhiteMinds:fix/improve-indexed-access-error-elaboration

Conversation

@WhiteMinds
Copy link
Copy Markdown

Fixes microsoft/TypeScript#63206

Note: I understand the current contribution scope is limited to 6.0/7.0 differences and crashes. This is a diagnostic quality improvement rather than a behavioral change — no type-checking logic is altered, only the error message detail level. Happy to hold this until 7.0 if preferred.

Problem

When assigning to an indexed access type (e.g. this["faa"]), reportRelationError unconditionally clears errorChain before reporting the generic "could be instantiated with an arbitrary type" message, discarding the specific elaboration already computed from the constraint comparison (missing properties, type mismatches, etc.).

Fix

Preserve errorChain when the target is an IndexedAccess type, so users see both the generic message and the specific details.

Before

this.foo = {}
// Type '{}' is not assignable to type 'this["faa"]'.
//   'this["faa"]' could be instantiated with an arbitrary type which could be unrelated to '{}'.

After

this.foo = {}
// Type '{}' is not assignable to type 'this["faa"]'.
//   'this["faa"]' could be instantiated with an arbitrary type which could be unrelated to '{}'.
//     Property 'key' is missing in type '{}' but required in type '{ key: number | null; }'.

this.foo = {key: "str"}
// Type '{ key: string; }' is not assignable to type 'this["faa"]'.
//   'this["faa"]' could be instantiated with an arbitrary type which could be unrelated to '{ key: string; }'.
//     Type '{ key: string; }' is not assignable to type '{ key: number | null; }'.
//       Types of property 'key' are incompatible.
//         Type 'string' is not assignable to type 'number'.

Two existing baselines updated to reflect the additional elaboration (no errors removed, only more specific detail added).

When assigning to an indexed access type like this["faa"], the checker
resolves the constraint and performs a structural comparison that
generates specific error messages (e.g., missing properties or type
mismatches). However, reportRelationError unconditionally clears
errorChain before reporting the generic "could be instantiated with an
arbitrary type" message, discarding the useful elaboration.

This change preserves errorChain when the target is an IndexedAccess
type, so users see both the generic message and the specific details
about what went wrong (e.g., "Property 'key' is missing" or "Types of
property 'key' are incompatible").

Fixes microsoft/TypeScript#63206
@jakebailey
Copy link
Copy Markdown
Member

Maybe I don't have context here, but, I don't find the new messages more helpful than the existing one...

@gary-donut
Copy link
Copy Markdown

Maybe I don't have context here, but, I don't find the new messages more helpful than the existing one...

Consider this example:

class A {
    declare foo: this["faa"];
    declare faa: {key: number|null}

    fuu() {
        this.foo = {}           // vague error
        this.foo = {key: "str"} // vague error
    }
}

let foo: {key: number|null};
foo = {}           // precise error
foo = {key: "str"} // precise error

Both assign to the same underlying type {key: number|null}.
Direct assignment gives clear errors — "Property 'key' is missing"* or *"Type 'string' is not assignable to type 'number'".
But via this["faa"], you only get "could be instantiated with an arbitrary type which could be unrelated to '{}'".

Comment on lines +4718 to 4721
if target.flags&TypeFlagsIndexedAccess == 0 {
r.errorChain = nil
}
r.reportError(diagnostics.X_0_could_be_instantiated_with_an_arbitrary_type_which_could_be_unrelated_to_1, targetType, generalizedSourceType)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the error in general, it looks pretty confusing to have this message anywhere other than at the bottom of an elaboration.

Maybe guard all of this in an if r.errorChain != nil

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean skipping the entire "could be instantiated..." elaboration when errorChain already has specific diagnostics? Something like adding r.errorChain == nil to the outer guard:

if targetFlags&TypeFlagsTypeParameter != 0 && target != r.c.markerSuperTypeForCheck && target != r.c.markerSubTypeForCheck && r.errorChain == nil {

So when the constraint comparison has already produced precise errors (e.g., missing property or type mismatch), we skip the generic message entirely instead of prepending it.

Before (current):

Type '{}' is not assignable to type 'this["faa"]'.
  'this["faa"]' could be instantiated with an arbitrary type which could be unrelated to '{}'.

After:

Type '{}' is not assignable to type 'this["faa"]'.
  Property 'key' is missing in type '{}' but required in type '{ key: number | null; }'.
Type '{ key: string; }' is not assignable to type 'this["faa"]'.
  Type '{ key: string; }' is not assignable to type '{ key: number | null; }'.
    Types of property 'key' are incompatible.
      Type 'string' is not assignable to type 'number'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants