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

valid-jsdoc should not require @return for ES2015 class constructor #4976

Closed
zandaqo opened this Issue Jan 16, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@zandaqo
Copy link
Contributor

commented Jan 16, 2016

Hi,

Isn't the @return tag a bit redundant for ES2015 class constructors? Especially considering that valid-jsdoc rule doesn't require the tag from functions that are marked as constructors with @constructor tag. I propose to not require the @return tag for constructors by default, just like how it's done right now with getters and functions with the @constructor tag.

@eslintbot

This comment has been minimized.

Copy link

commented Jan 16, 2016

@zandaqo Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Jan 16, 2016

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 17, 2016

It sounds like you're reporting a possible bug. Can you please provide the information requested in the last comment?

@zandaqo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2016

  1. ESLint version 1.10.3
  2. Just omitted @constructor and @return tags in the comment for ES2015 class constructor: the source code, the config
  3. 15:3 error Missing JSDoc @returns for function valid-jsdoc
  4. I expected it to recognize the constructor and to not require the the return tag.

I too thought that it was a bug at first, fixed it and went to add tests, but found that they are explicitly set to require @return from the ES2015 constructors, see this test case: https://github.com/eslint/eslint/blob/master/tests/lib/rules/valid-jsdoc.js#L469

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 18, 2016

Ah, that's very helpful. We can probably not require @returns for class constructors.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jan 19, 2016

should we warn for unnecessary return if constructor has a return defined?

@zandaqo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2016

@gyandeeps I am all for it, but I believe it should be separated into an issue of its own. With this enhancement we are relaxing the rule and it won't affect those who so far followed the previous rule of adding returns to constructors, thus, no breaking change is happening, not in my pull request #4977 at least.

Maybe we should push it a bit further and add checks for other unnecessary tags, like @constructor for ES2015, and introduce an option for them, say unnecessaryTags, so that users who would still prefer to add @constructor, @return, @class and the likes in ES2015 code can continue doing so by setting unnecessaryTags to false. If people support this idea, I'll try to implement it.

@nzakas

This comment has been minimized.

Copy link
Member

commented Jan 19, 2016

@gyandeeps I agree with @zandaqo , that would be a separate rule, not part of this.

@zandaqo that sounds like more than is necessary at this point. If you'd like to discuss unnecessary taga further, please open a new issue.

@gyandeeps gyandeeps closed this in 2b33c74 Jan 20, 2016

gyandeeps added a commit that referenced this issue Jan 20, 2016

Merge pull request #4977 from zandaqo/issue4976
Update: valid-jsdoc to not require `@return` in constructors (fixes #4976)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.