Return checks are matched against a bodyless type signature, not the function body implementation

[remcohaszing]

Expected behavior

@returns is allowed when a function has multiple type signatures, but the implementation has a return statement.

Actual behavior

jsdoc/require-returns-check disallows the @returns

ESLint Config

module.exports = {
  rules: {
    'jsdoc/require-returns-check': 'error'
  }
}

ESLint sample

/**
 * Read a test fixture.
 *
 * @param path The path to resolve relative to the fixture base. It will be normalized for the
 * operating system.
 * @param encoding The text encoding of the file.
 * @returns The file contents as a string or buffer.
 */
export function readFixture(path: string): Promise<Buffer>;
export function readFixture(path: string, encoding: BufferEncoding): Promise<string>;
export function readFixture(path: string, encoding?: BufferEncoding): Promise<Buffer | string> {
  return readFile(resolveFixture(path), encoding);
}

This also affects jsdoc/check-param-names.

Environment

• Node version: 18.7.0
• ESLint version: 8.22.0
• eslint-plugin-jsdoc version: 39.3.6 (It worked fine in version 39.3.5)

[brettz9]

So, it seems you are asking for an exception to the normal behavior of our rules only checking maxLines with a default of 1 (and minLines of default of 0) to discover the appropriate comment. You want the comment discovery mechanism to skip over any intervening bodyless signatures. I think that is reasonable.

However, before addressing that issue, we really need to indicate what should be done for the reporting of bodyless type signatures since the comment above them might be seen as applying to them as well.

If we allow them to pass validation when they have a return type annotation, require-returns-check will no longer report the cases you have as problematic (though under this proposal, it will report them if the return type annotation is missing regardless of whether the subsequent body implementation has a return). Does that sound reasonable or do you think return checking should be skipped for such bodyless type signatures even if a return type annotation is missing? I think it would get too complicated for us to have different behaviors here depending on whether a body implementation existed or not.

Likewise, what to do with require-returns? If a return type annotation is present on a bodyless type signature, but a comment block doesn't have a @returns, report it?

[alejandroclaro]

I agree with the worng/unexpected behavior. This is about function overloading and happends even when each overload is documented:

/**
 * Reads a test fixture.
 *
 * @param path The path to resolve relative to the fixture base. It will be normalized for the
 * operating system.
 *
 * @returns The file contents as buffer.
 */
export function readFixture(path: string): Promise<Buffer>;

/**
 * Reads a test fixture.
 *
 * @param path The path to resolve relative to the fixture base. It will be normalized for the
 * operating system.
 * @param encoding The text encoding of the file.
 *
 * @returns The file contents as a string.
 */
export function readFixture(path: string, encoding: BufferEncoding): Promise<string>;

/**
 * Reads a test fixture.
 *
 * @param path The path to resolve relative to the fixture base. It will be normalized for the
 * operating system.
 * @param encoding The optional text encoding of the file.
 *
 * @returns The file contents as a string or buffer.
 */
export function readFixture(path: string, encoding?: BufferEncoding): Promise<Buffer | string> {
  return readFile(resolveFixture(path), encoding);
}

and it was working well in previous versions.

[brettz9]

@alejandroclaro : Please see my comment at #903 (comment) . As of 39.3.6, we started checking for TSDeclareFunction for function-type rules, and I think that is desirable but we need to determine how bodyless signatures are satisfied for this rule (and require-returns).

If a return type exists on the signature, do we want it to satisfy the return check (and only for TSDeclareFunction)? I think that makes sense, and it should avoid adding an error for most users.

[alejandroclaro]

@brettz9. Agree, at least for typescript, what makes sense to me is that the documentation matches the function's interface/signature.

If the overload returns 'void', no return description should be needed, but for other overload functions, return descriptions must be present and describe the overload case.

[brettz9]

If folks want to try out #914 , I think I've fixed the cases where the comment block is immediately above the bodyless signature. This still doesn't address the case for allowing checking of a function whose comment block is interrupted by bodyless signatures.

[alejandroclaro]

@brettz9 it LOGTM. Thanks.

When is next release planned?

[brettz9]

@alejandroclaro , I'm hoping @gajus might be able to debug why the releases aren't going out. Probably 2FA-related...

[alejandroclaro]

Thanks!

[brettz9]

Still awaiting the releases.

As far as checking a function whose comment block is interrupted by bodyless signatures, this would be a bit more complicated to implement (we'd have to find the currently iterated node within its parent container and jump ourselves past all bodyless signature AST).