Add tests, also fix accessors while we're here

Author: Gerrit0

src/lib/converter/plugins/CommentPlugin.ts

@@ -203,26 +203,6 @@ export class CommentPlugin extends ConverterComponent {
         this.removeExcludedTags(comment);
     }
 
-    private getHiddenItems(reflection: Reflection) {
-        const hiddenItems = new Set<Reflection>();
-        if (reflection.kindOf(ReflectionKind.FunctionOrMethod)) {
-            const decl = reflection as DeclarationReflection;
-            if (decl.signatures) {
-                for (const sig of decl.signatures) {
-                    if (!sig.comment) {
-                        hiddenItems.add(sig);
-                    }
-                }
-            }
-            if (!decl.signatures?.some((sig) => sig.comment)) {
-                hiddenItems.add(decl);
-            }
-        } else {
-            hiddenItems.add(reflection);
-        }
-        return hiddenItems || [];
-    }
-
     /**
      * Triggered when the converter begins resolving a project.
      *
@@ -245,31 +225,29 @@ export class CommentPlugin extends ConverterComponent {
             }
 
             if (this.isHidden(ref)) {
-                this.getHiddenItems(ref).forEach((item) => hidden.add(item));
+                hidden.add(ref);
             }
         }
         hidden.forEach((reflection) => project.removeReflection(reflection));
 
         // remove functions with empty signatures after their signatures have been removed
         const [allRemoved, someRemoved] = partition(
-            filterMap(hidden, (reflection) =>
-                reflection.parent?.kindOf(
-                    ReflectionKind.FunctionOrMethod | ReflectionKind.Constructor
-                )
-                    ? reflection.parent
-                    : void 0
-            ) as DeclarationReflection[],
-            (method) => method.signatures?.length === 0
+            unique(
+                filterMap(hidden, (reflection) =>
+                    reflection.parent?.kindOf(ReflectionKind.SignatureContainer)
+                        ? reflection.parent
+                        : void 0
+                ) as DeclarationReflection[]
+            ),
+            (method) => method.getNonIndexSignatures().length === 0
         );
         allRemoved.forEach((reflection) => {
             project.removeReflection(reflection);
         });
         someRemoved.forEach((reflection) => {
-            reflection.sources = unique(
-                reflection.signatures!.flatMap<SourceReference>(
-                    (s) => s.sources ?? []
-                )
-            );
+            reflection.sources = reflection
+                .getNonIndexSignatures()
+                .flatMap<SourceReference>((s) => s.sources ?? []);
         });
     }
 
@@ -425,26 +403,36 @@ export class CommentPlugin extends ConverterComponent {
 
         if (!comment) {
             if (this.excludeNotDocumented) {
-                // Only allow excludeNotDocumented to exclude root level reflections
-                if (!(reflection instanceof DeclarationReflection)) {
+                // Don't let excludeNotDocumented remove parameters.
+                if (
+                    !(reflection instanceof DeclarationReflection) &&
+                    !(reflection instanceof SignatureReflection)
+                ) {
                     return false;
                 }
 
                 // excludeNotDocumented should hide a module only if it has no visible children
                 if (reflection.kindOf(ReflectionKind.SomeModule)) {
-                    if (!reflection.children) {
+                    if (!(reflection as DeclarationReflection).children) {
                         return true;
                     }
-                    return reflection.children.every((child) =>
-                        this.isHidden(child)
-                    );
+                    return (
+                        reflection as DeclarationReflection
+                    ).children!.every((child) => this.isHidden(child));
                 }
 
                 // enum members should all be included if the parent enum is documented
                 if (reflection.kind === ReflectionKind.EnumMember) {
                     return false;
                 }
 
+                // signature containers should only be hidden if all their signatures are hidden
+                if (reflection.kindOf(ReflectionKind.SignatureContainer)) {
+                    return (reflection as DeclarationReflection)
+                        .getAllSignatures()
+                        .every((child) => this.isHidden(child));
+                }
+
                 // excludeNotDocumented should never hide parts of "type" reflections
                 return inTypeLiteral(reflection) === false;
             }

src/lib/models/reflections/kind.ts

@@ -93,4 +93,11 @@ export namespace ReflectionKind {
         ReflectionKind.Constructor |
         ReflectionKind.Function |
         ReflectionKind.Method;
+
+    /**
+     * Note: This does not include Class/Interface, even though they technically could contain index signatures
+     * @internal
+     */
+    export const SignatureContainer =
+        ContainsCallSignatures | ReflectionKind.Accessor;
 }

src/test/converter2.test.ts

@@ -78,7 +78,7 @@ describe("Converter2", () => {
         } else {
             runTest(
                 name,
-                issueTests[`pre${entry}`],
+                issueTests[`pre${entry[0].toUpperCase()}${entry.slice(1)}`],
                 join("issues", entry),
                 check as (
                     project: ProjectReflection,

src/test/converter2/issues/gh1994.ts

@@ -0,0 +1,39 @@
+/**
+ * Has docs
+ */
+export function documented() {}
+
+/**
+ * Some signatures with docs
+ */
+export function documented2(): void;
+export function documented2(x: number): number;
+export function documented2(x?: number) {
+    return x;
+}
+
+export function notDocumented() {}
+
+/** Docs */
+export class Docs {
+    /** Docs */
+    get x() {
+        return 1;
+    }
+    /** Docs */
+    set x(value: number) {
+        throw value;
+    }
+
+    /** Docs */
+    get y() {
+        return 2;
+    }
+    set y(value: number) {
+        throw value;
+    }
+
+    get z() {
+        return 3;
+    }
+}

src/test/issueTests.ts

@@ -608,4 +608,20 @@ export const issueTests: {
         logger.discardDebugMessages();
         logger.expectNoOtherMessages();
     },
+
+    preGh1994(app) {
+        app.options.setValue("excludeNotDocumented", true);
+    },
+    gh1994(project) {
+        for (const exp of ["documented", "documented2", "Docs.x", "Docs.y"]) {
+            query(project, exp);
+        }
+        for (const rem of ["notDocumented", "Docs.z"]) {
+            ok(!project.getChildByName(rem));
+        }
+
+        const y = query(project, "Docs.y");
+        ok(y.getSignature);
+        ok(!y.setSignature);
+    },
 };