Fixed an issue with missingType narrowing by case undefined in default cases

[Andarist]

fixes #57999

[RyanCavanaugh]

@typescript-bot test it

[typescript-bot]

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
test top400	✅ Started	✅ Results
user test this	✅ Started	✅ Results
run dt	✅ Started	✅ Results
perf test this faster	✅ Started	👀 Results

[RyanCavanaugh]

I'm curious why we don't just put the regular undefined type into optionalProp; seems bad that it's so easy for this kind of mistake to happen. Wondering if there are others.

We should source out all the "compiler-inserted undefineds" in tests, e.g.

function func2(arg: A) {
  // Under noUncheckedIndexedAccess
  const optionalProp = ["hello" as const][Math.random()];
  switch (optionalProp) {
    case undefined:
    case "hello":
      return;
    default:
    // Should OK
      assertUnreachable(optionalProp)
  }
}

plus vary this test by all quadrants of NUIA / EOPT

[RyanCavanaugh]

@typescript-bot pack this

[typescript-bot]

Starting jobs; this comment will be updated as builds start and complete.

Command	Status	Results
pack this	✅ Started	✅ Results

[typescript-bot]

Hey @RyanCavanaugh, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/160860/artifacts?artifactName=tgz&fileId=FA67167521C00F0002CA45FFD874FED8C4CE7681FB236E593D3CC0383057EDCE02&fileName=/typescript-5.5.0-insiders.20240329.tgz"
    }
}

and then running npm install.

---

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.5.0-pr-58001-5".;

[typescript-bot]

Hey @RyanCavanaugh, the results of running the DT tests are ready.

Everything looks the same!

You can check the log here.

[typescript-bot]

@RyanCavanaugh Here are the results of running the user tests comparing main and refs/pull/58001/merge:

Everything looks good!

[typescript-bot]

@RyanCavanaugh
The results of the perf run you requested are in!

Here they are:

tsc

Comparison Report - baseline..pr

