Regression: Unrelated line causes complex type check error to disappear

[phiresky]

TypeScript Version: No issue in 3.0.3, broken in both 3.2.2 and 3.3.0-dev.20190112 (using tsconfig from tsc --init)

Search Terms: unrelated, nondeterministic, mapped type, generic

Code

I'm sorry, I couldn't manage to reduce this further since it disappears when the code is too simple (probably some checking depth limitation?)

// when I replace this import by just inlining the corresponding types,
// the error disappears
import { TypeOf, Type } from "io-ts";

type ToB<S extends any> = { [k in keyof S]: TypeOf<S[k]> };
type ToA<S> = { [k in keyof S]: Type<S[k]> };

type NeededInfo<MyNamespaceSchema = {}> = {
  ASchema: ToA<MyNamespaceSchema>;
};

export const ASchema = {
  initialize: null as any
};

export type MyInfo = NeededInfo<ToB<typeof ASchema>>;

const tmp1: MyInfo = null!;
function tmp2<N extends NeededInfo>(n: N) {}
// tmp2(tmp1); // uncommenting this line removes a type error from a completely unrelated line ??

class Server<X extends NeededInfo> {}
export class MyServer extends Server<MyInfo> {} // not assignable error at `MyInfo`

Expected behavior: No type error regardless of whether the line //tmp2(tmp1) is commented or not.

Actual behavior:

When that line is commented out, I get a type error in the export class line, which is completely unrelated:

[ts]
Type 'NeededInfo<ToB<{ initialize: any; }>>' does not satisfy the constraint 'NeededInfo<{}>'.
  Types of property 'ASchema' are incompatible.
    Type 'ToA<ToB<{ initialize: any; }>>' is not assignable to type 'ToA<{}>'.
      Type '{}' is not assignable to type 'ToB<{ initialize: any; }>'. [2344]

Playground Link: Impossible because it stops happening when Type and TypeOf are inlined.

I know the above code looks dumb, but trust me this is causing weird breakage in a large code base.

[phiresky]

I just did a git bisect because this issue is preventing me from upgrading from 3.0 to a newer version of TS. The first bad commit is e2100cd by @weswigham , which was merged in #27804

I guess this comment is relevant:

However, I'm definitely concerned about the fact that the presence or absence of alias symbols now has a semantic effect. @ahejlsberg

Sadly I'm not really able to understand that code, otherwise I would try and fix it myself. I'm guessing it's related to this part: e2100cd#diff-c3ed224e4daa84352f7f1abcd23e8ccaR11944

I might be able to add a test case if that would help you fix it?

bisect log:

# bad: [25370369b23f025815999ede8ecd2df2185d71c0] Enable debug info by default when debugging the language server
# good: [7e1711dfb6759b6773abe2bcf6e956aac5399523] Update LKG
git bisect start 'v3.3-rc' 'v3.0.0'
# good: [bb57c5a6b24696eefefe5378d29c8497dee5194c] Update user baselines (#25482)
git bisect good bb57c5a6b24696eefefe5378d29c8497dee5194c
# good: [5f2741b2ba5251f2fb131c540c85606ad6038232] Make RenameInfo a union (#27382)
git bisect good 5f2741b2ba5251f2fb131c540c85606ad6038232
# bad: [830be0651c45a4f2fd51a0aefbe2bc0bffcf047e] Merge pull request #27669 from Microsoft/betterErrorForAccidentalCall
git bisect bad 830be0651c45a4f2fd51a0aefbe2bc0bffcf047e
# good: [00fbdedbccdb2daf10ee56ed0793ecc1d7a1d727] Accept new baselines
git bisect good 00fbdedbccdb2daf10ee56ed0793ecc1d7a1d727
# bad: [f9dd44517a77eb0692510a8ecbe12a056a2ef1f1] Merge branch 'master' into genericRest
git bisect bad f9dd44517a77eb0692510a8ecbe12a056a2ef1f1
# bad: [8e4b90da003292c60e2c92f4e975b88f6a3948ba] Merge pull request #28234 from Microsoft/genericSpread
git bisect bad 8e4b90da003292c60e2c92f4e975b88f6a3948ba
# bad: [364d4bd7d584361b73b58cc7ec03990e931a1324] LEGO: Merge pull request 28216
git bisect bad 364d4bd7d584361b73b58cc7ec03990e931a1324
# good: [0e372e98e83e649a1a7c103ea3dd4df8ce5f51bb] LEGO: Merge pull request 28146
git bisect good 0e372e98e83e649a1a7c103ea3dd4df8ce5f51bb
# good: [ccc16136b2974c90fd6c317b1a85d9458b786997] Merge pull request #28170 from Microsoft/fixGenericMappedTypeConstraint
git bisect good ccc16136b2974c90fd6c317b1a85d9458b786997
# bad: [672b0e3e16ad18b422dbe0cec5a98fce49881b76] Have flatMap return a ReadonlyArray by default (#28205)
git bisect bad 672b0e3e16ad18b422dbe0cec5a98fce49881b76
# bad: [a6952887e9215ab1601aabfc1eca29322f84273a] Use same condition in isReferencedAliasDeclaration as isAliasResolvedToValue (#28171)
git bisect bad a6952887e9215ab1601aabfc1eca29322f84273a
# bad: [e2100cd2cc6ba488cdd2a4bf055d78e2af4f0b5f] Measure variance of aliased conditional types using variance markers (#27804)
git bisect bad e2100cd2cc6ba488cdd2a4bf055d78e2af4f0b5f
# first bad commit: [e2100cd2cc6ba488cdd2a4bf055d78e2af4f0b5f] Measure variance of aliased conditional types using variance markers (#27804)

[ahejlsberg]

@weswigham FYI, when mapped types are excluded from the logic based on variance measuring for aliased types, the issues in this PR and #29698 disappear, so that's one possible fix. But of course the issue still exists for a mapped type nested in a conditional type.

[weswigham]

The real issue here is the inconsistency due to caching, not the variance measurements (we already knew variance was occasionally incorrect, which is why it is permissive and allows fallback to structural comparisons). That a second route to comparing MyInfo to NeededInfo can cache a different result is the really troublesome bug here - this implies that there is some codepath which skips variance measurement (or structural comparison) when it should not.

[weswigham]

Hahhhh, yeah - it's not the variance checking, per sey, that is causing the issue - in structuredTypeRelatedTo we set originalErrorInfo = errorInfo; to try to preserve the potentially nicer error info from the variance relationship when we report errors, and errorInfo is only ever set when diagnostic reporting is requested. The issue when arises where we override the result with false if originalErrorInfo is set, making the result depend on if diagnsotics were requested for the check or not! If the first check doesn't request diagnostics, a success is cached - otherwise, a failure is. With more convoluted types, the same bug can probably be witnessed with interface types; however the early-out false and empty-array check in the interface reference comparator block make it way harder to trigger.

I'm going to unify the two blocks, and then ensure the final result doesn't depend on originalErrorInfo, since its presence varies with the errorNode's undefineded-ness, which destroys our cache.

[ahejlsberg]

Right, agreed that the real issue in this PR is the inconsistent error message. But of course we also need to tackle the issue in #29698.