ESLint JSDoc rules removed

[tdulcet]

The two ESLint JSDoc rules used by this template and your other add-ons were deprecated in 2018: https://eslint.org/blog/2018/11/jsdoc-end-of-life/ and it looks like they might be removed soon: eslint/eslint#15820

AddonTemplate/.eslintrc

Lines 79 to 102 in 2068d62

	"require-jsdoc": ["error", {
	"require": {
	"FunctionDeclaration": true,
	"MethodDefinition": false,
	"ClassDeclaration": false,
	"ArrowFunctionExpression": false
	}
	}],
	"valid-jsdoc": ["error", {
	"prefer": {
	"return": "returns"
	},
	"preferType": {
	"Boolean": "boolean",
	"Number": "number",
	"object": "Object",
	"String": "string",
	"HtmlElement": "HTMLElement"
	},
	"requireReturnType": true,
	"matchDescription": ".+",
	"requireParamDescription": false,
	"requireReturnDescription": false
	}],

It would probably be good to find a replacement. They recommended using the eslint-plugin-jsdoc plugin, but I would recommend using the TypeScript compiler instead (it supports checking JS code), as it is built into many editors by default (such as VS Code) and it can also validate the data types. Using it just requires adding a few keys to the compilerOptions section of the existing jsconfig.json file, such as:

    "module": "es2021",
    "checkJs": true,
    "strict": true,
    "noImplicitAny": false,
    "forceConsistentCasingInFileNames": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,

See the documentation for more information: https://www.typescriptlang.org/docs/handbook/tsconfig-json.html

Using this to check Unicodify and the Awesome Emoji Picker found a significant number of JSDoc (mostly incorrect datatypes) and other issues, which I can summit respective PRs to fix if you want.

Mozilla is tracking this issue in Bug 1510561.

[rugk]

Thanks for the heads-up! So, sure feel free to raise a PR and migrate the existing rules.

I looked whether there are some base configs we can use, but I've found no useful ones.

However, what we should keep is the requirement for JSDoc, i.e. maybe migrate that to that require-jsdoc option? Because AFAIK/understood the TypeScript languageserver or so also only checks JSDoc when it is present. As such, we need to require it's presence.

[tdulcet]

However, what we should keep is the requirement for JSDoc, i.e. maybe migrate that to that require-jsdoc option?

Yeah, to replicate the existing require-jsdoc rule, you would need to use that plugin. If you do, you could just set "extends": ["plugin:jsdoc/recommended"] to enable a bunch of rules. Although you would then need to update your documentation to require contributors to download that plugin in addition to ESLint.

If we are adding plugins, we should probably add Mozilla's eslint-plugin-no-unsanitized and eslint-plugin-mozilla plugins as well. The eslint-plugin-unicorn plugin might also be good (see bug 1539513).

Because AFAIK/understood the TypeScript languageserver or so also only checks JSDoc when it is present. As such, we need to require it's presence.

If you set the noImplicitAny option to true (or when strict is true), it will complain about any variables it is unable to automatically determine the datatype for, which includes most function parameters without JSDoc comments, but also many arrow functions and non-constant global variables. You of course currently do not require JSDoc comments for arrow functions and global variables, which is why I explicitly set it to false in my example above.

Anyway, it may be good to setup GitHub Actions CI for each of your add-ons to run both ESLint and the TypeScript compiler, so that it can automatically check PRs (similar to rugk/unicodify#7).

[rugk]

Anyway, it may be good to setup GitHub Actions CI for each of your add-ons to run both ESLint and the TypeScript compiler, so that it can automatically check PRs (similar to rugk/unicodify#7).

Yeah, that would be good…

I just don't have much time currently for stuff, as you probably noticed, so contributors adding CI stuff or so, are always welcome. Maybe there is even a GitHub Action for that that does test everything.

Also web-ext lint would propbably be a good addition: https://github.com/marketplace/actions/web-ext-lint

You of course currently do not require JSDoc comments for arrow functions and global variables, which is why I explicitly set it to false in my example above.

Ah was unaware/missed that this of course catches missing JSDoc comments, yeah I see.
We cannot only require it for non-lambda functions, can we? Hmm… too bad.

Otherwise I would have said yeah not too bad, let's not care about EsLint plugins.

However… if we need them anyway…

If we are adding plugins, we should probably add Mozilla's eslint-plugin-no-unsanitized plugin as well.

Sure, totally agree, of course.

[tdulcet]

I just don't have much time currently for stuff, as you probably noticed, so contributors adding CI stuff or so, are always welcome.

I have not tested it, but something like this should do the trick (along with those changes to the jsconfig.json file above):

jobs:
  ESLint:
    name: ESLint

    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v3
    - uses: actions/setup-node@v3
    - name: Install
      run: npm install -g eslint
    - name: Script
      run: eslint --report-unused-disable-directives .
      continue-on-error: true

  TSC:
    name: TypeScript Compiler

    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v3
      with:
        submodules: recursive
    - uses: actions/setup-node@v3
    - name: Install
      run: npm install -g typescript
    - name: Script
      run: tsc -p jsconfig.json --noEmit
      continue-on-error: true

Those continue-on-error could be removed after all the existing errors are fixed. More changes would of course be needed if you decide to add any ESLint plugins or TypeScript type definitions.

We cannot only require it for non-lambda functions, can we? Hmm… too bad.

No, TypeScript is really all or nothing, as it wants to know the datatype for all variables. If you want some examples, just add those suggested keys to your jsconfig.json file, change that noImplicitAny option to true and then open a JS file in VS Code.

[rugk]

I'm happy for any PR for any simple add-on to test this on. Note that even on PRs if you add the file it should run (maybe once I approve that you may execute pipelines or so), so you can easily test it.

[tdulcet]

The JSDoc rules were just removed in the upcoming ESLint v9: eslint/eslint#17694