Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require-description for @type when used with arrow function #1167

Closed
thernstig opened this issue Nov 8, 2023 · 7 comments · Fixed by #1168
Closed

require-description for @type when used with arrow function #1167

thernstig opened this issue Nov 8, 2023 · 7 comments · Fixed by #1168

Comments

@thernstig
Copy link

I am not 100% sure this is a bug, but believe it is.

Expected behavior

This should pass require-description:

app.use(
  /** @type {express.ErrorRequestHandler} */
  (err, req, res, next) => {
    // foo
  }
);

Actual behavior

The code fails on require-description with "Missing JSDoc block description".

Additional context

This works though. Notice the extra parenthesis:

app.use(
  /** @type {express.ErrorRequestHandler} */
  (
    (err, req, res, next) => {
      // foo
    }
  )
);

ESLint Config

I will supply this immediately if this is considered a bug even.

ESLint sample

See above.

Environment

  • Node version: v18.18.2
  • ESLint version: v8.31.0
  • eslint-plugin-jsdoc version: 44.2.2
@brettz9
Copy link
Collaborator

brettz9 commented Nov 8, 2023

Your first example should be violating the rule unless you are using exemptTags. The reason the second example is not violating the rule is because the comment is placed before 1 line above the responsible structure (and parentheses don't get their own AST structure). If settings.jsdoc.maxLines is 2 or more, you will also see this one violating the rule (since its comment will be found).

@thernstig
Copy link
Author

@brettz9 we have not set exemptTag, but we still do not get violations for things like:

    /** @type {TreeViewItemData[]} */
    this.treeViewSelection = [];

Hence I found it confusing the @type usage in my first example required this.

So you are saying this is expected? I am not entirely sure I understood the reasons you mentioned, but will re-read once more later.

@brettz9
Copy link
Collaborator

brettz9 commented Nov 9, 2023

Yes, the behavior is expected, even though I'll admit that we'd ideally be able to allow one line above parentheses to trigger, even if the underlying structure were one line below the parentheses.

The reason it does not trigger for this.treeViewSelection = []; is that this rule only applies by default to functions. You have to supply contexts to get it to work elsewhere (such as AssignmentExpression).

@brettz9
Copy link
Collaborator

brettz9 commented Nov 9, 2023

I do see we can make the change with parentheses. So I guess maxLines and minLines will best be counted relative to the parentheses if there is one. As they are not their own AST structure, it makes sense, I think, to treat them as an extension of the structure, given how it looks for JSDoc.

@thernstig
Copy link
Author

@brettz9 I am not having the best time following, but are you indicating you might do some kind of "fix" for this 😆 ?

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Nov 10, 2023
@brettz9
Copy link
Collaborator

brettz9 commented Nov 10, 2023

Yes. I am planning on changing the behavior so that the settings maxLines and minLines will do their calculations relative to any enclosing parenthesis before falling back to calculating relative to the language feature (e.g., a function). This should better ensure, especially with the frequent @type casts, that rules can find the preceding comment blocks.

Copy link

🎉 This issue has been resolved in version 46.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants