Skip to content

Commit

Permalink
Use parent comment class when reviving
Browse files Browse the repository at this point in the history 
Resolves #2622
  • Loading branch information
@Gerrit0
Gerrit0 committed Jul 13, 2024
1 parent 794d4ae commit 59b11bd
Show file tree
Hide file tree
Showing 3 changed files with 171 additions and 100 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Bug Fixes

- Constructor parameters which share a name with a property on a parent class will no longer inherit the comment on the parent class, #2636.
- Packages mode will now attempt to use the comment declared in the comment class for inherited members, #2622.

## v0.26.4 (2024-07-10)

Expand Down
227 changes: 127 additions & 100 deletions src/lib/converter/plugins/ImplementsPlugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type { TranslatedString } from "../../internationalization/internationali
export class ImplementsPlugin extends ConverterComponent {
private resolved = new WeakSet<Reflection>();
private postponed = new WeakMap<Reflection, Set<DeclarationReflection>>();
private revivingSerialized = false;

/**
* Create a new ImplementsPlugin instance.
Expand All @@ -44,7 +45,7 @@ export class ImplementsPlugin extends ConverterComponent {
this.onSignature.bind(this),
1000,
);
this.application.on(ApplicationEvents.REVIVE, this.resolve.bind(this));
this.application.on(ApplicationEvents.REVIVE, this.onRevive.bind(this));
}

/**
Expand All @@ -55,7 +56,7 @@ export class ImplementsPlugin extends ConverterComponent {
classReflection: DeclarationReflection,
interfaceReflection: DeclarationReflection,
) {
handleInheritedComments(classReflection, interfaceReflection);
this.handleInheritedComments(classReflection, interfaceReflection);
if (!interfaceReflection.children) {
return;
}
Expand Down Expand Up @@ -109,7 +110,7 @@ export class ImplementsPlugin extends ConverterComponent {
}
}

handleInheritedComments(classMember, interfaceMember);
this.handleInheritedComments(classMember, interfaceMember);
});
}

Expand All @@ -130,7 +131,7 @@ export class ImplementsPlugin extends ConverterComponent {
);

for (const parent of extendedTypes) {
handleInheritedComments(reflection, parent.reflection);
this.handleInheritedComments(reflection, parent.reflection);

for (const parentMember of parent.reflection.children ?? []) {
const child = findMatchingMember(parentMember, reflection);
Expand All @@ -157,7 +158,7 @@ export class ImplementsPlugin extends ConverterComponent {
project,
);

handleInheritedComments(child, parentMember);
this.handleInheritedComments(child, parentMember);
}
}
}
Expand All @@ -167,6 +168,12 @@ export class ImplementsPlugin extends ConverterComponent {
this.resolve(context.project);
}

private onRevive(project: ProjectReflection) {
this.revivingSerialized = true;
this.resolve(project);
this.revivingSerialized = false;
}

private resolve(project: ProjectReflection) {
for (const id in project.reflections) {
const refl = project.reflections[id];
Expand Down Expand Up @@ -362,6 +369,105 @@ export class ImplementsPlugin extends ConverterComponent {
}
}
}

/**
* Responsible for copying comments from "parent" reflections defined
* in either a base class or implemented interface to the child class.
*/
private handleInheritedComments(
child: DeclarationReflection,
parent: DeclarationReflection,
) {
this.copyComment(child, parent);

if (
parent.kindOf(ReflectionKind.Property) &&
child.kindOf(ReflectionKind.Accessor)
) {
if (child.getSignature) {
this.copyComment(child.getSignature, parent);
child.getSignature.implementationOf = child.implementationOf;
}
if (child.setSignature) {
this.copyComment(child.setSignature, parent);
child.setSignature.implementationOf = child.implementationOf;
}
}
if (
parent.kindOf(ReflectionKind.Accessor) &&
child.kindOf(ReflectionKind.Accessor)
) {
if (parent.getSignature && child.getSignature) {
this.copyComment(child.getSignature, parent.getSignature);
}
if (parent.setSignature && child.setSignature) {
this.copyComment(child.setSignature, parent.setSignature);
}
}

if (
parent.kindOf(ReflectionKind.FunctionOrMethod) &&
parent.signatures &&
child.signatures
) {
for (const [cs, ps] of zip(child.signatures, parent.signatures)) {
this.copyComment(cs, ps);
}
} else if (
parent.kindOf(ReflectionKind.Property) &&
parent.type instanceof ReflectionType &&
parent.type.declaration.signatures &&
child.signatures
) {
for (const [cs, ps] of zip(
child.signatures,
parent.type.declaration.signatures,
)) {
this.copyComment(cs, ps);
}
}
}

/**
* Copy the comment of the source reflection to the target reflection with a JSDoc style copy
* function. The TSDoc copy function is in the InheritDocPlugin.
*/
private copyComment(target: Reflection, source: Reflection) {
if (!shouldCopyComment(target, source, this.revivingSerialized)) {
return;
}

target.comment = source.comment!.clone();

if (
target instanceof DeclarationReflection &&
source instanceof DeclarationReflection
) {
for (const [tt, ts] of zip(
target.typeParameters || [],
source.typeParameters || [],
)) {
this.copyComment(tt, ts);
}
}
if (
target instanceof SignatureReflection &&
source instanceof SignatureReflection
) {
for (const [tt, ts] of zip(
target.typeParameters || [],
source.typeParameters || [],
)) {
this.copyComment(tt, ts);
}
for (const [pt, ps] of zip(
target.parameters || [],
source.parameters || [],
)) {
this.copyComment(pt, ps);
}
}
}
}

