Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix getEffectiveCheckNode #60309

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix getEffectiveCheckNode #60309

wants to merge 1 commit into from

Conversation

gabritto
Copy link
Member

@gabritto gabritto commented Oct 22, 2024

getEffectiveCheckNode is currently used to adjust error position by skipping outer expressions of a node.
This fixes the following problems in error position related to usage of getEffectiveCheckNode.

Nested satisfies are not skipped:

((): { a: true } => (({}) satisfies unknown) satisfies unknown)(); // Error on `satisfies`, should be on `{}`

Playground

In JS files, JSDoc type assertions are skipped:

/**
 * @template T
 * @param {T|undefined} value value or not
 * @returns {T} result value
 */
const foo1 = value => /** @type {string} */({ ...value }); // Error on `{ ...value }`, should be on `({ ...value })`. Small difference.

/**
 * @template T
 * @param {T|undefined} value value or not
 * @returns {T} result value
 */
const foo2 = value => /** @type {string} */(/** @type {T} */({ ...value })); // Error on `{ ...value }`, should be on `(/** @type {T} */({ ...value }))`, which is closer to the cast.

Playground

Found those problems when working on another PR.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Oct 22, 2024
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@gabritto
Copy link
Member Author

@typescript-bot test it

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 22, 2024

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
Copy link
Collaborator

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

Everything looks good!

@typescript-bot
Copy link
Collaborator

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

Everything looks the same!

You can check the log here.

@typescript-bot
Copy link
Collaborator

