Skip to content

Commit

Permalink
Allow functions to have comments
Browse files Browse the repository at this point in the history
Resolves: #2521
  • Loading branch information
Gerrit0 committed May 2, 2024
1 parent e30ed7a commit d1be956
Show file tree
Hide file tree
Showing 15 changed files with 197 additions and 194 deletions.
5 changes: 3 additions & 2 deletions .github/ISSUE_TEMPLATE/bug_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ labels: bug
Note: Turn off skipErrorChecks before reporting a crash. Bug reports for crashes with that option
on are out of scope.
If possible, please create a *minimal* repo reproducing your problem and link it.
You can easily do this by submitting a pull request to https://github.com/TypeStrong/typedoc-repros
If possible, please create a *minimal* repo reproducing your problem.
If it is more than a single small file, please submit a pull request to
https://github.com/TypeStrong/typedoc-repros
which changes the files necessary to reproduce your bug.
If this is not possible, include at least:
Expand Down
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Beta

Docs needing updating:
@license, @import
sitemapBaseUrl
markedOptions -> markdownItOptions, markdownItLoader, navigation

## Breaking Changes

- Drop support for Node 16.
Expand All @@ -9,6 +14,11 @@
- Updated Shiki from 0.14 to 1.3. This should mostly be a transparent update which adds another 23 supported languages and 13 supported themes.
- Renamed `--sitemapBaseUrl` to `--hostedBaseUrl` to reflect that it can be used for more than just the sitemap.
- Removed deprecated `navigation.fullTree` option.
- All function-likes may now have comments directly attached to them. This is a change from previous versions of TypeDoc where functions comments
were always moved down to the signature level. This mostly worked, but caused problems with type aliases, so was partially changed in 0.25.13.
This change was extended to apply not only to type aliases, but also other function-likes declared with variables and callable properties.
As a part of this change, comments on the implementation signature of overloaded functions will now be added to the function reflection, and will
not be inherited by signatures of that function, #2521.
- API: `MapOptionDeclaration.mapError` has been removed.
- API: Deprecated `BindOption` decorator has been removed.
- API: `DeclarationReflection.indexSignature` has been renamed to `DeclarationReflection.indexSignatures`.
Expand Down
15 changes: 9 additions & 6 deletions src/lib/converter/comments/discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,19 +178,22 @@ export function discoverComment(
}
seen.add(node);

