-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Description
Bug Report
Currently when createUnionOfSignaturesForOverloadFailure() in src/compiler/checker.ts creates a union of signatures where one signature has @deprecated, that deprecation can propagate up and make the entire union of signatures appear as deprecated.
🔎 Search Terms
deprecated overload jsdoc
🕗 Version & Regression Information
- This is the behavior in every version I tried that has supported
@deprecated, from4.0.5and still in nightly, and I reviewed the FAQ for entries about overloading
⏯ Playground Link
Playground link with relevant code in nightly
Playground link with relevant code in4.0.5
💻 Code
const concat: {
/**
* @deprecated
*/
(foo: string, bar: string): string;
(foo: string, bar: number): string;
} = (foo, bar) => {
return foo + bar;
};
concat('abc', 'xyz'); // Shows as deprecated
concat('abc', 123);
concat(123, 123); // Wrongly shows as deprecated
const concat2: {
(foo: string, bar: string): string;
/**
* @deprecated
*/
(foo: string, bar: number): string;
} = (foo, bar) => {
return foo + bar;
};
concat2('abc', 'xyz');
concat2('abc', 123); // Shows as deprecated
concat2(123, 123); // Correctly not shown as deprecated🙁 Actual behavior
The createUnionOfSignaturesForOverloadFailure() in src/compiler/checker.ts creates a union of signatures for the overload failure, but doesn't care for merging the JSDoc tags in any ways, just picking the JSDoc tag of the first candidate.
This becomes especially clear for the @deprecated tag as it makes the created union look like its deprecated, something I ran into here: fastify/fastify#4874 (comment)
Screenshot from the above PR where I wanted to deprecate a special case overload but it made all normal type errors look deprecated as well so I had to opt out.
🙂 Expected behavior
I expect that the union of signatures for the overload failure only show up as deprecated if all of the signatures in the union are deprecated. As long as any signature isn't deprecated the union should not show up as deprecated.
🤔 Possible solution
The Signature that's returned in createUnionOfSignaturesForOverloadFailure() has a getJsDocTags() function that seems to default to a this.jsDocTags that's only available on SignatureObject not on Signature:
TypeScript/src/services/services.ts
Lines 930 to 932 in 88d59e4
| getJsDocTags(): JSDocTagInfo[] { | |
| return this.jsDocTags || (this.jsDocTags = getJsDocTagsOfDeclarations(singleElementArray(this.declaration), this.checker)); | |
| } |
If one could precalculate the JSDoc/deprecation tags for all candidates in createUnionOfSignaturesForOverloadFailure() using something like:
const jsDocTags = mapDefined(candidates, (c) => c.declaration ? getJSDocTags(c.declaration) : undefined);Or:
const deprecationTags = mapDefined(candidates, (c) => c.declaration ? getJSDocDeprecatedTag(c.declaration) : undefined);And then calculate a union of the resulting (deprecation) tags and inject the result somehow into this.jsDocTags of the Signature – through eg. createSignature() or such – then that would force the union of signatures to also have a union of at least @deprecated tags.
I tried fixing a PR for this myself, but especially since checker.ts is such a humongous file and the relation between Signature and SignatureObject in the code is a bit unclear to me I failed to come up with a proper PR. (Though I did hack it locally to verify my ideas)
