bogdanb commented 2 years ago

Bug Report

🔎 Search Terms

union return type

🕗 Version & Regression Information

This changed between versions 3.3.3333 and 3.5.1

The same bug happens every version newer than 3.3.3 that I tried, including 4.8.4 and nightly.

⏯ Playground Link

Playground link with relevant code

💻 Code

interface TestType<P> {
  prop: P;
}

interface StringType extends TestType<string> {}

interface NumberType extends TestType<number> {}

interface UnknownType extends TestType<unknown> {}

interface ResultOne<G extends UnknownType> {
  type: "one";
  value: G["prop"];
}

interface ResultTwo<G extends UnknownType> {
  type: "two";
  other: G["prop"];
}

function callback<G extends UnknownType>(): ResultOne<G> | ResultTwo<G> {
  const dt: ResultOne<StringType> = {
    type: "one",
    value: "abc",
  };

  return dt; // this should not be allowed, G['prop'] is not known to be a string
}

const tst = callback<NumberType>();
const shouldBeNumber: number = tst.type === "one" ? tst.value : tst.other;

console.log(
  shouldBeNumber,
  typeof shouldBeNumber,
  '<- this should say "number"'
);

🙁 Actual behavior

The callback happily returns the value, although its type does not match the declared return type. None of the other type annotations complain, either, but the program typing is clearly wrong. The log at the end makes it clear that the behavior does not match the declared types.

🙂 Expected behavior

The line that says return dt; should have a type error, because ResultOne<StringType> is not a valid match for the return type of the function.

Interestingly, v3.3.3 in Playground does report the correct error exactly on that line:

const dt: ResultOne<StringType>
Type 'ResultOne<StringType>' is not assignable to type 'ResultOne<G> | ResultTwo<G>'.
  Type 'ResultOne<StringType>' is not assignable to type 'ResultOne<G>'.
    Type 'StringType' is not assignable to type 'G'.

bogdanb commented 2 years ago

Note that the bug no longer happens if, instead of taking an XyzType and using G["prop"], the ResultXyz types receive the type of value as an argument directly, like:

interface ResultOne<G extends unknown> {
  type: "one";
  value: G;
}

RyanCavanaugh commented 2 years ago

Removing anything that might possibly be a red herring:

interface StringType { prop: string }
interface UnknownType { prop: unknown }
interface ResultOne<G extends UnknownType> {
  type: "one";
  value: G["prop"];
}
interface ResultTwo<G extends UnknownType> {
  type: "two";
  other: G["prop"];
}

function callback<G extends UnknownType>(s: ResultOne<StringType>) {
    // Fails, good
    const a1: ResultOne<G> = s;
    // Fails, good
    const a2: ResultTwo<G> = s;
    // OK (??????)
    // @ts-expect-error
    const m: ResultOne<G> | ResultTwo<G> = s;
}

I don't even understand how this is possible -- after a1 and a2 put the correct results in the assignability cache, m should have no choice but to fail. 😵

Andarist commented 1 year ago

@RyanCavanaugh the behavior here has changed in 5.1 (bisected to https://github.com/microsoft/TypeScript/pull/52106 ). Now the structural comparison kicks in sooner and based on this rule the source seems to satisfy the target.

Could you recheck how correct this is? I vaguely recall seeing this piece of code already and some issues that indicated that unsoundness like this is often the lesser evil but the inconsistency mentioned by OP here is somewhat worrying.

RyanCavanaugh commented 1 year ago

Well, new repro I guess

interface NumberType { prop: number}
interface StringType { prop: string }
interface UnknownType { prop: unknown }
interface ResultOne<G extends UnknownType> {
  type: "one";
  value: G["prop"];
}

function callback<G extends UnknownType>(s: ResultOne<StringType>): ResultOne<G> {
    // Should fail but does not
    return s;
}

// Unsound
const m = callback<NumberType>({ type: "one", value: "string" })
m.value.toFixed();