Correct behavior of excludeNotDocumented

Resolves #1752 Resolves #1948
TypeStrong · Jun 4, 2022 · 7918fba · 7918fba
1 parent 3edb447
commit 7918fba
Show file tree

Hide file tree

Showing 9 changed files with 1,232 additions and 170 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ These TODOs will be resolved before a full release. ([GitHub project](https://gi
 
 -   Node 12 is no longer officially supported as it has gone end of life as of 2022-04-30. It might still work, but may stop working at any time.
 -   Dropped support for TypeScript before 4.6.
+-   TypeDoc will now produce warnings for bracketed links (`[[ target ]]`). Use `{@link target}` instead. The `{@link}` syntax will be recognized by [TypeScript 4.3](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-3.html#editor-support-for-link-tags) and later and used to provide better intellisense. TypeDoc version 0.24.0 will remove support for `[[ target ]]` style links.
 -   `extends` in typedoc.json is now resolved using NodeJS module resolution, so a local path must begin with `./`.
 -   In the JSON output for `DeclarationReflection`s, `getSignature` is no longer a one-tuple.
 -   In the JSON output for `DeclarationReflection`s, `setSignature` is no longer a one-tuple.
@@ -23,7 +24,6 @@ These TODOs will be resolved before a full release. ([GitHub project](https://gi
 -   The deprecated `listInvalidSymbolLinks` option has been removed. Use `validation.invalidLink` instead.
 -   The deprecated `true` and `false` values have been removed from `--emit`, to migrate replace `true` with `"both"` and `false` with `"docs"`.
 -   Links are no longer be resolved against a global list of all symbols. See [the documentation](https://typedoc.org/guides/link-resolution/) for details on link resolution.
--   TypeDoc will now produce warnings for bracketed links (`[[ target ]]`). Use `{@link target}` instead. The `{@link}` syntax will be recognized by [TypeScript 4.3](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-3.html#editor-support-for-link-tags) and later and used to provide better intellisense. TypeDoc version 0.24.0 will remove support for `[[ target ]]` style links.
 -   The `validation.invalidLink` option is now on by default.
 -   `reflection.decorates`, `reflection.decorators`, and their corresponding interfaces have been removed as no code in TypeDoc used them.
 -   The shape of the `Comment` class has changed significantly to support multiple tag kinds.
@@ -68,6 +68,8 @@ These TODOs will be resolved before a full release. ([GitHub project](https://gi
 -   TypeDoc will now automatically inherit documentation from classes `implements` by other interfaces/classes.
 -   Fixed `@inheritDoc` on accessors, #1927.
 -   JS exports defined as `exports.foo = ...` will now be converted as variables rather than properties.
+-   The `excludeNotDocumented` option will no longer hide a module if it has a documentation comment, #1948.
+-   Prevent `--excludeNotDocumented` from hiding properties of type literals (`a` in `function fn(p: { a: string })`), #1752.
 -   Corrected schema generation for https://typedoc.org/schema.json
 
 ### Thanks!

diff --git a/src/lib/converter/context.ts b/src/lib/converter/context.ts
@@ -240,7 +240,7 @@ export class Context {
     }
 
     shouldIgnore(symbol: ts.Symbol) {
-        return this.converter.shouldIgnore(symbol, this.checker);
+        return this.converter.shouldIgnore(symbol);
     }
 
     /**

diff --git a/src/lib/converter/converter.ts b/src/lib/converter/converter.ts
@@ -12,7 +12,6 @@ import { convertSymbol } from "./symbols";
 import { createMinimatch, matchesAny } from "../utils/paths";
 import type { IMinimatch } from "minimatch";
 import { hasAllFlags, hasAnyFlag } from "../utils/enum";
-import { resolveAliasedSymbol } from "./utils/symbols";
 import type { DocumentationEntryPoint } from "../utils/entry-point";
 import type { CommentParserConfig } from "./comments";
 
@@ -224,11 +223,9 @@ export class Converter extends ChildableComponent<
 
         const allExports = getExports(context, node, symbol);
 
-        if (
-            allExports.every((exp) => this.shouldIgnore(exp, context.checker))
-        ) {
+        if (allExports.every((exp) => this.shouldIgnore(exp))) {
             this.owner.logger.verbose(
-                `Ignoring entry point ${entryName} since all members will be ignored.`
+                `All members of ${entryName} are excluded, ignoring entry point.`
             );
             return;
         }
@@ -292,28 +289,18 @@ export class Converter extends ChildableComponent<
         this.trigger(Converter.EVENT_RESOLVE_END, context);
     }
 
-    /** @internal */
-    shouldIgnore(symbol: ts.Symbol, checker: ts.TypeChecker) {
-        if (
-            this.excludeNotDocumented &&
-            // If the enum is included, we should include members even if not documented.
-            !hasAllFlags(symbol.flags, ts.SymbolFlags.EnumMember) &&
-            resolveAliasedSymbol(symbol, checker).getDocumentationComment(
-                checker
-            ).length === 0
-        ) {
-            return true;
-        }
-
+    /**
+     * Used to determine if we should immediately bail when creating a reflection.
+     * Note: This should not be used for excludeNotDocumented because we don't have enough
+     * information at this point since comment discovery hasn't happened.
+     * @internal
+     */
+    shouldIgnore(symbol: ts.Symbol) {
         if (this.isExcluded(symbol)) {
             return true;
         }
 
-        if (!this.excludeExternals) {
-            return false;
-        }
-
-        return this.isExternal(symbol);
+        return this.excludeExternals && this.isExternal(symbol);
     }
 
     private isExcluded(symbol: ts.Symbol) {

diff --git a/src/lib/converter/plugins/CommentPlugin.ts b/src/lib/converter/plugins/CommentPlugin.ts
@@ -63,6 +63,9 @@ export class CommentPlugin extends ConverterComponent {
     @BindOption("excludeProtected")
     excludeProtected!: boolean;
 
+    @BindOption("excludeNotDocumented")
+    excludeNotDocumented!: boolean;
+
     /**
      * Create a new CommentPlugin instance.
      */
@@ -375,7 +378,7 @@ export class CommentPlugin extends ConverterComponent {
      *
      * @param reflection Reflection to check if hidden
      */
-    private isHidden(reflection: Reflection) {
+    private isHidden(reflection: Reflection): boolean {
         const comment = reflection.comment;
 
         if (
@@ -393,6 +396,30 @@ export class CommentPlugin extends ConverterComponent {
         }
 
         if (!comment) {
+            if (this.excludeNotDocumented) {
+                // Only allow excludeNotDocumented to exclude root level reflections
+                if (!(reflection instanceof DeclarationReflection)) {
+                    return false;
+                }
+
+                // excludeNotDocumented should hide a module only if it has no visible children
+                if (reflection.kindOf(ReflectionKind.SomeModule)) {
+                    if (!reflection.children) {
+                        return true;
+                    }
+                    return reflection.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;
+                }
+
+                // excludeNotDocumented should never hide parts of "type" reflections
+                return inTypeLiteral(reflection) === false;
+            }
             return false;
         }
 
@@ -404,6 +431,16 @@ export class CommentPlugin extends ConverterComponent {
     }
 }
 
+function inTypeLiteral(refl: Reflection | undefined) {
+    while (refl) {
+        if (refl.kind === ReflectionKind.TypeLiteral) {
+            return true;
+        }
+        refl = refl.parent;
+    }
+    return false;
+}
+
 // Moves tags like `@param foo.bar docs for bar` into the `bar` property of the `foo` parameter.
 function moveNestedParamTags(comment: Comment, parameter: ParameterReflection) {
     const visitor: Partial<TypeVisitor> = {

diff --git a/src/lib/converter/symbols.ts b/src/lib/converter/symbols.ts
@@ -336,8 +336,7 @@ function convertTypeAlias(
         context.finalizeDeclarationReflection(reflection);
 
         // Do this after finalization so that the CommentPlugin can get @typeParam tags
-        // from the parent comment. Ugly, but works for now. Should be cleaned up with TSDoc
-        // support.
+        // from the parent comment. Ugly, but works for now. Should be cleaned up eventually.
         reflection.typeParameters = declaration.typeParameters?.map((param) =>
             createTypeParamReflection(param, context.withScope(reflection))
         );

diff --git a/src/test/converter/exports/no-doc-members.ts b/src/test/converter/exports/no-doc-members.ts
@@ -0,0 +1,6 @@
+/**
+ * This module has documentation, but no exported members do.
+ * @see https://github.com/TypeStrong/typedoc/issues/1948
+ * @module
+ */
+export const abc = 123;
diff --git a/src/test/converter/exports/specs.json b/src/test/converter/exports/specs.json
@@ -12,7 +12,7 @@
       "flags": {},
       "children": [
         {
-          "id": 46,
+          "id": 48,
           "name": "GH1453Helper",
           "kind": 8388608,
           "kindString": "Reference",
@@ -28,7 +28,7 @@
           "target": 35
         },
         {
-          "id": 39,
+          "id": 41,
           "name": "Mod",
           "kind": 8388608,
           "kindString": "Reference",
@@ -44,7 +44,7 @@
           "target": 29
         },
         {
-          "id": 41,
+          "id": 43,
           "name": "Mod2",
           "kind": 8388608,
           "kindString": "Reference",
@@ -68,7 +68,7 @@
           "target": 29
         },
         {
-          "id": 40,
+          "id": 42,
           "name": "ModDefault",
           "kind": 8388608,
           "kindString": "Reference",
@@ -84,7 +84,7 @@
           "target": 36
         },
         {
-          "id": 45,
+          "id": 47,
           "name": "ThisModule",
           "kind": 8388608,
           "kindString": "Reference",
@@ -100,7 +100,7 @@
           "target": 34
         },
         {
-          "id": 42,
+          "id": 44,
           "name": "a",
           "kind": 8388608,
           "kindString": "Reference",
@@ -116,7 +116,7 @@
           "target": 30
         },
         {
-          "id": 43,
+          "id": 45,
           "name": "b",
           "kind": 8388608,
           "kindString": "Reference",
@@ -140,7 +140,7 @@
           "target": 31
         },
         {
-          "id": 38,
+          "id": 40,
           "name": "c",
           "kind": 8388608,
           "kindString": "Reference",
@@ -398,14 +398,14 @@
         {
           "title": "References",
           "children": [
-            46,
-            39,
+            48,
             41,
-            40,
-            45,
-            42,
             43,
-            38
+            42,
+            47,
+            44,
+            45,
+            40
           ]
         },
         {
@@ -853,6 +853,95 @@
           "url": "https://github.com/TypeStrong/typedoc/blob/fake/src/test/converter/exports/mod.ts#L8"
         }
       ]
+    },
+    {
+      "id": 38,
+      "name": "no-doc-members",
+      "kind": 2,
+      "kindString": "Module",
+      "flags": {},
+      "comment": {
+        "summary": [
+          {
+            "kind": "text",
+            "text": "This module has documentation, but no exported members do."
+          }
+        ],
+        "blockTags": [
+          {
+            "tag": "@see",
+            "content": [
+              {
+                "kind": "text",
+                "text": "https://github.com/TypeStrong/typedoc/issues/1948"
+              }
+            ]
+          }
+        ]
+      },
+      "children": [
+        {
+          "id": 39,
+          "name": "abc",
+          "kind": 32,
+          "kindString": "Variable",
+          "flags": {
+            "isConst": true
+          },
+          "comment": {
+            "summary": [
+              {
+                "kind": "text",
+                "text": "This module has documentation, but no exported members do."
+              }
+            ],
+            "blockTags": [
+              {
+                "tag": "@see",
+                "content": [
+                  {
+                    "kind": "text",
+                    "text": "https://github.com/TypeStrong/typedoc/issues/1948"
+                  }
+                ]
+              },
+              {
+                "tag": "@module",
+                "content": []
+              }
+            ]
+          },
+          "sources": [
+            {
+              "fileName": "no-doc-members.ts",
+              "line": 6,
+              "character": 13,
+              "url": "https://github.com/TypeStrong/typedoc/blob/fake/src/test/converter/exports/no-doc-members.ts#L6"
+            }
+          ],
+          "type": {
+            "type": "literal",
+            "value": 123
+          },
+          "defaultValue": "123"
+        }
+      ],
+      "groups": [
+        {
+          "title": "Variables",
+          "children": [
+            39
+          ]
+        }
+      ],
+      "sources": [
+        {
+          "fileName": "no-doc-members.ts",
+          "line": 6,
+          "character": 0,
+          "url": "https://github.com/TypeStrong/typedoc/blob/fake/src/test/converter/exports/no-doc-members.ts#L6"
+        }
+      ]
     }
   ],
   "groups": [
@@ -863,7 +952,8 @@
         1,
         6,
         8,
-        29
+        29,
+        38
       ]
     }
   ]
-Original file line number
+Diff line change
@@ Expand Up / @@ -240,7 +240,7 @@ export class Context { @@
         }
         shouldIgnore(symbol: ts.Symbol) {
-            return this.converter.shouldIgnore(symbol, this.checker);
+            return this.converter.shouldIgnore(symbol);
         }
         /**
@@ Expand Down @@