Metric	baseline	pr	Delta	Best	Worst	p-value
Angular - node (v18.15.0, x64)
Memory used	295,664k (± 0.01%)	295,682k (± 0.01%)	~	295,634k	295,708k	p=0.378 n=6
Parse Time	2.66s (± 0.15%)	2.66s (± 0.19%)	~	2.66s	2.67s	p=0.114 n=6
Bind Time	0.83s (± 0.62%)	0.83s (± 0.62%)	~	0.83s	0.84s	p=1.000 n=6
Check Time	8.24s (± 0.24%)	8.23s (± 0.53%)	~	8.16s	8.28s	p=0.747 n=6
Emit Time	7.05s (± 0.12%)	7.06s (± 0.50%)	~	7.03s	7.12s	p=1.000 n=6
Total Time	18.78s (± 0.13%)	18.78s (± 0.36%)	~	18.67s	18.87s	p=0.871 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used	192,713k (± 0.75%)	193,312k (± 0.91%)	~	192,011k	195,599k	p=1.000 n=6
Parse Time	1.36s (± 0.86%)	1.36s (± 1.04%)	~	1.34s	1.38s	p=0.803 n=6
Bind Time	0.73s (± 0.56%)	0.73s (± 0.00%)	~	0.73s	0.73s	p=0.405 n=6
Check Time	9.56s (± 0.89%)	9.58s (± 0.71%)	~	9.52s	9.69s	p=0.628 n=6
Emit Time	2.63s (± 0.39%)	2.63s (± 0.56%)	~	2.61s	2.65s	p=0.554 n=6
Total Time	14.28s (± 0.64%)	14.30s (± 0.55%)	~	14.23s	14.42s	p=0.686 n=6
Monaco - node (v18.15.0, x64)
Memory used	347,411k (± 0.00%)	347,418k (± 0.00%)	~	347,391k	347,432k	p=0.378 n=6
Parse Time	2.49s (± 0.66%)	2.49s (± 0.60%)	~	2.47s	2.51s	p=0.491 n=6
Bind Time	0.93s (± 0.44%)	0.93s (± 0.44%)	~	0.93s	0.94s	p=0.218 n=6
Check Time	7.03s (± 0.45%)	7.03s (± 0.34%)	~	7.01s	7.07s	p=0.935 n=6
Emit Time	4.07s (± 0.55%)	4.06s (± 0.40%)	~	4.04s	4.08s	p=0.565 n=6
Total Time	14.52s (± 0.12%)	14.52s (± 0.36%)	~	14.45s	14.58s	p=0.807 n=6
TFS - node (v18.15.0, x64)
Memory used	302,778k (± 0.01%)	302,770k (± 0.01%)	~	302,711k	302,827k	p=1.000 n=6
Parse Time	2.98s (± 1.12%)	2.96s (± 1.08%)	~	2.90s	2.99s	p=0.466 n=6
Bind Time	1.48s (± 0.55%)	1.48s (± 0.35%)	~	1.47s	1.48s	p=0.752 n=6
Check Time	9.24s (± 0.47%)	9.23s (± 0.50%)	~	9.18s	9.30s	p=0.687 n=6
Emit Time	5.30s (± 0.20%)	5.30s (± 0.46%)	~	5.27s	5.34s	p=0.566 n=6
Total Time	19.00s (± 0.31%)	18.97s (± 0.29%)	~	18.91s	19.04s	p=0.377 n=6
material-ui - node (v18.15.0, x64)
Memory used	510,044k (± 0.01%)	510,052k (± 0.00%)	~	510,018k	510,085k	p=0.689 n=6
Parse Time	2.66s (± 0.37%)	2.67s (± 0.55%)	~	2.65s	2.69s	p=0.508 n=6
Bind Time	0.98s (± 0.83%)	0.99s (± 0.82%)	~	0.98s	1.00s	p=0.077 n=6
Check Time	17.39s (± 0.58%)	17.33s (± 0.31%)	~	17.27s	17.41s	p=0.298 n=6
Emit Time	0.00s (± 0.00%)	0.00s (± 0.00%)	~	0.00s	0.00s	p=1.000 n=6
Total Time	21.04s (± 0.45%)	20.99s (± 0.26%)	~	20.92s	21.06s	p=0.423 n=6
mui-docs - node (v18.15.0, x64)
Memory used	1,744,689k (± 0.00%)	1,744,676k (± 0.00%)	~	1,744,642k	1,744,729k	p=0.689 n=6
Parse Time	9.62s (± 0.20%)	9.61s (± 0.70%)	~	9.54s	9.72s	p=0.687 n=6
Bind Time	3.47s (± 0.45%)	3.45s (± 0.86%)	~	3.40s	3.48s	p=0.139 n=6
Check Time	81.63s (± 0.27%)	81.74s (± 0.30%)	~	81.46s	82.13s	p=0.422 n=6
Emit Time	0.19s (± 2.81%)	0.19s (± 2.67%)	~	0.19s	0.20s	p=0.640 n=6
Total Time	94.91s (± 0.24%)	94.99s (± 0.23%)	~	94.73s	95.28s	p=0.689 n=6
self-build-src - node (v18.15.0, x64)
Memory used	2,399,785k (± 0.02%)	2,399,574k (± 0.04%)	~	2,398,530k	2,400,790k	p=0.689 n=6
Parse Time	6.02s (± 0.56%)	6.08s (± 2.15%)	~	5.98s	6.34s	p=0.199 n=6
Bind Time	2.25s (± 1.50%)	2.27s (± 3.00%)	~	2.21s	2.40s	p=1.000 n=6
Check Time	39.89s (± 0.28%)	40.00s (± 0.35%)	~	39.78s	40.16s	p=0.199 n=6
Emit Time	3.20s (± 1.89%)	3.20s (± 0.85%)	~	3.17s	3.24s	p=0.936 n=6
Total Time	51.38s (± 0.25%)	51.58s (± 0.53%)	~	51.36s	52.11s	p=0.093 n=6
self-compiler - node (v18.15.0, x64)
Memory used	416,097k (± 0.03%)	416,089k (± 0.01%)	~	416,056k	416,115k	p=0.229 n=6
Parse Time	4.13s (± 0.45%)	4.13s (± 0.68%)	~	4.10s	4.17s	p=0.936 n=6
Bind Time	1.59s (± 1.26%)	1.61s (± 1.51%)	~	1.58s	1.65s	p=0.134 n=6
Check Time	22.50s (± 0.36%)	22.49s (± 0.37%)	~	22.40s	22.61s	p=1.000 n=6
Emit Time	1.70s (± 1.56%)	1.70s (± 1.98%)	~	1.64s	1.73s	p=0.871 n=6
Total Time	29.92s (± 0.38%)	29.93s (± 0.28%)	~	29.84s	30.07s	p=0.936 n=6
vscode - node (v18.15.0, x64)
Memory used	2,898,757k (± 0.00%)	2,898,705k (± 0.00%)	~	2,898,420k	2,898,786k	p=0.689 n=6
Parse Time	12.95s (± 0.30%)	12.94s (± 0.40%)	~	12.90s	13.02s	p=0.871 n=6
Bind Time	4.14s (± 0.36%)	4.15s (± 0.54%)	~	4.12s	4.17s	p=0.566 n=6
Check Time	72.19s (± 0.57%)	72.00s (± 0.62%)	~	71.31s	72.60s	p=0.575 n=6
Emit Time	19.39s (± 0.24%)	20.12s (± 8.34%)	~	19.20s	23.52s	p=0.422 n=6
Total Time	108.67s (± 0.34%)	109.20s (± 1.69%)	~	108.10s	112.92s	p=0.688 n=6
webpack - node (v18.15.0, x64)
Memory used	408,918k (± 0.01%)	408,941k (± 0.01%)	~	408,870k	409,008k	p=0.575 n=6
Parse Time	4.79s (± 0.65%)	4.82s (± 0.40%)	~	4.80s	4.85s	p=0.145 n=6
Bind Time	2.05s (± 0.91%)	2.05s (± 0.85%)	~	2.04s	2.08s	p=0.802 n=6
Check Time	20.89s (± 0.61%)	20.91s (± 0.24%)	~	20.83s	20.95s	p=0.746 n=6
Emit Time	0.00s (± 0.00%)	0.00s (± 0.00%)	~	0.00s	0.00s	p=1.000 n=6
Total Time	27.74s (± 0.50%)	27.78s (± 0.29%)	~	27.67s	27.88s	p=0.748 n=6
xstate - node (v18.15.0, x64)
Memory used	513,447k (± 0.01%)	513,477k (± 0.01%)	~	513,414k	513,538k	p=0.471 n=6
Parse Time	3.96s (± 0.47%)	4.02s (± 4.43%)	~	3.91s	4.38s	p=0.570 n=6
Bind Time	1.89s (± 0.64%)	1.88s (± 1.15%)	~	1.86s	1.91s	p=0.808 n=6
Check Time	3.45s (± 0.75%)	3.45s (± 0.95%)	~	3.41s	3.48s	p=0.629 n=6
Emit Time	0.09s (± 5.95%)	0.09s (± 6.44%)	~	0.08s	0.09s	p=0.640 n=6
Total Time	9.39s (± 0.24%)	9.43s (± 1.95%)	~	9.31s	9.79s	p=1.000 n=6

