Use parent comment class when reviving

Gerrit0 · Gerrit0 · commit 59b11bdf1530 · 2024-07-13T10:05:12.000-06:00
Resolves #2622
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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)
 
diff --git a/src/lib/converter/plugins/ImplementsPlugin.ts b/src/lib/converter/plugins/ImplementsPlugin.ts
@@ -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.
@@ -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));
     }
 
     /**
@@ -55,7 +56,7 @@ export class ImplementsPlugin extends ConverterComponent {
         classReflection: DeclarationReflection,
         interfaceReflection: DeclarationReflection,
     ) {
-        handleInheritedComments(classReflection, interfaceReflection);
+        this.handleInheritedComments(classReflection, interfaceReflection);
         if (!interfaceReflection.children) {
             return;
         }
@@ -109,7 +110,7 @@ export class ImplementsPlugin extends ConverterComponent {
                 }
             }
 
-            handleInheritedComments(classMember, interfaceMember);
+            this.handleInheritedComments(classMember, interfaceMember);
         });
     }
 
@@ -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);
@@ -157,7 +158,7 @@ export class ImplementsPlugin extends ConverterComponent {
                         project,
                     );
 
-                    handleInheritedComments(child, parentMember);
+                    this.handleInheritedComments(child, parentMember);
                 }
             }
         }
@@ -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];
@@ -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(
@@ -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,
     ) {
@@ -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(
diff --git a/src/lib/models/comments/comment.ts b/src/lib/models/comments/comment.ts
@@ -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,
@@ -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.
      */