// Special behavior here! We temporarily put the implementation comment
// on the reflection which contains all the signatures. This lets us pull
// the comment on the implementation if some signature does not have a comment.
// However, we don't want to skip the node if it is a reference to something.
// See the gh1770 test for an example.
// Special behavior here!
// Signatures and symbols have two distinct discovery methods as of TypeDoc 0.26.
// This method discovers comments for symbols, and function-likes will only have
// a symbol comment if there is more than one signature (== more than one declaration)
// and there is a comment on the implementation signature.
if (
kind & ReflectionKind.ContainsCallSignatures &&
[
ts.SyntaxKind.FunctionDeclaration,
ts.SyntaxKind.MethodDeclaration,
ts.SyntaxKind.Constructor,
].includes(node.kind) &&
!(node as ts.FunctionDeclaration).body
(symbol.declarations!.filter((d) =>
wantedKinds[kind].includes(d.kind),
).length === 1 ||
!(node as ts.FunctionDeclaration).body)
) {
continue;
}
Expand Down
164 changes: 59 additions & 105 deletions src/lib/converter/plugins/CommentPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import {
SignatureReflection,
type ParameterReflection,
Comment,
ReflectionType,
type SourceReference,
type TypeVisitor,
CommentTag,
ReflectionType,
} from "../../models";
import {
Option,
Expand Down Expand Up @@ -359,123 +359,78 @@ export class CommentPlugin extends ConverterComponent {
movePropertyTags(reflection.comment, reflection);
}

if (!(reflection instanceof DeclarationReflection)) {
return;
if (reflection instanceof DeclarationReflection && reflection.comment) {
let sigs: SignatureReflection[];
if (reflection.type instanceof ReflectionType) {
sigs = reflection.type.declaration.getNonIndexSignatures();
} else {
sigs = reflection.getNonIndexSignatures();
}

// For variables and properties, the symbol might own the comment but we might also
// have @param and @returns comments for an owned signature. Only do this if there is
// exactly one signature as otherwise we have no hope of doing validation right.
if (sigs.length === 1 && !sigs[0].comment) {
this.moveSignatureParamComments(sigs[0], reflection.comment);
const returnsTag = reflection.comment.getTag("@returns");
if (returnsTag) {
sigs[0].comment = new Comment();
sigs[0].comment.blockTags.push(returnsTag);
reflection.comment.removeTags("@returns");
}
}
}

if (reflection.type instanceof ReflectionType) {
this.moveCommentToSignatures(
reflection,
reflection.type.declaration.getNonIndexSignatures(),
);
} else {
this.moveCommentToSignatures(
reflection,
reflection.getNonIndexSignatures(),
);
if (reflection instanceof SignatureReflection) {
this.moveSignatureParamComments(reflection);
}
}

private moveCommentToSignatures(
reflection: DeclarationReflection,
signatures: SignatureReflection[],
private moveSignatureParamComments(
signature: SignatureReflection,
comment = signature.comment,
) {
if (!signatures.length) {
return;
}

const comment = reflection.kindOf(ReflectionKind.ClassOrInterface)
? undefined
: reflection.comment;

for (const signature of signatures) {
const signatureHadOwnComment = !!signature.comment;
const childComment = (signature.comment ||= comment?.clone());
if (!childComment) continue;

signature.parameters?.forEach((parameter, index) => {
if (parameter.name === "__namedParameters") {
const commentParams = childComment.blockTags.filter(
(tag) =>
tag.tag === "@param" && !tag.name?.includes("."),
);
if (
signature.parameters?.length === commentParams.length &&
commentParams[index].name
) {
parameter.name = commentParams[index].name!;
}
}
if (!comment) return;

const tag = childComment.getIdentifiedTag(
parameter.name,
"@param",
signature.parameters?.forEach((parameter, index) => {
if (parameter.name === "__namedParameters") {
const commentParams = comment.blockTags.filter(
(tag) => tag.tag === "@param" && !tag.name?.includes("."),
);

if (tag) {
parameter.comment = new Comment(
Comment.cloneDisplayParts(tag.content),
);
}
});

for (const parameter of signature.typeParameters || []) {
const tag =
childComment.getIdentifiedTag(
parameter.name,
"@typeParam",
) ||
childComment.getIdentifiedTag(
parameter.name,
"@template",
) ||
childComment.getIdentifiedTag(
`<${parameter.name}>`,
"@param",
);
if (tag) {
parameter.comment = new Comment(
Comment.cloneDisplayParts(tag.content),
);
if (
signature.parameters?.length === commentParams.length &&
commentParams[index].name
) {
parameter.name = commentParams[index].name!;
}
}

this.validateParamTags(
signature,
childComment,
signature.parameters || [],
signatureHadOwnComment,
);
const tag = comment.getIdentifiedTag(parameter.name, "@param");

childComment?.removeTags("@param");
childComment?.removeTags("@typeParam");
childComment?.removeTags("@template");
}
if (tag) {
parameter.comment = new Comment(
Comment.cloneDisplayParts(tag.content),
);
}
});

// Since this reflection has signatures, we need to remove the comment from the non-primary
// declaration location. For functions/methods/constructors, this means removing it from
// the wrapping reflection. For type aliases, classes, and interfaces, this means removing
// it from the contained signatures... if it's the same as what is on the signature.
// This is important so that in type aliases we don't end up with a comment rendered twice.
if (reflection.kindOf(ReflectionKind.SignatureContainer)) {
delete reflection.comment;
} else {
reflection.comment?.removeTags("@param");
reflection.comment?.removeTags("@typeParam");
reflection.comment?.removeTags("@template");

const parentComment = Comment.combineDisplayParts(
reflection.comment?.summary,
);
for (const sig of signatures) {
if (
Comment.combineDisplayParts(sig.comment?.summary) ===
parentComment
) {
delete sig.comment;
}
for (const parameter of signature.typeParameters || []) {
const tag =
comment.getIdentifiedTag(parameter.name, "@typeParam") ||
comment.getIdentifiedTag(parameter.name, "@template") ||
comment.getIdentifiedTag(`<${parameter.name}>`, "@param");
if (tag) {
parameter.comment = new Comment(
Comment.cloneDisplayParts(tag.content),
);
}
}

this.validateParamTags(signature, comment, signature.parameters || []);

comment.removeTags("@param");
comment.removeTags("@typeParam");
comment.removeTags("@template");
}

private removeExcludedTags(comment: Comment) {
Expand Down Expand Up @@ -607,7 +562,6 @@ export class CommentPlugin extends ConverterComponent {
signature: SignatureReflection,
comment: Comment,
params: ParameterReflection[],
signatureHadOwnComment: boolean,
) {
const paramTags = comment.blockTags.filter(
(tag) => tag.tag === "@param",
Expand All @@ -619,7 +573,7 @@ export class CommentPlugin extends ConverterComponent {

moveNestedParamTags(/* in-out */ paramTags, params);

if (signatureHadOwnComment && paramTags.length) {
if (paramTags.length) {
for (const tag of paramTags) {
this.application.logger.warn(
this.application.i18n.signature_0_has_unused_param_with_name_1(
Expand Down
2 changes: 0 additions & 2 deletions src/lib/converter/symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,6 @@ function convertFunctionOrMethod(

const scope = context.withScope(reflection);

// Can't use zip here. We might have less declarations than signatures
// or less signatures than declarations.
for (const sig of signatures) {
createSignature(scope, ReflectionKind.CallSignature, sig, symbol);
}
Expand Down
6 changes: 4 additions & 2 deletions src/lib/models/reflections/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,10 @@ export abstract class Reflection {
* Test whether this reflection is of the given kind.
*/
kindOf(kind: ReflectionKind | ReflectionKind[]): boolean {
const kindArray = Array.isArray(kind) ? kind : [kind];
return kindArray.some((kind) => (this.kind & kind) !== 0);
const kindFlags = Array.isArray(kind)
? kind.reduce((a, b) => a | b, 0)
: kind;
return (this.kind & kindFlags) !== 0;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/lib/utils/options/sources/typedoc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,9 @@ export function addTypeDocOptions(options: Pick<Options, "addDeclaration">) {
help: (i18n) => i18n.help_markdownItOptions(),
type: ParameterType.Mixed,
configFileOnly: true,
defaultValue: {},
defaultValue: {
linkify: true,
},
validate(value, i18n) {
if (!Validation.validate({}, value)) {
throw new Error(
Expand Down
4 changes: 2 additions & 2 deletions src/lib/validation/documentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function validateDocumentation(
if (seen.has(ref)) continue;
seen.add(ref);

// If we're a non-parameter inside a parameter, we shouldn't care. Parameters don't get deeply documented
// If inside a parameter, we shouldn't care. Callback parameter's values don't get deeply documented.
let r: Reflection | undefined = ref.parent;
while (r) {
if (r.kindOf(ReflectionKind.Parameter)) {
Expand Down Expand Up @@ -71,7 +71,7 @@ export function validateDocumentation(
continue;
}

// Call signatures are considered documented if they are directly within a documented type alias.
// Construct signatures are considered documented if they are directly within a documented type alias.
if (
ref.kindOf(ReflectionKind.ConstructorSignature) &&
ref.parent?.parent?.kindOf(ReflectionKind.TypeAlias)
Expand Down
7 changes: 5 additions & 2 deletions src/test/behavior.c2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -957,8 +957,11 @@ describe("Behavior Tests", () => {
const barComments = bar.signatures?.map((sig) =>
Comment.combineDisplayParts(sig.comment?.summary),
);
equal(barComments, ["Implementation comment", "Custom comment"]);
equal(bar.comment, undefined);
equal(barComments, ["", "Custom comment"]);
equal(
Comment.combineDisplayParts(bar.comment?.summary),
"Implementation comment",
);

logger.expectMessage(
'warn: The label "bad" for badLabel cannot be referenced with a declaration reference. Labels may only contain A-Z, 0-9, and _, and may not start with a number.',
Expand Down
12 changes: 6 additions & 6 deletions src/test/converter/function/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ export function multipleSignatures(value: string): string;
export function multipleSignatures(value: { name: string }): string;

/**
* This is the actual implementation, this comment will not be visible
* in the generated documentation. The `@inheritdoc` tag can not be used
* to pull content from this signature into documentation for the real
* signatures.
*
* @return This is the return value of the function.
* This comment is on the actual implementation of the function.
* TypeDoc used to allow this for providing "default" comments that would be
* copied to each signature. It no longer does this, and instead treats
* this comment as belonging to the function reflection itself.
* Any `@param` or `@returns` tags within this comment won't be applied
* to signatures.
*/
export function multipleSignatures(): string {
if (arguments.length > 0) {
Expand Down
Loading

0 comments on commit d1be956

Please sign in to comment.