validation.notDocumented doesn't correctly handle some cases

[thislooksfun]

Search terms

function type definitions, methods with computed names, generated types

Expected Behavior

Enabling validation.notDocumented should correctly report on:

• function type definitions (type Foo = () => void)
• methods with computed names:

  const name = "foo";
  class SomeClass {
    [name]() { return "huh"; }
  }

• methods inside generated definitions used by excluded methods:

  class SomeClass {
    doSomething() {
      return {
        foo() {}
      }
    }
  }

Actual Behavior

All of the above cases falsely report that they are undocumented.

Steps to reproduce the bug

1. Clone the main branch of https://github.com/thislooksfun/typedoc-repros
2. Install deps and run npm run gendoc
3. The docs should be generated without any warning. Instead, it prints:

Warning: FunctionType, defined at index.ts:2, does not have any documentation.
Warning: [iterator], defined at index.ts:7, does not have any documentation.
Warning: index, defined at index.ts:10, does not have any documentation.
Warning: next, defined at index.ts:12, does not have any documentation.
Warning: next, defined at index.ts:28, does not have any documentation.

Environment

• Typedoc version: HEAD (98841f5)
• TypeScript version: any
• Node.js version: 16
• OS: macOS

[thislooksfun]

I also just noticed that for some incredibly strange reason it ignores @internal on every other function parameter (when passing requiredToBeDocumented contains Parameter, of course):

The following method definition:

class SomeClass {
  /** @internal */
  foobar(
    first: string,
    second: string,
    third: string,
    fourth: string,
    fifth: string,
    sixth: string,
    seventh: string,
    eighth: string
  ) {
    first;
    second;
    third;
    fourth;
    fifth;
    sixth;
    seventh;
    eighth;
  }
}

Fails with the error:

Warning: SomeClass.foobar.second, defined at src/reddit/subreddit/object.ts:429, does not have any documentation.
Warning: SomeClass.foobar.fourth, defined at src/reddit/subreddit/object.ts:431, does not have any documentation.
Warning: SomeClass.foobar.sixth, defined at src/reddit/subreddit/object.ts:433, does not have any documentation.
Warning: SomeClass.foobar.eighth, defined at src/reddit/subreddit/object.ts:435, does not have any documentation.

[Gerrit0]

Note to self, never accept a feature without a large battery of tests... I'm also being reminded of how much I dislike TypeDoc's use of reflections for object types :/

[thislooksfun]

Heh, I know the feeling. I just had to revert a nice looking change in one of my projects that completely failed in production because I didn't test it enough.

Thankfully this particular bug is in a non-critical section, and only really matters if you have both validation.notDocumented and treatWarningsAsErrors turned on, so it could be much worse.

[Gerrit0]

on every other function parameter

This is an easy bug at least. First thing I checked was the issue. TypeDoc .traverse didn't clone lists when iterating, so if we remove from the list while iterating over it, we'll end up skipping every other item.

[thislooksfun]

Yeah that'll do it! Removing things from arrays while iterating over them is so easy, it's one of my favorite things (massive /s). At least it was an easy fix. If my guess is correct the function types will be easy as well. Good luck on the other two though, they seem like they'll be a pain.

[Gerrit0]

Well, there's that fixed... hopefully this lasts longer than the other one before horrible bugs are discovered ;)

[thislooksfun]

I can confirm that my project now passes without error when run against ea16a7b! I'm probably not using every possible reflected type, but I ran with requiredToBeDocumented set to everything other than "Project" and got 0 false matches.