[api-documenter] Function overloads sometimes are displayed with the same name

[AlexJerabek]

Our office-js d.ts file has many instances of overloaded functions. For example, AppointmentCompose contains
addFileAttachmentAsync(uri: string, attachmentName: string): void;
addFileAttachmentAsync(uri: string, attachmentName: string, options: Office.AsyncContextOptions): void;
addFileAttachmentAsync(uri: string, attachmentName: string, callback: (result: AsyncResult<string>) => void): void;
addFileAttachmentAsync(uri: string, attachmentName: string, options?: Office.AsyncContextOptions, callback?: (result: AsyncResult<string>) => void): void;

Currently, only one overload (the sequentially first one) makes it to the json. We would like all of them to.

Thanks!

[octogonz]

I'm workin' on it. This will definitely be supported in API Extractor 7. It's a nontrivial undertaking though. :-)

[AlexJerabek]

Excellent! Thank you Pete!

[octogonz]

This is now working in PR #951 for the Markdown documentation generator. Still need to test the YAML output.

For an example output, see DocClass1.exampleFunction.

[AlexJerabek]

Tested with @microsoft/api-extractor: "7.0.13" and @microsoft/api-documenter: "7.0.19"

The overloaded functions show up, which is great. However, because the parameter names are used instead of the parameter type, the yaml names are often the same. Here's an example screenshot:

Having the types in the name would alleviate this problem (and look like .NET APIs, like this one).

[AlexJerabek]

Tagging @iclanton for awareness.

[octogonz]

Using types in the name is problematic because in TypeScript the type expressions can be arbitrarily large. In the worst case it can be an entire interface declaration including other nested interface declarations.

Offhand I can't think of a straightforward solution, other than manually annotating them similar to the @label tag proposed in TSDoc. ( https://github.com/Microsoft/tsdoc/blob/master/spec/code-snippets/DeclarationReferences.ts )

If we can come up with a straightforward design, the implementation should be easy.

[AlexJerabek]

How adverse are we to having the tool change the parameter names in these overload scenarios? The easiest thing would be to add a "2" or something.

Perhaps parameter name could have the type appended to it if a duplicate name is detected. Maybe this only works for "simple" types (leaving "complex" types with the original name). That wouldn't help the case where there's "complex" types for multiple overloads, but it'd be a start. I guess you could also just use the first parsable token in the "complex" type (e.g. "Function" or "Array") as the name addition.

[octogonz]

I think numeric suffixes are the most natural user experience: calculate(calculationType) vs calculate2(calculationType)

But there isn't a stable way to automatically assign these numbers. For example, if someone moves the code around, or adds/removes overloads, the indexes would get shuffled, which potentially could break documentation references from other packages.

The proposed @label tag solves this problem for declaration references:

/**
 * {@label WITH_ENUM}
 */
function calculate(calculationType: CalculationType): void;

/**
 * {@label WITH_STRINGS}
 */
function calculate(calculationType: "Recalculate" | "Full"): void;

/**
 * Please see {@link calculate:WITH_ENUM} or {@link calculate:WITH_STRINGS}
 * for more information.
 *
 * We proposed that autonumbering would also be supported, but at your own risk:
 *
 * Please see {@link calculate:1} or {@link calculate:2}
 * for more information.
 */
function referenceToCalculate(): void;

We didn't consider this, but I suppose @label could be used to explicitly specify the integers:

/**
 * {@label 1}
 */
function calculate(calculationType: CalculationType): void;

/**
 * {@label 2}
 */
function calculate(calculationType: "Recalculate" | "Full"): void;

/**
 * Please see {@link calculate:1} or {@link calculate:2}
 * for more information.
 */
function referenceToCalculate(): void;

If we reused this idea, then api-documenter could generate titles such as:

• calculate:1(calculationType) and calculate:2(calculationType)
• calculate:WITH_ENUM(calculationType) and calculate:WITH_STRINGS(calculationType)

If the : is too confusingly similar to a TypeScript type annotation, we could map it to some other symbol. (You can imagine variations such as calculate[1](calculationType] or somesuch.)

But if you look at DeclarationReferences.ts you will find all sorts of other nasty cases that will cause trouble, e.g. ECMAScript symbols, identifiers containing spaces and other arbitrary characters, etc. The declaration reference notation is our best attempt to handle all those cases (after a ton of deliberation and backtracking), so I feel that the design of the YAML names will inevitably eventually arrive at the same outcome.

@dend for awareness

[octogonz]

Another idea would be to report an error/warning and force you to rename one of your parameters to make the names unique. What would you prefer to see on your web site?

A. renaming a parameter

calculate(calculationType)

function calculate(calculationType: CalculationType): void;

calculate(calculationTypeString)

function calculate(calculationTypeString: "Recalculate" | "Full"): void;

OR:

B. label selectors

calculate:ENUM

function calculate(calculationType: CalculationType): void;

calculate:STRING

function calculate(calculationType: "Recalculate" | "Full"): void;

OR:

C. index selectors

calculate:1

function calculate(calculationType: CalculationType): void;

calculate:2

function calculate(calculationType: "Recalculate" | "Full"): void;

[AlexJerabek]

Thanks for the proposals @octogonz.

I discussed the possibilities with my team and we're leaning towards Option A. Having the tool provide a warning for overloaded functions with identical names seems like the most agreeable solution. That encourages good parameter naming and doesn't requires a unique syntax.

[octogonz]

Hmm.. it occurs to me that this same problem will arise with other types of merged declarations (e.g. ApiItemContainerMixin on this page is simultaneously a function, an interface, and a namespace.) In declaration reference notation, they would be distinguished as ApiItemContainerMixin:function, ApiItemContainerMixin:interface, and ApiItemContainerMixin:namespace. So options B and C are definitely the more general solution.

But I agree that option A looks more friendly to a documentation audience. Perhaps we should treat overloaded functions as a special case. From a technical perspective, the documentation system doesn't care too much how we format the titles, as long as distinct things get mapped to unique names.

So we could do something like this:

• Function that's not overloaded: functionName
• Overloaded function (following option A): functionName(arg1, arg2)
• General declaration reference: ApiItemContainerMixin (function)

We can probably experiment with different refinements without breaking things too much, since the rule for generating URLs and doc IDs is separate. (Although it probably needs to be reconsidered with this feature as well.)

[octogonz]

Bumping this issue. Now that I'm done with AE7 (hurray!) I should hopefully have some time to get to this. It's an easy change.

[octogonz]

This issue belongs to a family of closely related issues. I've created metaissue #1308 to come up with a unified fix that tackles them all together.