function constructorInheritance(
Expand Down Expand Up @@ -419,7 +525,9 @@ function createLink(
}

// Intentionally create broken links here. These will be replaced with real links during
// resolution if we can do so.
// resolution if we can do so. We create broken links rather than real links because in the
// case of an inherited symbol, we'll end up referencing a single symbol ID rather than one
// for each class.
function link(
target: DeclarationReflection | SignatureReflection | undefined,
) {
Expand Down Expand Up @@ -448,111 +556,30 @@ function createLink(
}
}

/**
* Responsible for copying comments from "parent" reflections defined
* in either a base class or implemented interface to the child class.
*/
function handleInheritedComments(
child: DeclarationReflection,
parent: DeclarationReflection,
function shouldCopyComment(
target: Reflection,
source: Reflection,
revivingSerialized: boolean,
) {
copyComment(child, parent);

if (
parent.kindOf(ReflectionKind.Property) &&
child.kindOf(ReflectionKind.Accessor)
) {
if (child.getSignature) {
copyComment(child.getSignature, parent);
child.getSignature.implementationOf = child.implementationOf;
}
if (child.setSignature) {
copyComment(child.setSignature, parent);
child.setSignature.implementationOf = child.implementationOf;
}
}
if (
parent.kindOf(ReflectionKind.Accessor) &&
child.kindOf(ReflectionKind.Accessor)
) {
if (parent.getSignature && child.getSignature) {
copyComment(child.getSignature, parent.getSignature);
}
if (parent.setSignature && child.setSignature) {
copyComment(child.setSignature, parent.setSignature);
}
if (!source.comment) {
return false;
}

if (
parent.kindOf(ReflectionKind.FunctionOrMethod) &&
parent.signatures &&
child.signatures
) {
for (const [cs, ps] of zip(child.signatures, parent.signatures)) {
copyComment(cs, ps);
}
} else if (
parent.kindOf(ReflectionKind.Property) &&
parent.type instanceof ReflectionType &&
parent.type.declaration.signatures &&
child.signatures
) {
for (const [cs, ps] of zip(
child.signatures,
parent.type.declaration.signatures,
)) {
copyComment(cs, ps);
if (target.comment) {
// If we're reviving, then the revived project might have a better comment
// on source, so copy it.
if (revivingSerialized && source.comment.similarTo(target.comment)) {
return true;
}
}
}

/**
* Copy the comment of the source reflection to the target reflection with a JSDoc style copy
* function. The TSDoc copy function is in the InheritDocPlugin.
*/
function copyComment(target: Reflection, source: Reflection) {
if (target.comment) {
// We might still want to copy, if the child has a JSDoc style inheritDoc tag.
const tag = target.comment.getTag("@inheritDoc");
if (!tag || tag.name) {
return;
return false;
}
}

if (!source.comment) {
return;
}

target.comment = source.comment.clone();

if (
target instanceof DeclarationReflection &&
source instanceof DeclarationReflection
) {
for (const [tt, ts] of zip(
target.typeParameters || [],
source.typeParameters || [],
)) {
copyComment(tt, ts);
}
}
if (
target instanceof SignatureReflection &&
source instanceof SignatureReflection
) {
for (const [tt, ts] of zip(
target.typeParameters || [],
source.typeParameters || [],
)) {
copyComment(tt, ts);
}
for (const [pt, ps] of zip(
target.parameters || [],
source.parameters || [],
)) {
copyComment(pt, ps);
}
}
return true;
}

function findMatchingMember(
Expand Down
43 changes: 43 additions & 0 deletions src/lib/models/comments/comment.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,20 @@ export class CommentTag {
this.content = text;
}

/**
* Checks if this block tag is roughly equal to the other tag.
* This isn't exactly equal, but just "roughly equal" by the tag
* text.
*/
similarTo(other: CommentTag) {
return (
this.tag === other.tag &&
this.name === other.tag &&
Comment.combineDisplayParts(this.content) ===
Comment.combineDisplayParts(other.content)
);
}

clone(): CommentTag {
const tag = new CommentTag(
this.tag,
Expand Down Expand Up @@ -420,6 +434,35 @@ export class Comment {
extractLabelTag(this);
}

/**
* Checks if this comment is roughly equal to the other comment.
* This isn't exactly equal, but just "roughly equal" by the comment
* text.
*/
similarTo(other: Comment): boolean {
if (
Comment.combineDisplayParts(this.summary) !==
Comment.combineDisplayParts(other.summary)
) {
return false;
}

// Ignore modifier tags, as they could cause false negatives
// if a cascaded modifier tag is present in one comment but not the other.

if (this.blockTags.length !== other.blockTags.length) {
return false;
}

for (let i = 0; i < this.blockTags.length; ++i) {
if (!this.blockTags[i].similarTo(other.blockTags[i])) {
return false;
}
}

return true;
}

/**
* Create a deep clone of this comment.
*/
Expand Down

0 comments on commit 59b11bd

Please sign in to comment.