@gabritto
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 31 31 ~ ~ ~ p=1.000 n=6
Symbols 62,340 62,340 ~ ~ ~ p=1.000 n=6
Types 50,379 50,379 ~ ~ ~ p=1.000 n=6
Memory used 194,017k (± 0.92%) 193,993k (± 0.93%) ~ 192,803k 196,348k p=0.810 n=6
Parse Time 1.31s (± 1.01%) 1.32s (± 0.62%) ~ 1.31s 1.33s p=0.546 n=6
Bind Time 0.72s 0.72s ~ ~ ~ p=1.000 n=6
Check Time 9.75s (± 0.33%) 9.74s (± 0.24%) ~ 9.70s 9.77s p=0.517 n=6
Emit Time 2.73s (± 0.55%) 2.73s (± 0.46%) ~ 2.71s 2.74s p=0.607 n=6
Total Time 14.51s (± 0.26%) 14.51s (± 0.16%) ~ 14.47s 14.53s p=0.371 n=6
angular-1 - node (v18.15.0, x64)
Errors 33 33 ~ ~ ~ p=1.000 n=6
Symbols 947,886 947,886 ~ ~ ~ p=1.000 n=6
Types 410,840 410,840 ~ ~ ~ p=1.000 n=6
Memory used 1,224,586k (± 0.00%) 1,224,627k (± 0.00%) ~ 1,224,548k 1,224,719k p=0.378 n=6
Parse Time 6.66s (± 1.02%) 6.62s (± 0.65%) ~ 6.58s 6.69s p=0.295 n=6
Bind Time 1.88s (± 0.29%) 1.88s (± 0.34%) ~ 1.87s 1.89s p=0.201 n=6
Check Time 31.81s (± 0.34%) 31.88s (± 0.22%) ~ 31.79s 31.97s p=0.170 n=6
Emit Time 15.11s (± 0.27%) 15.14s (± 0.24%) ~ 15.09s 15.18s p=0.228 n=6
Total Time 55.48s (± 0.18%) 55.52s (± 0.09%) ~ 55.45s 55.58s p=0.521 n=6
mui-docs - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 2,494,373 2,494,373 ~ ~ ~ p=1.000 n=6
Types 908,290 908,290 ~ ~ ~ p=1.000 n=6
Memory used 2,306,658k (± 0.00%) 2,306,709k (± 0.00%) +50k (+ 0.00%) 2,306,648k 2,306,739k p=0.020 n=6
Parse Time 9.32s (± 0.21%) 9.29s (± 0.26%) ~ 9.25s 9.32s p=0.052 n=6
Bind Time 2.15s (± 0.70%) 2.15s (± 0.62%) ~ 2.13s 2.17s p=0.931 n=6
Check Time 74.91s (± 0.38%) 74.95s (± 0.41%) ~ 74.54s 75.33s p=0.873 n=6
Emit Time 0.29s (± 1.41%) 0.28s (± 3.04%) 🟩-0.01s (- 4.62%) 0.27s 0.29s p=0.018 n=6
Total Time 86.67s (± 0.33%) 86.67s (± 0.34%) ~ 86.27s 86.99s p=1.000 n=6
self-build-src - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,258,033 1,258,045 +12 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,263 266,265 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 3,146,670k (± 0.02%) 3,146,670k (± 0.02%) ~ 3,145,713k 3,147,321k p=1.000 n=6
Parse Time 6.61s (± 1.16%) 6.55s (± 0.98%) ~ 6.48s 6.67s p=0.128 n=6
Bind Time 2.29s (± 4.17%) 2.30s (± 1.23%) ~ 2.26s 2.34s p=0.334 n=6
Check Time 42.95s (± 0.49%) 43.01s (± 0.41%) ~ 42.82s 43.28s p=0.936 n=6
Emit Time 3.53s (± 1.59%) 3.49s (± 1.57%) ~ 3.40s 3.57s p=0.336 n=6
Total Time 55.39s (± 0.45%) 55.35s (± 0.27%) ~ 55.20s 55.55s p=0.575 n=6
self-build-src-public-api - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 1,258,033 1,258,045 +12 (+ 0.00%) ~ ~ p=0.001 n=6
Types 266,263 266,265 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 2,496,444k (± 0.02%) 2,496,333k (± 0.01%) ~ 2,496,156k 2,496,554k p=0.689 n=6
Parse Time 5.42s (± 0.40%) 5.37s (± 0.52%) -0.05s (- 0.95%) 5.33s 5.40s p=0.013 n=6
Bind Time 1.74s (± 0.30%) 1.73s (± 0.47%) ~ 1.72s 1.74s p=0.523 n=6
Check Time 35.85s (± 0.18%) 35.86s (± 0.17%) ~ 35.77s 35.92s p=0.810 n=6
Emit Time 3.04s (± 1.94%) 3.07s (± 0.93%) ~ 3.04s 3.12s p=0.191 n=6
Total Time 46.04s (± 0.10%) 46.03s (± 0.15%) ~ 45.93s 46.10s p=0.809 n=6
self-compiler - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 261,789 261,790 +1 (+ 0.00%) ~ ~ p=0.001 n=6
Types 106,511 106,513 +2 (+ 0.00%) ~ ~ p=0.001 n=6
Memory used 438,830k (± 0.01%) 438,817k (± 0.01%) ~ 438,726k 438,873k p=1.000 n=6
Parse Time 3.54s (± 0.94%) 3.54s (± 0.64%) ~ 3.50s 3.56s p=0.936 n=6
Bind Time 1.32s (± 1.35%) 1.32s (± 0.92%) ~ 1.31s 1.34s p=0.864 n=6
Check Time 18.93s (± 0.36%) 18.97s (± 0.34%) ~ 18.87s 19.03s p=0.295 n=6
Emit Time 1.54s (± 1.09%) 1.54s (± 0.54%) ~ 1.53s 1.55s p=0.621 n=6
Total Time 25.33s (± 0.29%) 25.35s (± 0.22%) ~ 25.27s 25.43s p=0.688 n=6
ts-pre-modules - node (v18.15.0, x64)
Errors 68 68 ~ ~ ~ p=1.000 n=6
Symbols 225,919 225,919 ~ ~ ~ p=1.000 n=6
Types 94,415 94,415 ~ ~ ~ p=1.000 n=6
Memory used 371,070k (± 0.00%) 371,086k (± 0.01%) ~ 371,035k 371,160k p=0.520 n=6
Parse Time 2.91s (± 1.03%) 2.90s (± 0.60%) ~ 2.87s 2.92s p=0.627 n=6
Bind Time 1.58s (± 1.86%) 1.60s (± 1.16%) ~ 1.58s 1.62s p=0.120 n=6
Check Time 16.39s (± 0.32%) 16.39s (± 0.52%) ~ 16.24s 16.47s p=0.872 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 20.88s (± 0.31%) 20.89s (± 0.49%) ~ 20.72s 20.99s p=0.748 n=6
vscode - node (v18.15.0, x64)
Errors 3 3 ~ ~ ~ p=1.000 n=6
Symbols 3,124,554 3,124,554 ~ ~ ~ p=1.000 n=6
Types 1,077,073 1,077,073 ~ ~ ~ p=1.000 n=6
Memory used 3,217,541k (± 0.01%) 3,217,368k (± 0.01%) ~ 3,216,782k 3,217,898k p=0.810 n=6
Parse Time 14.07s (± 0.23%) 14.10s (± 0.31%) ~ 14.02s 14.14s p=0.147 n=6
Bind Time 4.46s (± 0.31%) 4.48s (± 0.46%) +0.03s (+ 0.60%) 4.46s 4.51s p=0.043 n=6
Check Time 87.07s (± 2.24%) 85.88s (± 1.88%) ~ 84.02s 87.99s p=0.261 n=6
Emit Time 26.29s (± 6.60%) 26.61s (± 3.57%) ~ 25.21s 27.49s p=0.378 n=6
Total Time 131.89s (± 1.64%) 131.09s (± 1.68%) ~ 129.04s 133.78s p=0.423 n=6
webpack - node (v18.15.0, x64)
Errors 0 0 ~ ~ ~ p=1.000 n=6
Symbols 286,375 286,375 ~ ~ ~ p=1.000 n=6
Types 116,078 116,078 ~ ~ ~ p=1.000 n=6
Memory used 437,158k (± 0.02%) 437,035k (± 0.02%) ~ 436,907k 437,156k p=0.066 n=6
Parse Time 5.05s (± 0.24%) 5.09s (± 0.44%) +0.05s (+ 0.96%) 5.06s 5.11s p=0.003 n=6
Bind Time 2.18s (± 1.02%) 2.15s (± 1.31%) ~ 2.12s 2.19s p=0.051 n=6
Check Time 22.90s (± 0.46%) 23.02s (± 1.21%) ~ 22.84s 23.58s p=0.520 n=6
Emit Time 0.00s 0.00s ~ ~ ~ p=1.000 n=6
Total Time 30.13s (± 0.41%) 30.26s (± 0.98%) ~ 30.04s 30.85s p=0.419 n=6
xstate-main - node (v18.15.0, x64)
Errors 3 3 ~ ~ ~ p=1.000 n=6
Symbols 543,130 543,130 ~ ~ ~ p=1.000 n=6
Types 181,889 181,889 ~ ~ ~ p=1.000 n=6
Memory used 485,534k (± 0.01%) 485,547k (± 0.02%) ~ 485,448k 485,673k p=0.936 n=6
Parse Time 4.17s (± 0.47%) 4.18s (± 0.36%) ~ 4.16s 4.20s p=0.607 n=6
Bind Time 1.45s (± 1.38%) 1.46s (± 0.93%) ~ 1.45s 1.48s p=0.437 n=6
Check Time 23.69s (± 0.43%) 23.75s (± 0.19%) ~ 23.69s 23.80s p=0.173 n=6
Emit Time 0.00s (±244.70%) 0.00s (±244.70%) ~ 0.00s 0.01s p=1.000 n=6
Total Time 29.31s (± 0.36%) 29.38s (± 0.21%) ~ 29.32s 29.46s p=0.065 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 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@typescript-bot
Copy link
Collaborator

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

Everything looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

2 participants