Chore: enable eslint-plugin-jsdoc (refs #11146)#12332
Merged
mysticatea merged 17 commits intomasterfrom Sep 29, 2019
Merged
Conversation
24e99fc to
f1b974f
Compare
kaicataldo
commented
Sep 28, 2019
| * @property {number} fixableWarningCount Number of fixable warnings for the result. | ||
| * @property {string=} [source] The source code of the file that was linted. | ||
| * @property {string=} [output] The source code of the file that was linted, with as many fixes applied as possible. | ||
| * @property {string} [source] The source code of the file that was linted. |
Member
Author
There was a problem hiding this comment.
jsdoc/check-syntax enforces this syntax for optional parameters.
kaicataldo
commented
Sep 28, 2019
| /** | ||
| * Collect used deprecated rules. | ||
| * @param {ConfigArray[]} usedConfigArrays The config arrays which were used. | ||
| * @param {Map<string, Object>} ruleMap The rule definitions which were used (built-ins). |
Member
Author
There was a problem hiding this comment.
This parameter no longer exists.
kaicataldo
commented
Sep 28, 2019
| /** | ||
| * Creates a new instance of the core CLI engine. | ||
| * @param {CLIEngineOptions} providedOptions The options for this instance. | ||
| * @constructor |
Member
Author
There was a problem hiding this comment.
ES2015 classes no longer need to be documented as @class or @constructor - the parser can now identify this case.
kaicataldo
commented
Sep 28, 2019
| } | ||
|
|
||
| /* eslint-disable valid-jsdoc */ | ||
| /* eslint-disable jsdoc/check-param-names, jsdoc/require-param */ |
Member
Author
There was a problem hiding this comment.
A really nice thing about the separate checks is that we can disable specific checks and still enforce other things we care about!
kaicataldo
commented
Sep 28, 2019
| * configuration or options. | ||
| * | ||
| * @typedef {array|number} ruleConfig | ||
| * @typedef {Array|number} ruleConfig |
Member
Author
There was a problem hiding this comment.
I'm not sure why valid-jsdoc didn't catch this.
kaicataldo
commented
Sep 28, 2019
| /** | ||
| * Stored valid rule configurations for this instance | ||
| * @type {array} | ||
| * @type {Array} |
Member
Author
There was a problem hiding this comment.
I'm not sure why valid-jsdoc didn't catch this.
kaicataldo
commented
Sep 28, 2019
|
|
||
| /** | ||
| * Returns the path inside of the fixture directory. | ||
| * @param {...string} args file path segments. |
Member
Author
There was a problem hiding this comment.
It doesn't seem like valid-jsdoc enforced documenting rest parameters.
mysticatea
reviewed
Sep 28, 2019
added 16 commits
September 28, 2019 20:00
5b1d393 to
60486d1
Compare
Member
Author
|
I'm not sure why the commit-message check is failing - it looks like it should accept |
This was referenced Sep 29, 2019
This was referenced Oct 22, 2019
This was referenced Nov 9, 2019
This was referenced Mar 11, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:
refs #11146
Finally have the time to switch to
eslint-plugin-jsdocineslint-config-eslintnow that our core JSDoc rules are deprecated!What changes did you make? (Give an overview)
This PR disables the deprecated
valid-jsdocandrequire-jsdoccore rules in favor of a series of (mostly) corresponding rules ineslint-plugin-jsdoc. This should probably be a major release ofeslint-config-eslint, as the behavior isn't entirely the same and I've added a few extra rules (please let me know if this isn't desired).New warnings:
It seems like these rules catch some
valid-jsdocfalse negatives and it's actually really nice that we can disable individual rules when we need to. I think it would also be nice to add the following rules in a future PR:Is there anything you'd like reviewers to focus on?
I'd appreciate some eyes on the changes that I've made in the comments themselves. Will try to preemptively leave comments for the rationale for these changes.