require-jsdoc doesn't recognize comment blocks with newlines afterward

[jeremyruppel]

What version of ESLint are you using?

2.10.2

What parser (default, Babel-ESLint, etc.) are you using?

default

Please show your full configuration:

$ $(npm bin)/eslint --print-config index.js
{
  "globals": {},
  "env": {},
  "rules": {
    "require-jsdoc": "error"
  },
  "parserOptions": {}
}

What did you do? Please include the actual source code causing the issue.

/**
 * Returns "yay!"
 * @returns {String}
 */

function pleaseWork() {
  return "yay!";
}

What did you expect to happen?

No errors. jsdoc allows whitespace between the comment block and the commented node.

What actually happened? Please include the actual, raw output from ESLint.

$ $(npm bin)/eslint index.js

/Users/ruppel/Documents/Scratch/jsdoc/index.js
  6:1  error  Missing JSDoc comment  require-jsdoc

✖ 1 problem (1 error, 0 warnings)

[nzakas]

This was done intentionally. It's tricky to detect appropriate JSDoc comments for pieces of code, so one of the heuristics we use is proximity. We help to narrow the possibilities by assuming that JSDoc comments always appear on the line immediately above the code it describes.

So, I'm not sure we would want to change that, and if we did, it would have to be an option rather than a default behavior.

@eslint/eslint-team other thoughts on this?

[mikesherov]

I would say this is a relaxing option that detects doc-blocks as long as there is no other token between the docblock and the thing it's describing. This seems like a totally legit ask IMO

[nzakas]

For some context, The problem we were trying to avoid was this:

/**
 * @fileoverview This file contains utility functions.
 */

function doSomething() {}

Basically, there's no guarantee that a JSDoc block preceding a function actually refers to that function (and there are numerous types of JSDoc comments that do not).

@mikesherov so is that a 👍 from you?

[mikesherov]

Yeah, thats the exception that proves the rule. It's a 👍 from me to have an optional "relaxed" mode.

[pedrottimark]

How does this potential option for require-jsdoc relate to valid-jsdoc if both are enabled?

Unless valid-jsdoc has similar option for proximity, you can require a comment, but it is not validated, which might be unexpected linting behavior.

If valid-jsdoc has similar option which is set the same to allow non-adjacent comment, consider the problem to avoid in #6238 (comment)

• require-jsdoc does not report it as a problem, even though the comment is not for the function
• valid-jsdoc by default reports Missing JSDoc (at)returns for function; however, no problem for the rule with the "requireReturn": false option

The scenario where neither rule reports a problem [EDIT seems like a new false negative therefore] is the one that makes me ask if an option to allow non-adjacent comments would also need to enable additional constraints in the rules to detect that a preceding JSDoc comment could not be appropriate for the class, function, method? Just asking, not liking it. Am I misunderstanding expectations?

[ilyavolodin]

@eslint/eslint-team We need some more +1s here to mark it as accepted. @pedrottimark brings up a very good point. If we are adding relaxed option here, we should also add the same thing to valid-jsdoc, otherwise those two rules might start conflicting.

[nzakas]

Ack, that's a good point. So the only real option for implementation is to change how JSDoc comments are located rather than adding a rule option. That's a to the core.

Before going forward, we should probably see how JSDoc itself solves this problem and emulate that. Anyone want to volunteer for that?

@mikesherov how does JSCS deal with comment location?

[mikesherov]

@nzakas, JSCS has AST linked with token list. When dealing with comments, we descend down to the token list because it's easier to think about comments by source location instead of in tree.

With respect to JSDoc, there's a JSCS plugin that does that, and I think it leverages the real JSDoc parser although I'm not quite sure.

[kaicataldo]

Hasn't been any movement on this in a while - is this something we still want to discuss?

[jeremyruppel]

I'd still really like the comment detection of eslint to match what JSDoc supports. Leveraging the real JSDoc parser under the hood would be ideal.

[nzakas]

@jeremyruppel if you'd like to submit a PR for that, we'd love to take a look.

[kaicataldo]

I think it's about time to close this unless we decide we want to accept the issue with help wanted for trying to leverage the real JSDoc parser instead of Doctrine.

[jeremyruppel]

@kaicataldo I disagree. I think having jsdoc-related rules that don't match the semantics of the actual jsdoc parser is a defect, not an "enhancement", and this issue should be labeled as such.

[kaicataldo]

