Skip to content

Improve @overload's interactions with constructors #52577

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

Merged
merged 1 commit into from
Feb 2, 2023

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Feb 2, 2023

  1. Get @overload return type for constructors in getReturnTypeOfSignature
  2. No bogus implicit-any error on new calls in checkCallExpression
  3. No bogus implicit-any error in constructor @overloads in getSignaturesOfSymbol

Fixes #52477 (no bug for (3), which I discovered in the process of fixing (1) and (2))

1. Get `@overload` return type for constructors in getReturnTypeOfSignature
2. No bogus implicit-any error on `new` calls in checkCallExpression
3. No bogus implicit-any error in constructor `@overload`s in getSignaturesOfSymbol
@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Feb 2, 2023
Comment on lines +23 to +24
*/
/**
Copy link
Member

Choose a reason for hiding this comment

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

Does this test need to be written in such a way as to satisfy #52474?

Copy link
Member Author

@sandersn sandersn Feb 2, 2023

Choose a reason for hiding this comment

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

You mean putting all the tags into a single comment? No, this test showed that that requirement isn't feasible; the parser can't distinguish between the end of a return-less @overload and the beginning of the implementation's @params. So I reverted it and once again @overload is unlike all other jsdoc comments in checking all preceding /** comments.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, so #52474 is no longer a thing, okay.

Comment on lines +33780 to 33781
!(isJSDocSignature(declaration) && getJSDocRoot(declaration)?.parent?.kind === SyntaxKind.Constructor) &&
!isJSDocConstructSignature(declaration) &&
Copy link
Member

Choose a reason for hiding this comment

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

Is isJSDocConstructSignature supposed to do the check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Confusingly, JSDocConstructSignature refers to closure type syntax: function(new: SomeClass) is equivalent to TS new (): SomeClass.

The naming scheme could be better though.

@DanielRosenwasser
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 2, 2023

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 57bd245. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 2, 2023

Hey @DanielRosenwasser, 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/145222/artifacts?artifactName=tgz&fileId=09F9A96CA0B755F016683366DA413949CFC8656248B98D4FBB7A199FA82E080C02&fileName=/typescript-5.0.0-insiders.20230202.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.0.0-pr-52577-2".;

@sandersn sandersn merged commit 0d251ba into microsoft:main Feb 2, 2023
@sandersn sandersn deleted the fix-constructor-overload-tag branch February 2, 2023 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@overload doesn't work for constructors
4 participants