Skip to content

Commit

Permalink
Fix duplicate comments caused by signatures
Browse files Browse the repository at this point in the history
Resolves #2509
  • Loading branch information
Gerrit0 committed Mar 3, 2024
1 parent 9751e31 commit 657045c
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -3,6 +3,7 @@
### Bug Fixes

- Constructed references to enum types will be properly linked with `@interface`, #2508.
- Comments on property-methods will no longer be duplicated in generated documentation, #2509.
- Reduced rendered docs size by writing icons to a referenced SVG asset, #2505.
For TypeDoc's docs, this reduced the rendered documentation size by ~30%.
- The HTML docs now attempt to reduce repaints caused by dynamically loading the navigation, #2491.
Expand Down
24 changes: 14 additions & 10 deletions src/lib/converter/plugins/CommentPlugin.ts
Expand Up @@ -453,23 +453,27 @@ export class CommentPlugin extends ConverterComponent {
}

// Since this reflection has signatures, we need to remove the comment from the non-primary
// declaration location. For functions, this means removing it from the Function reflection.
// For type aliases, this means removing it from reflection.type.declaration.
// 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.FunctionOrMethod | ReflectionKind.Constructor,
)
) {
if (reflection.kindOf(ReflectionKind.SignatureContainer)) {
delete reflection.comment;
}
if (reflection.kindOf(ReflectionKind.TypeAlias)) {
} 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) {
delete sig.comment;
if (
Comment.combineDisplayParts(sig.comment?.summary) ===
parentComment
) {
delete sig.comment;
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/lib/output/components.ts
Expand Up @@ -29,8 +29,10 @@ export abstract class ContextAwareRendererComponent extends RendererComponent {
/**
* The url of the document that is being currently generated.
* Set when a page begins rendering.
*
* Defaulted to '.' so that tests don't have to set up events.
*/
private location!: string;
private location = ".";

/**
* Regular expression to test if a string looks like an external url.
Expand Down
15 changes: 10 additions & 5 deletions src/lib/validation/documentation.ts
Expand Up @@ -61,12 +61,13 @@ export function validateDocumentation(
toProcess.push(ref.parent);
continue;
}
// Ditto for signatures on type aliases.
// Call signatures are considered documented if they have a comment directly, or their
// container has a comment and they are directly within a type literal belonging to that container.
if (
ref.kindOf(ReflectionKind.CallSignature) &&
ref.parent?.parent?.kindOf(ReflectionKind.TypeAlias)
ref.parent?.kindOf(ReflectionKind.TypeLiteral)
) {
toProcess.push(ref.parent.parent);
toProcess.push(ref.parent.parent!);
continue;
}

Expand All @@ -78,9 +79,13 @@ export function validateDocumentation(

if (signatures.length) {
// We've been asked to validate this reflection, so we should validate that
// signatures all have comments, but we'll still have a comment here because
// type aliases always have their own comment.
// signatures all have comments
toProcess.push(...signatures);

if (ref.kindOf(ReflectionKind.SignatureContainer)) {
// Comments belong to each signature, and will not be included on this object.
continue;
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/test/converter2/issues/gh2509.ts
@@ -0,0 +1,9 @@
export interface Int {
/** Cb */
cb: () => Promise<any>;

nested: {
/** Cb2 */
cb: () => any;
};
}
24 changes: 17 additions & 7 deletions src/test/issues.c2.test.ts
Expand Up @@ -373,7 +373,7 @@ describe("Issue Tests", () => {

equal(
Comment.combineDisplayParts(
query(project, "Foo.baz").signatures?.[0]?.comment?.summary,
query(project, "Foo.baz").comment?.summary,
),
"Some property style doc.",
"Property methods declared in interface should still allow comment inheritance",
Expand Down Expand Up @@ -944,12 +944,7 @@ describe("Issue Tests", () => {
const project = convert();
const hook = query(project, "Camera.useCameraPermissions");
equal(hook.type?.type, "reflection" as const);
equal(
Comment.combineDisplayParts(
hook.type.declaration.signatures![0].comment?.summary,
),
"One",
);
equal(Comment.combineDisplayParts(hook.comment?.summary), "One");
});

it("#2150", () => {
Expand Down Expand Up @@ -1420,4 +1415,19 @@ describe("Issue Tests", () => {
equal(refl.type.toString(), "Color");
equal(refl.type.reflection?.id, query(project, "Color").id);
});

it("Does not duplicate comments due to signatures being present, #2509", () => {
const project = convert();
const cb = query(project, "Int.cb");
equal(Comment.combineDisplayParts(cb.comment?.summary), "Cb");
equal(cb.type?.type, "reflection");
equal(cb.type.declaration.signatures![0].comment, undefined);

const nested = query(project, "Int.nested");
equal(nested.type?.type, "reflection");
const cb2 = nested.type.declaration.children![0];
equal(Comment.combineDisplayParts(cb2.comment?.summary), "Cb2");
equal(cb2.type?.type, "reflection");
equal(cb2.type.declaration.signatures![0].comment, undefined);
});
});

0 comments on commit 657045c

Please sign in to comment.