I would be interested in working on this if no one else is.

(In reply to trickypr from comment #2)

I would be interested in working on this if no one else is.

Hi, thank you for the offer. This needs a little bit of work ahead before we are ready to implement - we also want to bring consistency for our rules across the tree, so we need to create a couple of configurations for the new rules which are then used everywhere.

If you are willing I think the starting point for this would be to go through the list of existing valid-jsdoc and require-jsdoc rules that are enabled across mozilla-central, and come up with a list of what the common (or mostly common) parts are, and then to map those to a list of rules in eslint-plugin-jsdoc.

It would probably be worth keeping the "valid" and "require" requirements as separate lists for now. It might be worth keeping a list of non-common rules that are used in various places, in case there's something else to enable.

We'll need to some level of agreement on the basic lists, but if they're fairly well matching what we have already, that hopefully shouldn't take too long.

Once we've got the lists, then we can work on the actual implementation.

If this is a bit more than you were expecting, I quite understand.

For the most part, those who are using jsdoc rules have a consistent rule set

Valid JSDoc

Status: Enabled, error

preferences:

• return -> returns
• Boolean -> boolean
• Number -> number
• String -> string
• bool -> boolean

requireParamDescription: false
requireReturn: false
requireReturnDescription: false

Varients

browser/components/newtab is disabled and has no rewriting preferences.

browser/components/pagedata has requireParamDescription set to true

debugger/client is disabled and has no rules

security and taskcluster/docker/periodic-updates do not have rewriting preferences

Require JSDoc

Status: enabled, error

FunctionDeclaration: false
MethodDefinition: false
ClassDeclaration: true
ArrowFunctionExpression: false
FunctionExpression: false

Varients

browser/components/newtab is disabled and has no rules

browser/components/pagedata has the following changes:

• FunctionDeclaration: true
• MethodDefinition: true

There are less users of require-jsdoc than there are valid-jsdoc

eslint-plugin-jsdoc config

{
  "rules": {},
  "settings": {
    "jsdoc": {
      "tagNamePreference": {
        "return": "returns"
      },
      "preferredTypes": {
        "Boolean": "boolean",
        "Number": "number",
        "String": string,
        "bool": "boolean"
      }
    }
  }
}

I don't think the new plugin has as valid controls as the old system, so I can't replicate require-jsdoc.

eslint-plugin-jsdoc seems to be stricter than the old system. There
are a few places where it caught errors that the old linter didn't and
that seemed valid, so I fixed them.

Thank you for starting this off, it is great to see. I'd like to structure this a little differently so that we can make it easier to roll-out, and I think we'll want some more rules enabled by default.

Here's what I suggest:

• I'd like to separate out adding the node module for eslint-plugin-jsdoc into a separate bug - I would like to generate the patch for that bit, as it'll require some additional work on our side so that automation correctly picks up the module, and there's some extra pieces/updates I want to include. I should be able to do that quite quickly.
• I'd like to start putting these rules into two configurations, which we can then use across the repository, e.g. "valid-jsdoc" and "require-jsdoc". To do this:
  • Create new configs alongside our existing ones
  • Reference those in the index
  • These can then be referenced in.eslintrc.js files via:

extends: [
  "plugin:mozilla/valid-jsdoc",
  "plugin:mozilla/require-jsdoc",
],

I'm suggesting two configurations for now, because I think that rolling out valid-jsdoc across the whole tree (though not as part of this bug!) is going to be much easier than require-jsdoc.

• In the valid-jsdoc configuration, I think we need to include more rules, these are probably all the ones we should have there:

    "jsdoc/check-access": "error",
    "jsdoc/check-param-names": "error",
    "jsdoc/check-property-names": "error"
    "jsdoc/check-tag-names": "error",
    "jsdoc/check-types": "error",
    "jsdoc/require-param-type": "error",
    "jsdoc/require-returns-type": "error",
    "jsdoc/valid-types": "error",

Although require-param-type and require-returns-type could be seen as part of the require section, I think that having the type specified is part of having a valid jsdoc, so if you're specifying one of those tags, you should also be specifying the type.

The other rules can go in the require-jsdoc configuration.

Does that sound reasonable?

That seems like a cleaner way of implementing it.

The only problem I would raise is that the configuration above is strict (82 errors, 42 fixable on places). From what I can see, most of the errors are legitimate problems. Once Bug 1792465 has been fixed, I will create a new patch.

I think it is fine that we go a little bit stricter - I'd rather go stricter as we roll it out jsdoc to more locations and have consistency, than to try and bring that in later.

Hi :trickypr, just in case you missed it, the other bug has landed now.

I have added a few style rules in valid-jsdoc. They can all be fixed via --fix, so I think they are worth including for consistency.

"jsdoc/check-alignment": "error",
"jsdoc/empty-tags": "error",
"jsdoc/newline-after-description": "error",
"jsdoc/no-multi-asterisks": "error",
"jsdoc/tag-lines": "error",

I should note that mozilla/require-jsdoc is much stricter than existing rules within the tree, but I think that it is more correct.

The two rules here (require-jsdoc and valid-jsdoc) will likely be
removed in the not-to-distant future, so I am cleaning up their usage.

That should be the last patches needed to close this bug.

Oops, still have a few more to land.

@trickypr: Thank you for all your work on this. It has been really great to get done and a great improvement for our docs. The last patch is now in the landing process, so I'll remove the leave-open, so once this lands the bug should get automatically marked as fixed.