fix53287 `mergeSymbol` checks if the resolved target can merge with the source

iisaduan commented 2 weeks ago

Fixes https://github.com/microsoft/TypeScript/issues/53287

While investigating the original issue, I found that re-exporting an alias would circumvent any checks to determine whether or not the source and the target are allowed to merge at all.

This adds a check in mergeSymbol to check if the resolved target can merge with the source, and if they cannot merge, then we issue the same errors as if there was no reexporting.

Got lots of help from @sandersn looking over the new errors 🙂

iisaduan commented 2 weeks ago

@typescript-bot test it

typescript-bot commented 2 weeks ago

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 commented 2 weeks ago

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

There were interesting changes:

Branch only errors:

Package: karma-chai-sinon Error: ``` Error: /mnt/vss/_work/1/DefinitelyTyped/types/karma-chai-sinon/index.d.ts 8:9 error TypeScript@local compile error: Duplicate identifier 'sinon' @definitelytyped/expect ✖ 1 problem (1 error, 0 warnings) at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240426/node_modules/@definitelytyped/dtslint/dist/index.js:194:28) at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240426/node_modules/@definitelytyped/dtslint/dist/index.js:186:20) ``` Package: bandagedbd__bdapi Error: ``` Error: /mnt/vss/_work/1/DefinitelyTyped/types/bandagedbd__bdapi/index.d.ts 9:11 error TypeScript@local compile error: Cannot redeclare block-scoped variable '_' @definitelytyped/expect 9:11 error TypeScript@local compile error: '_' is referenced directly or indirectly in its own type annotation @definitelytyped/expect ✖ 2 problems (2 errors, 0 warnings) at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240426/node_modules/@definitelytyped/dtslint/dist/index.js:194:28) at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240426/node_modules/@definitelytyped/dtslint/dist/index.js:186:20) ``` Package: regenerator-runtime Error: ``` Error: /mnt/vss/_work/1/DefinitelyTyped/types/regenerator-runtime/index.d.ts 1:21 error TypeScript@local compile error: Duplicate identifier 'regeneratorRuntime' @definitelytyped/expect 3:9 error TypeScript@local compile error: Duplicate identifier 'regeneratorRuntime' @definitelytyped/expect ✖ 2 problems (2 errors, 0 warnings) at combineErrorsAndWarnings (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240426/node_modules/@definitelytyped/dtslint/dist/index.js:194:28) at runTests (/mnt/vss/_work/1/DefinitelyTyped/node_modules/.pnpm/@definitelytyped+dtslint@0.2.20_typescript@5.5.0-dev.20240426/node_modules/@definitelytyped/dtslint/dist/index.js:186:20) ```

You can check the log here.

typescript-bot commented 2 weeks ago

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

Everything looks good!

typescript-bot commented 2 weeks ago

