Redesign option for require-jsdoc

[golopot]

The option for require-jsdoc is hard to use. It requires knowledge of ESTree. And it is not useful for this rule to make distinction between FunctionDeclaration, FunctionExpression, and ArrowFunctionExpression, conceptually there are just functions, whether it is arrow function does not affect whether it needs jsdoc comment.

A more user friendly option will be:

{
  requires: {
    functions: boolean,
    methods: boolean,
    classes: boolean,
  }
}

This would be a breaking change.

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[brettz9]

While they are indeed all functions, projects often use them for distinct purposes.

For example, some use arrow functions only within contexts like forEach, thus perhaps making them less desired for documentation by some projects.

Likewise do some use function expressions (or arrow functions) to denote functions for nested utilities that may be of less significance then those used as declarations.

While I would support additional shorthand options, I think we should preserve the old distinctive options for the above reason.

[golopot]

Likewise do some use function expressions (or arrow functions) to denote functions for nested utilities that may be of less significance then those used as declarations.

The schema you suggested seemed prone to abuse, it encourages people to write function in different syntax as a way to circumvent this rule.

It is not a good idea to hint the significance of a function by writing it in different syntax. There is no clear map between the chosen syntax and the significance of the function. The syntax chosen is usually decided by other considerations. In case like module.exports = function f() {}, choosing syntax FunctionExpression does not mean the function is not significant. In cases of arrow functions using lexical this, the choice of syntax has nothing to do with the significance of the function. In fact, the choice is mostly decided by consistency and personal preference.

Likewise do some use function expressions (or arrow functions) to denote functions for nested utilities that may be of less significance then those used as declarations.

Doing so is a shotgun that affects many other cases:

// ignore `FunctionExpression` because of this case:
const utils = {
  f(){},
  g(){}
}

// will also ignore these cases:
module.exports = function f() {}
module.exports = { f() {} }
exports.f = function f() {}

For example, some use arrow functions only within contexts like forEach, ...

This is a non-issue since function used like callback are ignored by this rule.

[brettz9]

The schema you suggested seemed prone to abuse, it encourages people to write function in different syntax as a way to circumvent this rule.

You speak about linting rules as though they are always meant to catch bad guys sneaking in bad code, whereas projects often seek it to legitimately make their lives easier. They simply don't want their code bloated with what they view as unnecessary comments. The interest in #192 on restricting to public exports is one example of this, but not the only one.

It is not a good idea to hint the significance of a function by writing it in different syntax.

That is what is done with https://eslint.org/docs/rules/prefer-arrow-callback though, as you note, it may not be affected here. However, as far as your comment about being "prone to abuse", would that not, per your view, encourage people to circumvent writing documented functions by stuffing everything into an inline callback rather than having the callback be defined at a higher level (e.g., outside of a loop)?

If one wishes to be more exacting (or draconian and distrustful as the case may be), require-jsdoc should be changed to check these functions as well (and I'd be ok with that as an option). I think we should learn from history here in why jshint garnered some support over jslint because of the latter being over-opinionated. However rational a project's decisions, projects come to different conclusions, and there are most often trade-offs and not always one "right" way.

In cases of arrow functions using lexical this, the choice of syntax has nothing to do with the significance of the function. In fact, the choice is mostly decided by consistency and personal preference.

I agree it can be decided by preference, and that is why it is good to let projects prefer how they do it, including allowing them to enforce a mapping to significance.

You do have a good point on the use of function expressions on object literals, but I think this argues for more control (e.g., the context of those expressions), rather than less. Likewise with exports (though in some projects and certain cases, it may instead be preferred to prohibit such usage entirely; for example, a project prohibiting anonymous default exports (see https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-anonymous-default-export.md for an already-existing ES6 import version, though the same could be made for CommonJS exports) along with enforcing the module.exports style: https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/exports-style.md ).

[brettz9]

I think one thing we're both missing here is the fact that Selectors (driven within eslint by esquery) are pretty powerful and users can already add them as contexts.

For example, esquery already has a shortcut for :function to cover various function types (which we should probably just use in examples here), so that could really catch these.

module.exports = function f() {}
module.exports = { f() {} }
exports.f = function f() {}

Or if one preferred, these expressions can also be quite explicit, so for the first one, for example, one could target only this particular syntax with:

ExpressionStatement[expression.type='AssignmentExpression'][expression.left.type='MemberExpression'][expression.left.object.name='module'][expression.left.property.name='exports'][expression.right.type='FunctionExpression']

...or the second type with the following (could probably be shortened):

ExpressionStatement[expression.type='AssignmentExpression'][expression.left.type='MemberExpression'][expression.left.property.name='exports'][expression.left.object.name='module'][expression.right.type='ObjectExpression'][expression.right.properties] > AssignmentExpression > ObjectExpression > Property > FunctionExpression

...or the third type with:

ExpressionStatement[expression.type='AssignmentExpression'][expression.left.type='MemberExpression'][expression.left.object.name='exports'][expression.right.type='FunctionExpression']

(Expressions as usual derived with help from https://astexplorer.net/ .)