Fix generic nullable checks #59437

ahejlsberg · 2024-07-26T21:18:05Z

This PR builds on #59415 to implement the following in aggregate:

In an indexed access expression obj[key], when obj and key both have generic types we no longer require the constraint of obj to be a non-nullable type. The rationale for this change is that when key is a valid property name for obj, it must be because obj is of a non-nullable type, as discussed here.
We now consider unconstrained generic types and generic types constrained to unknown to possibly be null or undefined in --strictNullChecks mode. Previously, we would only consider generic types with constraints including null or undefined to possibly be nullable.

ahejlsberg · 2024-07-26T21:18:29Z

typescript-bot · 2024-07-26T21:18:32Z

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

typescript-bot · 2024-07-26T21:35:22Z

@ahejlsberg Here are the results of running the user tests with tsc comparing main and refs/pull/59437/merge:

Everything looks good!

typescript-bot · 2024-07-26T21:37:38Z

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

Everything looks the same!

You can check the log here.

typescript-bot · 2024-07-26T21:56:11Z

@ahejlsberg
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
_{Compiler-Unions - node (v18.15.0, x64)}
_Errors	₃₀	₃₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_62,153	_62,153	_~	_~	_~	_{p=1.000 n=6}
_Types	_50,242	_50,242	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{193,065k (± 0.72%)}	_{194,729k (± 0.96%)}	_~	_192,305k	_196,060k	_{p=0.378 n=6}
_{Parse Time}	_{1.30s (± 1.57%)}	_{1.31s (± 0.89%)}	_~	_1.29s	_1.32s	_{p=0.391 n=6}
_{Bind Time}	_0.71s	_0.71s	_~	_~	_~	_{p=1.000 n=6}
_{Check Time}	_{9.59s (± 0.41%)}	_{9.53s (± 0.32%)}	_{-0.06s (- 0.63%)}	_9.47s	_9.56s	_{p=0.024 n=6}
_{Emit Time}	_{2.74s (± 0.78%)}	_{2.72s (± 1.38%)}	_~	_2.65s	_2.76s	_{p=0.293 n=6}
_{Total Time}	_{14.33s (± 0.23%)}	_{14.26s (± 0.48%)}	_{-0.07s (- 0.52%)}	_14.13s	_14.32s	_{p=0.016 n=6}
_{angular-1 - node (v18.15.0, x64)}
_Errors	₇	₁₇	_{🔻+10 (+142.86%)}	_~	_~	_{p=0.001 n=6}
_Symbols	_945,532	_945,532	_~	_~	_~	_{p=1.000 n=6}
_Types	_409,507	_409,512	_{+5 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{1,221,152k (± 0.00%)}	_{1,221,178k (± 0.00%)}	_~	_1,221,099k	_1,221,213k	_{p=0.630 n=6}
_{Parse Time}	_{6.68s (± 0.83%)}	_{6.68s (± 0.57%)}	_~	_6.64s	_6.75s	_{p=1.000 n=6}
_{Bind Time}	_{1.90s (± 0.54%)}	_{1.90s (± 0.58%)}	_~	_1.89s	_1.92s	_{p=0.558 n=6}
_{Check Time}	_{31.26s (± 0.27%)}	_{31.25s (± 0.57%)}	_~	_31.08s	_31.51s	_{p=0.377 n=6}
_{Emit Time}	_{15.18s (± 0.40%)}	_{15.19s (± 0.33%)}	_~	_15.14s	_15.27s	_{p=0.809 n=6}
_{Total Time}	_{55.02s (± 0.19%)}	_{55.02s (± 0.37%)}	_~	_54.87s	_55.33s	_{p=0.687 n=6}
_{mui-docs - node (v18.15.0, x64)}
_Errors	₀	₁₆	_{🔻+16 (+ ∞%)}	_~	_~	_{p=0.001 n=6}
_Symbols	_2,437,253	_2,437,253	_~	_~	_~	_{p=1.000 n=6}
_Types	_1,004,097	_1,004,141	_{+44 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,426,339k (± 0.00%)}	_{2,426,265k (± 0.00%)}	_~	_2,426,226k	_2,426,333k	_{p=0.066 n=6}
_{Parse Time}	_{8.43s (± 0.31%)}	_{8.44s (± 0.21%)}	_~	_8.42s	_8.47s	_{p=0.686 n=6}
_{Bind Time}	_{2.08s (± 0.56%)}	_{2.08s (± 0.30%)}	_~	_2.07s	_2.09s	_{p=1.000 n=6}
_{Check Time}	_{75.18s (± 0.66%)}	_{74.77s (± 0.39%)}	_{-0.40s (- 0.54%)}	_74.23s	_75.00s	_{p=0.045 n=6}
_{Emit Time}	_{0.28s (± 1.47%)}	_{0.28s (± 1.47%)}	_~	_0.27s	_0.28s	_{p=1.000 n=6}
_{Total Time}	_{85.96s (± 0.59%)}	_{85.57s (± 0.32%)}	_~	_85.06s	_85.77s	_{p=0.066 n=6}
_{self-build-src - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,227,735	_1,227,781	_{+46 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_265,136	_265,143	_{+7 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,346,556k (± 0.01%)}	_{2,346,515k (± 0.03%)}	_~	_2,345,869k	_2,347,878k	_{p=0.378 n=6}
_{Parse Time}	_{5.03s (± 0.57%)}	_{5.01s (± 1.07%)}	_~	_4.94s	_5.08s	_{p=0.471 n=6}
_{Bind Time}	_{1.92s (± 0.43%)}	_{1.92s (± 0.39%)}	_~	_1.91s	_1.93s	_{p=0.729 n=6}
_{Check Time}	_{34.74s (± 0.57%)}	_{34.78s (± 0.22%)}	_~	_34.69s	_34.89s	_{p=0.936 n=6}
_{Emit Time}	_{3.33s (± 0.94%)}	_{3.30s (± 1.61%)}	_~	_3.21s	_3.35s	_{p=0.229 n=6}
_{Total Time}	_{45.03s (± 0.46%)}	_{45.03s (± 0.27%)}	_~	_44.90s	_45.20s	_{p=0.936 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,227,735	_1,227,781	_{+46 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_265,136	_265,143	_{+7 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,420,283k (± 0.03%)}	_{2,420,571k (± 0.03%)}	_~	_2,419,926k	_2,421,919k	_{p=1.000 n=6}
_{Parse Time}	_{6.19s (± 0.70%)}	_{6.19s (± 0.69%)}	_~	_6.14s	_6.25s	_{p=1.000 n=6}
_{Bind Time}	_{2.02s (± 1.15%)}	_{2.03s (± 0.86%)}	_~	_2.01s	_2.06s	_{p=0.373 n=6}
_{Check Time}	_{41.17s (± 0.26%)}	_{41.05s (± 0.19%)}	_~	_40.94s	_41.17s	_{p=0.093 n=6}
_{Emit Time}	_{4.12s (± 4.15%)}	_{4.00s (± 1.06%)}	_~	_3.95s	_4.07s	_{p=0.107 n=6}
_{Total Time}	_{53.51s (± 0.35%)}	_{53.30s (± 0.27%)}	_~	_53.06s	_53.47s	_{p=0.066 n=6}
_{self-compiler - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_255,950	_255,952	_{+2 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_104,957	_104,964	_{+7 (+ 0.01%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{427,965k (± 0.04%)}	_{427,955k (± 0.04%)}	_~	_427,739k	_428,298k	_{p=0.936 n=6}
_{Parse Time}	_{3.35s (± 0.70%)}	_{3.36s (± 0.69%)}	_~	_3.33s	_3.39s	_{p=0.624 n=6}
_{Bind Time}	_{1.32s (± 0.88%)}	_{1.31s (± 1.42%)}	_~	_1.29s	_1.33s	_{p=0.445 n=6}
_{Check Time}	_{17.98s (± 0.45%)}	_{18.00s (± 0.39%)}	_~	_17.90s	_18.06s	_{p=0.809 n=6}
_{Emit Time}	_{1.65s (± 1.04%)}	_{1.64s (± 1.37%)}	_~	_1.62s	_1.67s	_{p=0.682 n=6}
_{Total Time}	_{24.31s (± 0.27%)}	_{24.32s (± 0.33%)}	_~	_24.21s	_24.39s	_{p=0.810 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_Errors	₃₅	₃₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_224,931	_224,931	_~	_~	_~	_{p=1.000 n=6}
_Types	_94,146	_94,146	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{370,148k (± 0.04%)}	_{370,101k (± 0.04%)}	_~	_369,914k	_370,319k	_{p=0.630 n=6}
_{Parse Time}	_{2.76s (± 0.78%)}	_{2.76s (± 0.42%)}	_~	_2.74s	_2.77s	_{p=0.625 n=6}
_{Bind Time}	_{1.58s (± 1.09%)}	_{1.58s (± 1.30%)}	_~	_1.55s	_1.61s	_{p=0.514 n=6}
_{Check Time}	_{15.67s (± 0.23%)}	_{15.71s (± 0.21%)}	_~	_15.65s	_15.75s	_{p=0.053 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{20.01s (± 0.13%)}	_{20.05s (± 0.07%)}	_{+0.04s (+ 0.19%)}	_20.03s	_20.07s	_{p=0.024 n=6}
_{vscode - node (v18.15.0, x64)}
_Errors	₁₁	₁₁	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,986,839	_2,986,839	_~	_~	_~	_{p=1.000 n=6}
_Types	_1,027,468	_1,027,468	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{3,110,626k (± 0.00%)}	_{3,110,688k (± 0.00%)}	_~	_3,110,618k	_3,110,747k	_{p=0.128 n=6}
_{Parse Time}	_{13.87s (± 0.52%)}	_{13.84s (± 0.40%)}	_~	_13.76s	_13.93s	_{p=0.687 n=6}
_{Bind Time}	_{4.35s (± 2.77%)}	_{4.32s (± 2.02%)}	_~	_4.28s	_4.50s	_{p=0.624 n=6}
_{Check Time}	_{79.38s (± 0.42%)}	_{79.18s (± 0.30%)}	_~	_78.91s	_79.54s	_{p=0.298 n=6}
_{Emit Time}	_{20.52s (± 0.37%)}	_{20.50s (± 0.76%)}	_~	_20.23s	_20.63s	_{p=1.000 n=6}
_{Total Time}	_{118.12s (± 0.34%)}	_{117.84s (± 0.29%)}	_~	_117.37s	_118.35s	_{p=0.298 n=6}
_{webpack - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_267,567	_267,567	_~	_~	_~	_{p=1.000 n=6}
_Types	_109,077	_109,077	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{412,438k (± 0.01%)}	_{412,463k (± 0.02%)}	_~	_412,378k	_412,539k	_{p=0.689 n=6}
_{Parse Time}	_{3.80s (± 0.86%)}	_{3.82s (± 0.59%)}	_~	_3.78s	_3.85s	_{p=0.744 n=6}
_{Bind Time}	_{1.70s (± 0.64%)}	_{1.71s (± 0.86%)}	_~	_1.68s	_1.72s	_{p=0.131 n=6}
_{Check Time}	_{16.85s (± 0.38%)}	_{16.82s (± 0.43%)}	_~	_16.74s	_16.94s	_{p=0.471 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{22.36s (± 0.31%)}	_{22.34s (± 0.43%)}	_~	_22.26s	_22.52s	_{p=0.574 n=6}
_{xstate-main - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_511,807	_511,807	_~	_~	_~	_{p=1.000 n=6}
_Types	_162,090	_162,090	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{449,073k (± 0.08%)}	_{449,313k (± 0.03%)}	_~	_449,121k	_449,516k	_{p=0.298 n=6}
_{Parse Time}	_{3.16s (± 0.51%)}	_{3.16s (± 0.62%)}	_~	_3.13s	_3.18s	_{p=0.618 n=6}
_{Bind Time}	_{1.16s (± 1.01%)}	_{1.16s (± 0.35%)}	_~	_1.15s	_1.16s	_{p=0.787 n=6}
_{Check Time}	_{17.16s (± 0.33%)}	_{17.18s (± 0.50%)}	_~	_17.07s	_17.33s	_{p=0.748 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{21.48s (± 0.29%)}	_{21.50s (± 0.43%)}	_~	_21.35s	_21.63s	_{p=0.936 n=6}

System info unknown

Hosts

node (v18.15.0, x64)

Scenarios

Compiler-Unions - node (v18.15.0, x64)
angular-1 - node (v18.15.0, x64)
mui-docs - node (v18.15.0, x64)
self-build-src - node (v18.15.0, x64)
self-build-src-public-api - node (v18.15.0, x64)
self-compiler - node (v18.15.0, x64)
ts-pre-modules - node (v18.15.0, x64)
vscode - node (v18.15.0, x64)
webpack - node (v18.15.0, x64)
xstate-main - node (v18.15.0, x64)

_Benchmark	_Name	_Iterations
_Current	_pr	₆
_Baseline	_baseline	₆

Developer Information:

Download Benchmarks

typescript-bot · 2024-07-26T22:56:29Z

@ahejlsberg Here are the results of running the top 400 repos with tsc comparing main and refs/pull/59437/merge:

Something interesting changed - please have a look.

Details

ag-grid/ag-grid

31 of 110 projects failed to build with the old tsc and were ignored

compiler-explorer/compiler-explorer

4 of 6 projects failed to build with the old tsc and were ignored

static/tsconfig.json

error TS18049: 'a' is possibly 'null' or 'undefined'.
- shared/common-utils.ts#L55
- shared/common-utils.ts#L57
error TS18049: 'b' is possibly 'null' or 'undefined'.
- shared/common-utils.ts#L55
- shared/common-utils.ts#L57

desktop/desktop

1 of 4 projects failed to build with the old tsc and were ignored

tsconfig.json

error TS18049: 'x' is possibly 'null' or 'undefined'.
error TS18049: 'y' is possibly 'null' or 'undefined'.

sequelize/sequelize

13 of 16 projects failed to build with the old tsc and were ignored

packages/utils/tsconfig.json

error TS18049: 'a' is possibly 'null' or 'undefined'.
- packages/utils/src/common/comparators/basic-comparator.ts#L21
- packages/utils/src/common/comparators/basic-comparator.ts#L25
error TS18049: 'b' is possibly 'null' or 'undefined'.
- packages/utils/src/common/comparators/basic-comparator.ts#L21
- packages/utils/src/common/comparators/basic-comparator.ts#L25

ahejlsberg · 2024-07-29T14:29:03Z

The new errors in the top 400 repos tests are all the same: Functions that compare values of unconstrained generic types. Our existing behavior is to permit such comparisons--we only error when the types have constraints that explicitly include null or undefined (which obviously is inconsistent).

We should decide if we think erroring on generic operands that are possibly null or undefined is the right thing to do since it basically amounts to a breaking change. The semantics of comparisons involving null and undefined are well defined (they act like NaN) and don't cause exceptions, yet it's probably rare for such comparisons to be intended.

ahejlsberg · 2024-07-30T21:06:37Z

We're going to wait for 5.7 on this one since it is technically a breaking change.

ahejlsberg added 6 commits July 24, 2024 15:16

In obj[key], omit non-nullable check when obj and key are both generic

7cba33c

Add tests

b93a10b

Accept new baselines

b9b07c4

Nullable errors on unconstrained generic types in strictNullChecks mode

9aec49b

Accept new baselines

1f5d7a3

Include tests from #59352

2e02dc4

typescript-bot assigned ahejlsberg Jul 26, 2024

typescript-bot added the Author: Team label Jul 26, 2024

typescript-bot assigned iisaduan Jul 26, 2024

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Jul 26, 2024

ahejlsberg mentioned this pull request Jul 26, 2024

consistently check expressions with >, >=, <, <= for unconstrained types in strictNullChecks #59352

Open

ahejlsberg mentioned this pull request Jul 29, 2024

No error on unconstrained type parameter in > comparison #50603

Open

ahejlsberg requested a review from RyanCavanaugh July 29, 2024 21:25

Fix generic nullable checks #59437

Are you sure you want to change the base?

Fix generic nullable checks #59437

Conversation

ahejlsberg commented Jul 26, 2024 • edited Loading

ahejlsberg commented Jul 26, 2024

typescript-bot commented Jul 26, 2024 • edited Loading

typescript-bot commented Jul 26, 2024

typescript-bot commented Jul 26, 2024

typescript-bot commented Jul 26, 2024

tsc

typescript-bot commented Jul 26, 2024

ahejlsberg commented Jul 29, 2024 • edited Loading

ahejlsberg commented Jul 30, 2024

ahejlsberg commented Jul 26, 2024 •

edited

Loading

typescript-bot commented Jul 26, 2024 •

edited

Loading

ahejlsberg commented Jul 29, 2024 •

edited

Loading