Skip to content

Have createUnionOfSignaturesForOverloadFailure() combine @deprecated tags properly #54872

@voxpelli

Description

@voxpelli

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, from 4.0.5 and 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.

Skärmavbild 2023-07-03 kl  12 40 41

🙂 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:

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)

Metadata

Metadata

Assignees

No one assigned

    Labels

    Help WantedYou can do thisPossible ImprovementThe current behavior isn't wrong, but it's possible to see that it might be better in some cases

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions