Chore: enable eslint-plugin-jsdoc (refs #11146) #12332
Merged
Conversation
This file contains 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
eslint-deprecated
bot
added
the
triage
kaicataldo
force-pushed
the
eslint-plugin-jsdoc
branch
from
24e99fc
to
f1b974f
Compare
kaicataldo
commented
Sep 28, 2019
| @@ -85,8 +85,8 @@ const validFixTypes = new Set(["problem", "suggestion", "layout"]); | |||
| * @property {number} warningCount Number of warnings for the result. | |||
| * @property {number} fixableErrorCount Number of fixable errors for the result. | |||
| * @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. | |||
There was a problem hiding this comment.
jsdoc/check-syntax enforces this syntax for optional parameters.
kaicataldo
commented
Sep 28, 2019
| @@ -329,7 +329,6 @@ function getRule(ruleId, configArrays) { | |||
| /** | |||
| * 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). | |||
There was a problem hiding this comment.
This parameter no longer exists.
kaicataldo
commented
Sep 28, 2019
| @@ -530,7 +529,6 @@ class CLIEngine { | |||
| /** | |||
| * Creates a new instance of the core CLI engine. | |||
| * @param {CLIEngineOptions} providedOptions The options for this instance. | |||
| * @constructor | |||
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
| @@ -78,7 +78,7 @@ function mergeDefaultOptions(options) { | |||
| return Object.assign({}, DEFAULT_OPTIONS, options); | |||
| } | |||
|
|
|||
| /* eslint-disable valid-jsdoc */ | |||
| /* eslint-disable jsdoc/check-param-names, jsdoc/require-param */ | |||
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
| @@ -99,7 +99,7 @@ function groupByProperty(objects) { | |||
| * Configs may also have one or more additional elements to specify rule | |||
| * configuration or options. | |||
| * | |||
| * @typedef {array|number} ruleConfig | |||
| * @typedef {Array|number} ruleConfig | |||
There was a problem hiding this comment.
I'm not sure why valid-jsdoc didn't catch this.
kaicataldo
commented
Sep 28, 2019
| @@ -185,7 +185,7 @@ class RuleConfigSet { | |||
|
|
|||
| /** | |||
| * Stored valid rule configurations for this instance | |||
| * @type {array} | |||
| * @type {Array} | |||
There was a problem hiding this comment.
I'm not sure why valid-jsdoc didn't catch this.
kaicataldo
commented
Sep 28, 2019
| @@ -85,6 +85,7 @@ describe("CascadingConfigArrayFactory", () => { | |||
|
|
|||
| /** | |||
| * Returns the path inside of the fixture directory. | |||
| * @param {...string} args file path segments. | |||
There was a problem hiding this comment.
It doesn't seem like valid-jsdoc enforced documenting rest parameters.
kaicataldo
added
chore
mysticatea
reviewed
Sep 28, 2019
kaicataldo
force-pushed
the
eslint-plugin-jsdoc
branch
from
5b1d393
to
60486d1
Compare
kaicataldo
changed the title
|
I'm not sure why the commit-message check is failing - it looks like it should accept |
platinumazure
changed the title
platinumazure
changed the title
mysticatea
changed the title
This was referenced Sep 29, 2019
This was referenced Oct 22, 2019
This was referenced Nov 9, 2019
This was referenced Mar 11, 2020
eslint-deprecated
bot
added
the
archived due to age
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.