@iisaduan 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,154	_62,154	_~	_~	_~	_{p=1.000 n=6}
_Types	_50,273	_50,273	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{194,541k (± 0.96%)}	_{192,791k (± 0.74%)}	_~	_192,101k	_195,677k	_{p=0.173 n=6}
_{Parse Time}	_{1.35s (± 1.95%)}	_{1.36s (± 1.18%)}	_~	_1.33s	_1.38s	_{p=0.739 n=6}
_{Bind Time}	_{0.72s (± 0.57%)}	_{0.72s (± 0.57%)}	_~	_0.72s	_0.73s	_{p=1.000 n=6}
_{Check Time}	_{9.58s (± 0.54%)}	_{9.60s (± 0.43%)}	_~	_9.52s	_9.63s	_{p=0.296 n=6}
_{Emit Time}	_{2.62s (± 0.82%)}	_{2.62s (± 0.37%)}	_~	_2.61s	_2.63s	_{p=0.805 n=6}
_{Total Time}	_{14.27s (± 0.39%)}	_{14.30s (± 0.32%)}	_~	_14.23s	_14.35s	_{p=0.377 n=6}
_{angular-1 - node (v18.15.0, x64)}
_Errors	₅	₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_945,172	_945,172	_~	_~	_~	_{p=1.000 n=6}
_Types	_408,068	_408,068	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{1,222,038k (± 0.00%)}	_{1,221,992k (± 0.01%)}	_~	_1,221,865k	_1,222,117k	_{p=0.378 n=6}
_{Parse Time}	_{6.91s (± 0.28%)}	_{6.90s (± 0.61%)}	_~	_6.84s	_6.96s	_{p=0.572 n=6}
_{Bind Time}	_{1.87s (± 0.88%)}	_{1.87s (± 0.53%)}	_~	_1.86s	_1.88s	_{p=0.934 n=6}
_{Check Time}	_{31.56s (± 0.54%)}	_{31.32s (± 0.17%)}	_{-0.25s (- 0.78%)}	_31.26s	_31.41s	_{p=0.020 n=6}
_{Emit Time}	_{14.67s (± 0.56%)}	_{14.58s (± 0.39%)}	_~	_14.50s	_14.66s	_{p=0.054 n=6}
_{Total Time}	_{55.01s (± 0.47%)}	_{54.66s (± 0.19%)}	_{-0.35s (- 0.63%)}	_54.48s	_54.80s	_{p=0.031 n=6}
_{mui-docs - node (v18.15.0, x64)}
_Errors	₅	₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,954,602	_1,954,602	_~	_~	_~	_{p=1.000 n=6}
_Types	_676,397	_676,397	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{1,753,365k (± 0.00%)}	_{1,753,357k (± 0.00%)}	_~	_1,753,336k	_1,753,405k	_{p=0.810 n=6}
_{Parse Time}	_{6.89s (± 0.12%)}	_{6.88s (± 0.43%)}	_~	_6.82s	_6.90s	_{p=0.354 n=6}
_{Bind Time}	_{2.30s (± 0.35%)}	_{2.30s (± 0.27%)}	_~	_2.29s	_2.31s	_{p=0.432 n=6}
_{Check Time}	_{56.85s (± 0.48%)}	_{56.82s (± 0.35%)}	_~	_56.61s	_57.03s	_{p=0.809 n=6}
_{Emit Time}	_0.14s	_{0.14s (± 5.31%)}	_~	_0.13s	_0.15s	_{p=0.598 n=6}
_{Total Time}	_{66.19s (± 0.41%)}	_{66.15s (± 0.28%)}	_~	_65.94s	_66.35s	_{p=0.810 n=6}
_{self-build-src - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,214,779	_1,214,782	_{+3 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_257,315	_257,316	_{+1 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,322,738k (± 0.03%)}	_{2,322,276k (± 0.03%)}	_~	_2,321,392k	_2,323,433k	_{p=0.575 n=6}
_{Parse Time}	_{5.04s (± 1.37%)}	_{5.05s (± 0.88%)}	_~	_4.99s	_5.12s	_{p=0.936 n=6}
_{Bind Time}	_{1.88s (± 0.92%)}	_{1.88s (± 0.87%)}	_~	_1.86s	_1.90s	_{p=0.871 n=6}
_{Check Time}	_{34.04s (± 0.30%)}	_{34.03s (± 0.32%)}	_~	_33.96s	_34.24s	_{p=0.936 n=6}
_{Emit Time}	_{2.74s (± 2.33%)}	_{2.76s (± 1.94%)}	_~	_2.65s	_2.79s	_{p=1.000 n=6}
_{Total Time}	_{43.73s (± 0.39%)}	_{43.75s (± 0.25%)}	_~	_43.63s	_43.95s	_{p=0.810 n=6}
_{self-build-src-public-api - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_1,214,779	_1,214,782	_{+3 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_257,315	_257,316	_{+1 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{2,397,465k (± 0.03%)}	_{2,396,917k (± 0.02%)}	_~	_2,396,150k	_2,397,356k	_{p=0.128 n=6}
_{Parse Time}	_{6.30s (± 1.11%)}	_{6.34s (± 1.03%)}	_~	_6.25s	_6.40s	_{p=0.298 n=6}
_{Bind Time}	_{2.00s (± 2.23%)}	_{2.01s (± 1.77%)}	_~	_1.97s	_2.06s	_{p=0.936 n=6}
_{Check Time}	_{40.58s (± 0.22%)}	_{40.61s (± 0.19%)}	_~	_40.49s	_40.70s	_{p=0.521 n=6}
_{Emit Time}	_{3.20s (± 1.81%)}	_{3.23s (± 2.96%)}	_~	_3.06s	_3.31s	_{p=0.378 n=6}
_{Total Time}	_{52.10s (± 0.28%)}	_{52.22s (± 0.22%)}	_~	_52.08s	_52.37s	_{p=0.298 n=6}
_{self-compiler - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_256,043	_256,046	_{+3 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_Types	_103,601	_103,602	_{+1 (+ 0.00%)}	_~	_~	_{p=0.001 n=6}
_{Memory used}	_{423,971k (± 0.01%)}	_{423,981k (± 0.01%)}	_~	_423,957k	_424,018k	_{p=0.229 n=6}
_{Parse Time}	_{3.48s (± 1.45%)}	_{3.48s (± 0.72%)}	_~	_3.45s	_3.52s	_{p=1.000 n=6}
_{Bind Time}	_{1.30s (± 1.26%)}	_{1.30s (± 0.97%)}	_~	_1.28s	_1.32s	_{p=0.864 n=6}
_{Check Time}	_{18.16s (± 0.24%)}	_{18.16s (± 0.31%)}	_~	_18.10s	_18.24s	_{p=0.936 n=6}
_{Emit Time}	_{1.41s (± 1.37%)}	_{1.40s (± 1.16%)}	_~	_1.38s	_1.43s	_{p=0.560 n=6}
_{Total Time}	_{24.36s (± 0.35%)}	_{24.34s (± 0.19%)}	_~	_24.27s	_24.40s	_{p=0.936 n=6}
_{ts-pre-modules - node (v18.15.0, x64)}
_Errors	₃₅	₃₅	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_224,824	_224,824	_~	_~	_~	_{p=1.000 n=6}
_Types	_93,390	_93,390	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{369,279k (± 0.01%)}	_{369,288k (± 0.01%)}	_~	_369,202k	_369,335k	_{p=0.471 n=6}
_{Parse Time}	_{3.69s (± 0.61%)}	_{3.67s (± 0.56%)}	_~	_3.64s	_3.70s	_{p=0.075 n=6}
_{Bind Time}	_{1.94s (± 0.92%)}	_{1.92s (± 1.17%)}	_~	_1.90s	_1.96s	_{p=0.224 n=6}
_{Check Time}	_{19.46s (± 0.69%)}	_{19.43s (± 0.55%)}	_~	_19.26s	_19.52s	_{p=0.575 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{25.09s (± 0.54%)}	_{25.02s (± 0.49%)}	_~	_24.81s	_25.12s	_{p=0.471 n=6}
_{vscode - node (v18.15.0, x64)}
_Errors	₄	₄	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_2,796,261	_2,796,261	_~	_~	_~	_{p=1.000 n=6}
_Types	_950,064	_950,064	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{2,924,835k (± 0.01%)}	_{2,924,849k (± 0.00%)}	_~	_2,924,612k	_2,924,958k	_{p=0.936 n=6}
_{Parse Time}	_{13.49s (± 0.22%)}	_{13.49s (± 0.48%)}	_~	_13.41s	_13.59s	_{p=0.871 n=6}
_{Bind Time}	_{4.10s (± 0.70%)}	_{4.09s (± 0.27%)}	_~	_4.07s	_4.10s	_{p=0.805 n=6}
_{Check Time}	_{72.93s (± 0.26%)}	_{72.91s (± 0.49%)}	_~	_72.46s	_73.32s	_{p=0.936 n=6}
_{Emit Time}	_{20.87s (± 9.44%)}	_{20.23s (± 7.76%)}	_~	_19.48s	_23.43s	_{p=0.575 n=6}
_{Total Time}	_{111.39s (± 1.75%)}	_{110.72s (± 1.32%)}	_~	_109.48s	_113.62s	_{p=0.810 n=6}
_{webpack - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_265,853	_265,853	_~	_~	_~	_{p=1.000 n=6}
_Types	_108,438	_108,438	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{410,417k (± 0.02%)}	_{410,417k (± 0.02%)}	_~	_410,334k	_410,603k	_{p=0.936 n=6}
_{Parse Time}	_{4.88s (± 0.82%)}	_{4.86s (± 0.55%)}	_~	_4.83s	_4.89s	_{p=0.453 n=6}
_{Bind Time}	_{2.08s (± 0.66%)}	_{2.08s (± 1.10%)}	_~	_2.06s	_2.11s	_{p=0.742 n=6}
_{Check Time}	_{21.13s (± 0.50%)}	_{21.14s (± 0.65%)}	_~	_20.91s	_21.31s	_{p=0.748 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{28.10s (± 0.48%)}	_{28.09s (± 0.51%)}	_~	_27.85s	_28.29s	_{p=1.000 n=6}
_{xstate-main - node (v18.15.0, x64)}
_Errors	₀	₀	_~	_~	_~	_{p=1.000 n=6}
_Symbols	_523,981	_523,981	_~	_~	_~	_{p=1.000 n=6}
_Types	_178,708	_178,708	_~	_~	_~	_{p=1.000 n=6}
_{Memory used}	_{461,341k (± 0.03%)}	_{461,238k (± 0.01%)}	_~	_461,165k	_461,309k	_{p=0.128 n=6}
_{Parse Time}	_{4.02s (± 0.70%)}	_{3.99s (± 0.47%)}	_~	_3.97s	_4.02s	_{p=0.147 n=6}
_{Bind Time}	_{1.47s (± 0.51%)}	_{1.47s (± 0.79%)}	_~	_1.46s	_1.49s	_{p=0.734 n=6}
_{Check Time}	_{22.49s (± 0.65%)}	_{22.40s (± 0.52%)}	_~	_22.27s	_22.62s	_{p=0.470 n=6}
_{Emit Time}	_0.00s	_0.00s	_~	_~	_~	_{p=1.000 n=6}
_{Total Time}	_{27.97s (± 0.51%)}	_{27.87s (± 0.38%)}	_~	_27.73s	_28.05s	_{p=0.375 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 commented 2 weeks ago

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

Something interesting changed - please have a look.

Details

actualbudget/actual

**2 of 4 projects failed to build with the old tsc and were ignored** ### [packages\/loot-core\/tsconfig.api.json](https://github.com/actualbudget/actual/blob/164dd399b0a5b2e5b3755d283020ba8f0eddc5b8/packages/loot-core/tsconfig.api.json) - `error TS2554: Expected 0 arguments, but got 1.` - [packages\/loot-core\/src\/server\/accounts\/rules.ts\#L39](https://github.com/actualbudget/actual/blob/164dd399b0a5b2e5b3755d283020ba8f0eddc5b8/packages/loot-core/src/server/accounts/rules.ts#L39) - [packages\/loot-core\/src\/server\/schedules\/app.ts\#L377](https://github.com/actualbudget/actual/blob/164dd399b0a5b2e5b3755d283020ba8f0eddc5b8/packages/loot-core/src/server/schedules/app.ts#L377) - [packages\/loot-core\/src\/server\/schedules\/find-schedules.ts\#L19](https://github.com/actualbudget/actual/blob/164dd399b0a5b2e5b3755d283020ba8f0eddc5b8/packages/loot-core/src/server/schedules/find-schedules.ts#L19) - `error TS2339: Property 'occurrences' does not exist on type 'Schedule'.` - [packages\/loot-core\/src\/server\/schedules\/app.ts\#L380](https://github.com/actualbudget/actual/blob/164dd399b0a5b2e5b3755d283020ba8f0eddc5b8/packages/loot-core/src/server/schedules/app.ts#L380) - [packages\/loot-core\/src\/server\/schedules\/find-schedules.ts\#L21](https://github.com/actualbudget/actual/blob/164dd399b0a5b2e5b3755d283020ba8f0eddc5b8/packages/loot-core/src/server/schedules/find-schedules.ts#L21)

iisaduan commented 1 week ago

@typescript-bot pack this

typescript-bot commented 1 week ago

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

Command	Status	Results
`pack this`	✅ Started	✅ Results

typescript-bot commented 1 week ago

Hey @iisaduan, 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/161623/artifacts?artifactName=tgz&fileId=0A48EC0B9A929F09DB33203B3AEB701FC87DBA101F38C8F5801425F36DA6779D02&fileName=/typescript-5.5.0-insiders.20240506.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-58326-8".;

iisaduan commented 1 week ago

Testing results analysis: The extra errors correctly identify wrong code.

✅@types/karma-chai-sinon: Sinon already contains export as namespace sinon, so declare global { var Sinon } is correctly identified as a duplicate identifier on line 9
✅ @types/bandagedb_dbapi: similar-- lodash has `export as namespace , anddeclare global { const : typeof ; }` on line 9 is redundant
✅ @types/regenerator-runtime: export as namespace regeneratorRuntime makes declare global { var regeneratorRuntime: typeof import("."); } redundant on lines 2-4
✅ actualbudget/actual: this is similar to the issue in the original bug. This package imports both @rschedule/core and @rschedule/json-tools. json-tools attempts to extend core. In json-tools, the declarations of namespace Schedule¹⁴ and class Schedule<Data = any> extends OccurrenceGenerator²⁷ clash with the export class Schedule<Data = any> extends ScheduleBase<Data> ²³ in core

microsoft / TypeScript

fix53287 `mergeSymbol` checks if the resolved target can merge with the source #58326

tsc

actualbudget/actual