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

Audit ESLint rules. I got 99 problems, but they seem to be JSDoc themed. #3035

Open
pdehaan opened this issue May 11, 2023 · 2 comments
Open
Labels
jira-synced Jira task created for this

Comments

@pdehaan
Copy link
Collaborator

pdehaan commented May 11, 2023

Was trying to find some ESLint errors in Circle-CI logs for a recent PR and noticed we currently have aboot 133 warnings.

npm run lint:js
…
✖ 133 problems (0 errors, 133 warnings)
  0 errors and 1 warning potentially fixable with the `--fix` option.

A bit of ugly grep-ing shows that about 99/133 of the warnings are jsdoc/* related.

npm run lint:js | grep -E "\sjsdoc/.*$" | wc -l # 99

We should probably either fix or mute those warnings, since it makes finding errors amongst the noise needlessly difficult.

As for the remaining warnings:

npm run lint:js | grep " warning " | grep -Ev "\sjsdoc/.*$"
LOC TYPE MESSAGE RULE
152:23 warning 's' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
152:23 warning 's' is defined but never used @typescript-eslint/no-unused-vars
7:10 warning 'UserInputError' is defined but never used. Allowed unused vars must match /^_/u no-unused-vars
7:10 warning 'UserInputError' is defined but never used @typescript-eslint/no-unused-vars
69:9 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
3:1 warning header should be a block comment header/header
12:49 warning 'page' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
12:49 warning 'page' is defined but never used @typescript-eslint/no-unused-vars
10:44 warning 'res' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
10:44 warning 'res' is defined but never used @typescript-eslint/no-unused-vars
10:49 warning 'next' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
10:49 warning 'next' is defined but never used @typescript-eslint/no-unused-vars
25:39 warning 'next' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
25:39 warning 'next' is defined but never used @typescript-eslint/no-unused-vars
19:20 warning '_weight' is assigned a value but never used @typescript-eslint/no-unused-vars
20:12 warning '_language' is assigned a value but never used @typescript-eslint/no-unused-vars
52:16 warning 'mailoptions' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
52:16 warning 'mailoptions' is defined but never used @typescript-eslint/no-unused-vars
52:29 warning 'callback' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
52:29 warning 'callback' is defined but never used @typescript-eslint/no-unused-vars
197:25 warning '_error' is defined but never used @typescript-eslint/no-unused-vars
39:44 warning 'isUserBrowserFirefox' is assigned a value but never used. Allowed unused vars must match /^_/u no-unused-vars
39:44 warning 'isUserBrowserFirefox' is assigned a value but never used @typescript-eslint/no-unused-vars
39:100 warning 'isUserLocaleEn' is assigned a value but never used. Allowed unused vars must match /^_/u no-unused-vars
39:100 warning 'isUserLocaleEn' is assigned a value but never used @typescript-eslint/no-unused-vars
9:22 warning '_data' is defined but never used @typescript-eslint/no-unused-vars
33:96 warning 'isUserLocalEn' is assigned a value but never used. Allowed unused vars must match /^_/u no-unused-vars
33:96 warning 'isUserLocalEn' is assigned a value but never used @typescript-eslint/no-unused-vars
5:26 warning 'data' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
5:26 warning 'data' is defined but never used @typescript-eslint/no-unused-vars
5:28 warning 'data' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
5:28 warning 'data' is defined but never used @typescript-eslint/no-unused-vars
7:25 warning 'data' is defined but never used. Allowed unused args must match /^_/u no-unused-vars
7:25 warning 'data' is defined but never used @typescript-eslint/no-unused-vars

Also seems fishy that we have both ESLint no-unused-vars and TypeScript-y @typescript-eslint/no-unused-vars rules enabled. Per the typescript-eslint docs:

"This rule extends the base eslint/no-unused-vars rule. It adds support for TypeScript features, such as types."

@Vinnl
Copy link
Collaborator

Vinnl commented May 12, 2023

Also seems fishy that we have both ESLint no-unused-vars and TypeScript-y @typescript-eslint/no-unused-vars rules enabled.

Oops, that's my fault. We should probably disable the former, and apply the configuration we currently have for that rule to the TS version. Unless the TS version doesn't lint JS files, which I'm not sure about.

@pdehaan
Copy link
Collaborator Author

pdehaan commented May 21, 2023

Actually, I built a quick little ESLint error/warning summarizer and ran it against the nextjs branch.
Looks like we should probably disable the jsdoc/* rule set which would remove about 79% of the warnings.
Disabling @typescript-eslint/no-explicit-any and @typescript-eslint/no-unused-vars would solve another 19% of the warnings.

import cp from "node:child_process";
import { inspect } from "node:util";

const ERROR_REGEXP = /^\s+\d+:\d+\s+(warning|error)\s+.*\s(.*?)$/i;

const errorMap = {};
let summary;

try {
  cp.execSync("npm run lint:js");
} catch (err) {
  const stderr = err.stdout.toString().split("\n");
  for (const line of stderr) {
    if (ERROR_REGEXP.test(line)) {
      const [, type, rule] = line.match(ERROR_REGEXP);
      errorMap[type] = errorMap[type] || {};
      errorMap[type][rule] = (errorMap[type][rule] || 0) + 1;
    }
    if (line.startsWith("✖")) {
      summary = line;
    }
  }
}

console.log(inspect({ ...errorMap, _summary: summary }, { sorted: true }));

OUTPUT

  {
    _summary: '✖ 142 problems (5 errors, 137 warnings)',
    error: {
      '@next/next/no-assign-module-variable': 2,
      'n/no-callback-literal': 1,
      'no-empty': 2
    },
    warning: {
      '@next/next/no-img-element': 1,
      '@typescript-eslint/no-explicit-any': 6,
      '@typescript-eslint/no-unused-vars': 20,
      'header/header': 2,
      'jsdoc/check-param-names': 1,
      'jsdoc/no-undefined-types': 13,
      'jsdoc/require-param-type': 6,
      'jsdoc/require-property-description': 40,
      'jsdoc/require-returns': 6,
      'jsdoc/require-returns-description': 14,
      'jsdoc/require-returns-type': 19,
      'jsdoc/valid-types': 9
    }
  }
  • 78.8% of warnings are jsdoc/* warnings.
  • 19.0% of warnings are typescript any/unused warnings.

@EMMLynch EMMLynch added the jira-synced Jira task created for this label Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira-synced Jira task created for this
Projects
None yet
Development

No branches or pull requests

3 participants