Skip to content

Infer from parameters annotated using JSDoc #55400

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 28, 2023
Merged

Infer from parameters annotated using JSDoc #55400

merged 2 commits into from
Aug 28, 2023

Conversation

Andarist
Copy link
Contributor

No description provided.

Andarist
Andarist commented Aug 16, 2023
View reviewed changes
tests/cases/compiler/contravariantOnlyInferenceFromAnnotatedFunctionJs.ts Outdated
Copy link
Contributor Author

@Andarist Andarist Aug 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an existing test case, just rewritten to JSDoc, see: https://github.com/microsoft/TypeScript/pull/52609/files#diff-85ed3085ca9f5d4c893196581ea0785e5d6a5aca5ae449afe499fb4d4d159728

Andarist
Andarist commented Aug 16, 2023
View reviewed changes
src/compiler/checker.ts Outdated
@@ -35117,11 +35117,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const len = signature.parameters.length - (signatureHasRestParameter(signature) ? 1 : 0);
for (let i = 0; i < len; i++) {
const declaration = signature.parameters[i].valueDeclaration as ParameterDeclaration;
if (declaration.type) {
Copy link
Contributor Author

@Andarist Andarist Aug 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this check is somewhat redundant because getEffectiveTypeAnnotationNode already resolves this and that it actually prevents this function from working on functions annotated using JSDoc

@DanielRosenwasser
Copy link
Member

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Aug 24, 2023 •
edited
Loading

Heya @DanielRosenwasser, I've started to run the regular perf test suite on this PR at 6be3219. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

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

Here they are:

Compiler

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Angular - node (v16.17.1, x64)
Memory used 300,260k (± 0.00%) 300,263k (± 0.00%) ~ 300,247k 300,270k p=0.470 n=6
Parse Time 3.03s (± 0.27%) 3.03s (± 0.17%) ~ 3.02s 3.03s p=0.929 n=6
Bind Time 0.93s (± 0.00%) 0.93s (± 0.00%) ~ 0.93s 0.93s p=1.000 n=6
Check Time 9.29s (± 0.27%) 9.30s (± 0.25%) ~ 9.27s 9.34s p=0.624 n=6
Emit Time 7.63s (± 0.16%) 7.61s (± 0.21%) ~ 7.59s 7.64s p=0.063 n=6
Total Time 20.88s (± 0.17%) 20.87s (± 0.16%) ~ 20.83s 20.92s p=0.872 n=6
Compiler-Unions - node (v16.17.1, x64)
Memory used 193,907k (± 0.01%) 193,904k (± 0.01%) ~ 193,890k 193,919k p=0.630 n=6
Parse Time 1.58s (± 0.26%) 1.58s (± 0.00%) ~ 1.58s 1.58s p=0.405 n=6
Bind Time 0.80s (± 0.00%) 0.80s (± 0.51%) ~ 0.79s 0.80s p=0.405 n=6
Check Time 9.96s (± 0.37%) 9.94s (± 0.24%) ~ 9.90s 9.97s p=0.517 n=6
Emit Time 2.74s (± 0.23%) 2.73s (± 0.20%) ~ 2.73s 2.74s p=0.201 n=6
Total Time 15.08s (± 0.30%) 15.05s (± 0.14%) ~ 15.02s 15.08s p=0.469 n=6
Monaco - node (v16.17.1, x64)
Memory used 347,139k (± 0.01%) 347,149k (± 0.01%) ~ 347,099k 347,171k p=0.873 n=6
Parse Time 2.69s (± 0.15%) 2.69s (± 0.23%) ~ 2.68s 2.70s p=0.673 n=6
Bind Time 0.99s (± 0.00%) 0.99s (± 0.00%) ~ 0.99s 0.99s p=1.000 n=6
Check Time 7.93s (± 0.23%) 7.92s (± 0.39%) ~ 7.89s 7.96s p=0.328 n=6
Emit Time 4.26s (± 0.32%) 4.26s (± 0.12%) ~ 4.25s 4.26s p=0.541 n=6
Total Time 15.87s (± 0.13%) 15.85s (± 0.22%) ~ 15.81s 15.89s p=0.683 n=6
TFS - node (v16.17.1, x64)
Memory used 301,146k (± 0.01%) 301,155k (± 0.01%) ~ 301,133k 301,181k p=0.574 n=6
Parse Time 2.18s (± 0.84%) 2.17s (± 0.48%) ~ 2.16s 2.18s p=0.142 n=6
Bind Time 1.12s (± 1.04%) 1.11s (± 0.73%) ~ 1.11s 1.13s p=0.340 n=6
Check Time 7.24s (± 0.27%) 7.26s (± 0.41%) ~ 7.22s 7.31s p=0.464 n=6
Emit Time 3.98s (± 0.57%) 3.98s (± 0.54%) ~ 3.95s 4.01s p=0.746 n=6
Total Time 14.53s (± 0.19%) 14.52s (± 0.32%) ~ 14.46s 14.57s p=1.000 n=6
material-ui - node (v16.17.1, x64)
Memory used 479,406k (± 0.00%) 479,416k (± 0.00%) ~ 479,411k 479,427k p=0.064 n=6
Parse Time 3.15s (± 0.13%) 3.16s (± 0.26%) ~ 3.15s 3.17s p=0.056 n=6
Bind Time 0.91s (± 0.00%) 0.91s (± 0.00%) ~ 0.91s 0.91s p=1.000 n=6
Check Time 17.75s (± 0.34%) 17.84s (± 0.30%) +0.09s (+ 0.50%) 17.75s 17.90s p=0.030 n=6
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) ~ 0.00s 0.00s p=1.000 n=6
Total Time 21.81s (± 0.26%) 21.90s (± 0.22%) +0.10s (+ 0.44%) 21.83s 21.96s p=0.024 n=6
xstate - node (v16.17.1, x64)
Memory used 542,855k (± 0.01%) 542,869k (± 0.01%) ~ 542,790k 542,971k p=0.575 n=6
Parse Time 3.70s (± 0.20%) 3.70s (± 0.20%) ~ 3.69s 3.71s p=0.487 n=6
Bind Time 1.40s (± 4.72%) 1.36s (± 3.60%) ~ 1.34s 1.46s p=0.652 n=6
Check Time 3.25s (± 2.29%) 3.34s (± 2.46%) ~ 3.17s 3.39s p=0.054 n=6
Emit Time 0.08s (± 0.00%) 0.08s (± 0.00%) ~ 0.08s 0.08s p=1.000 n=6
Total Time 8.44s (± 0.14%) 8.47s (± 0.44%) ~ 8.41s 8.52s p=0.065 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • Angular - node (v16.17.1, x64)
  • Compiler-Unions - node (v16.17.1, x64)
  • Monaco - node (v16.17.1, x64)
  • TFS - node (v16.17.1, x64)
  • material-ui - node (v16.17.1, x64)
  • xstate - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

tsserver

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
Compiler-UnionsTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,489ms (± 0.09%) 2,492ms (± 0.10%) ~ 2,489ms 2,495ms p=0.124 n=6
Req 2 - geterr 5,949ms (± 0.27%) 5,950ms (± 0.14%) ~ 5,943ms 5,965ms p=0.872 n=6
Req 3 - references 343ms (± 0.43%) 345ms (± 1.03%) ~ 342ms 352ms p=0.217 n=6
Req 4 - navto 277ms (± 0.20%) 278ms (± 0.81%) ~ 276ms 282ms p=0.342 n=6
Req 5 - completionInfo count 1,356 (± 0.00%) 1,356 (± 0.00%) ~ 1,356 1,356 p=1.000 n=6
Req 5 - completionInfo 89ms (± 8.59%) 82ms (±10.11%) ~ 76ms 93ms p=0.139 n=6
CompilerTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,635ms (± 0.55%) 2,619ms (± 0.66%) ~ 2,604ms 2,645ms p=0.196 n=6
Req 2 - geterr 4,775ms (± 0.14%) 4,773ms (± 0.17%) ~ 4,758ms 4,782ms p=0.574 n=6
Req 3 - references 350ms (± 0.15%) 350ms (± 0.30%) ~ 348ms 351ms p=0.794 n=6
Req 4 - navto 269ms (± 0.19%) 270ms (± 0.33%) ~ 269ms 271ms p=0.190 n=6
Req 5 - completionInfo count 1,518 (± 0.00%) 1,518 (± 0.00%) ~ 1,518 1,518 p=1.000 n=6
Req 5 - completionInfo 79ms (± 0.52%) 79ms (± 0.65%) ~ 79ms 80ms p=0.595 n=6
xstateTSServer - node (v16.17.1, x64)
Req 1 - updateOpen 2,717ms (± 0.17%) 2,717ms (± 0.19%) ~ 2,707ms 2,721ms p=0.936 n=6
Req 2 - geterr 1,918ms (± 2.08%) 1,934ms (± 2.19%) ~ 1,879ms 1,971ms p=0.127 n=6
Req 3 - references 130ms (± 9.16%) 138ms (± 2.57%) ~ 134ms 141ms p=0.250 n=6
Req 4 - navto 354ms (± 0.39%) 353ms (± 0.47%) ~ 351ms 355ms p=0.217 n=6
Req 5 - completionInfo count 2,071 (± 0.00%) 2,071 (± 0.00%) ~ 2,071 2,071 p=1.000 n=6
Req 5 - completionInfo 314ms (± 2.07%) 315ms (± 2.24%) ~ 309ms 325ms p=0.869 n=6
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • CompilerTSServer - node (v16.17.1, x64)
  • Compiler-UnionsTSServer - node (v16.17.1, x64)
  • xstateTSServer - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Startup

Comparison Report - baseline..pr
Metric baseline pr Delta Best Worst p-value
tsc-startup - node (v16.17.1, x64)
Execution time 155.65ms (± 0.14%) 155.58ms (± 0.17%) -0.07ms (- 0.04%) 154.38ms 158.76ms p=0.000 n=600
tsserver-startup - node (v16.17.1, x64)
Execution time 230.86ms (± 0.15%) 230.77ms (± 0.16%) -0.09ms (- 0.04%) 229.67ms 237.50ms p=0.000 n=600
tsserverlibrary-startup - node (v16.17.1, x64)
Execution time 234.90ms (± 0.12%) 235.99ms (± 0.14%) +1.08ms (+ 0.46%) 234.66ms 239.45ms p=0.000 n=600
typescript-startup - node (v16.17.1, x64)
Execution time 236.06ms (± 0.11%) 235.92ms (± 0.11%) -0.15ms (- 0.06%) 234.52ms 239.17ms p=0.000 n=600
System info unknown
Hosts
  • node (v16.17.1, x64)
Scenarios
  • tsc-startup - node (v16.17.1, x64)
  • tsserver-startup - node (v16.17.1, x64)
  • tsserverlibrary-startup - node (v16.17.1, x64)
  • typescript-startup - node (v16.17.1, x64)
Benchmark Name Iterations
Current pr 6
Baseline baseline 6

Developer Information:

Download Benchmarks

@sandersn sandersn merged commit 418494e into microsoft:main Aug 28, 2023
@Andarist Andarist deleted the fix/infer-from-annotated-parameters-in-js-files branch August 29, 2023 12:23
snovader pushed a commit to EG-A-S/TypeScript that referenced this pull request Sep 23, 2023
@Andarist @snovader
Infer from parameters annotated using JSDoc (microsoft#55400)
77cee24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sandersn sandersn sandersn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Andarist @DanielRosenwasser @typescript-bot @sandersn