Oh, definitely not saying this isn't worth exploring. We've just found that issues that sit unaccepted for a long time without anyone stepping up and taking on the work tend to not get completed. We try to keep the issues list pruned to prevent overload, both for us as a team and to make it as easy as possible for contributors to find relevant things to work on.

As @nzakas mentioned, we'd definitely welcome a PR!

[kaicataldo]

I'm closing this issues because it looks like consensus couldn't be reached. While we wish we could accommodate all requests, we have limited resources and do need to prioritize. We've found that issues failing to reach consensus after 21 days tend to never reach consensus, and as a team have decided to close such issues. This doesn't mean the idea isn't interesting or valuable, only that it's not something the team can commit to at the moment.

[nnmrts]

I never understood why such issues get closed, but nevermind. However, the rule "require-jsdoc" with all settings set to true isn't working for me at all. I understand the "tricky" thing, but I currently have 20 undocumented modules in my project and none of them gets the require-jsdoc error. I don't think this feature is really working well with function expressions and has some strange behaviour when it comes to arrow functions.

For me these function blocks are throwing an error:

// function declaration
function foo() {
  return 1;
};

// function expression + arrow function (with any variable keyword)
var foo = () => 1;

let foo = () => 1;

const foo = () => 1;

var foo = () => {
  return 1;
};

let foo = () => {
  return 1;
};

const foo = () => {
  return 1;
};

But these aren't throwing an error:

// function expression (without any variable keyword)
foo = function() {
  return 1;
};

// function expression (with any variable keyword)
var foo = function() {
  return 1;
};

let foo = function() {
  return 1;
};

const foo = function() {
  return 1;
};

// function expression + arrow function (without any variable keyword)
foo = () => 1;

foo = () => {
  return 1;
};

// arrow function
() => 1;

() => {
  return 1;
};

// weird function expression + declaration (without any variable keyword)
foo = function foo() {
  return 1;
};

// weird function expression + declaration (with any variable keyword)
var foo = function foo() {
  return 1;
};

let foo = function foo() {
  return 1;
};

const foo = function foo() {
  return 1;
};

I tried them in several different environments now, they are behaving the same anywhere. Analysing my small "research", shows us, that the issue hasn't to do anything with es6 features or with variable keywords, although there is a difference if you don't use a variable keyword at all.

(With "variable keyword" I'm referring to var, let and const, I'm sorry if they are called differently. :D)

I would love to see this issue open again to together steer a middle course between switching to another parser and letting eslint show up an error every second line by default because it thinks there should be a jsdoc comment.

To be honest, I don't have a problem with eslint painting some wavy green underlines where they shouldn't be, if I explicitly configure it to do so.

There is probably a reason behind this behaviour and why it was implemented this way. I have a lot of respect for all the sleepless nights you tried to perfectly implement this rule and then probably just decided to narrow the possibilities of detecting missing jsdoc comments. Don't get me wrong, I think that was the right decision. My only dream is to see this already requested "relaxed" option implemented in eslint.

I didn't look too deep into the source code yet, but will definitely try to help where I can. I hope this issue gets discussed again.

Peace <3

[platinumazure]

@eslint/eslint-team Should we revisit this? One of the earlier comments talked about how JSCS looked at comments at the token level instead of the AST. We have since made that change in our comment handling logic. Is there anything we could/should consider doing here to improve the JSDoc validation experience?

[kaicataldo]

@platinumazure If you'd like to advocate for this, can you make a new issue? Old, closed issues don't get much visibility.

[platinumazure]

@kaicataldo I don't have the knowledge to know if I can/should advocate for it. Seemed wiser to ask in case someone else knew and could either run with it or tell me why we shouldn't. I can create a new issue if preferred.

[kaicataldo]

I'm all for discussing how we handle JSDoc comments again.

I think there are two separate issues that have been discussed here (though there may be one solution that solves them both). One is re-evaluating the logic that locates JSDoc comments, and the second is seeing if we can move away from Doctrine to another implementation.

For the first, the logic we use to locate JSDoc comments could probably be refactored and simplified, now that I look at it. Though I'm still unsure of how we would handle the cases described above.

For the latter, JSCS uses the comment-parser and jsdoctypeparser packages to parse JSDocs. We could try to do something similar, though I don't think it's a trivial amount of work. Both packages have quite a few monthly downloads (though one hasn't been updated in a year).

[kaicataldo]

To add to this, I also would interested in exploring alternatives to Doctrine since very few of the maintainers have much knowledge about its internals at this point and it's hard for us to make modifications.