System info unknown

Hosts

• node (v18.15.0, x64)

Scenarios

• Angular - node (v18.15.0, x64)
• Compiler-Unions - node (v18.15.0, x64)
• Monaco - node (v18.15.0, x64)
• TFS - node (v18.15.0, x64)
• material-ui - node (v18.15.0, x64)
• mui-docs - node (v18.15.0, x64)
• self-build-src - node (v18.15.0, x64)
• self-compiler - node (v18.15.0, x64)
• vscode - node (v18.15.0, x64)
• webpack - node (v18.15.0, x64)
• xstate - node (v18.15.0, x64)

Benchmark	Name	Iterations
Current	pr	6
Baseline	baseline	6

Developer Information:

Download Benchmarks

[Andarist]

I'm curious why we don't just put the regular undefined type into optionalProp; seems bad that it's so easy for this kind of mistake to happen.

Yeah, same. I actually have thought that this missing type should be converted to undefined way earlier but I could not find evidence of that anywhere right now.

Wondering if there are others.

I have 2 other open PRs related to EOPT ;p so yes, this is very easy to miss and I wouldn't be surprised if more bugs like this are still out there.

[Andarist]

a potential alternative to this could look like this:

Suggested change

	const defaultType = filterType(type, t => !(isUnitLikeType(t) && contains(switchTypes, t.flags & TypeFlags.Undefined ? undefinedType : getRegularTypeOfLiteralType(extractUnitType(t)))));
	const switchTypesUnion = getUnionType(filter(switchTypes, isUnitType));
	const defaultType = filterType(type, t => !(isUnitLikeType(t) && areTypesComparable(t, switchTypesUnion)));

this is inspired by the logic used by narrowTypeByEquality and I've checked that this alternative would still pass the existing test cases.

[typescript-bot]

@RyanCavanaugh Here are the results of running the top 400 repos comparing main and refs/pull/58001/merge:

Everything looks good!

[RyanCavanaugh]

Behavior / implementation seem fine but I'd love to know if there's a more comprehensive fix

[moataz2002]

Looks completed very well!