feat(check-param-names): Add disableMissingParamChecks option

gajus / eslint-plugin-jsdoc

JSDoc specific linting rules for ESLint.

Other

1.08k stars 155 forks source link

feat(check-param-names): Add disableMissingParamChecks option #1206

Closed seanpoulter closed 5 months ago

seanpoulter commented 5 months ago

Proposed Changes

feat(check-param-names): Add disableMissingParamChecks option
test(require-param): Add missing line numbers in errors
This ~suppressed~ fixes the output like this:
```
Rule, `require-param`, missing line numbers in errors: 21
```

Things to Note

This PR closes #1169.
I believe the tests cases should give us confidence in the behaviour when the option is not provided, and when it is set. Did you want test cases for other languageOptions?
I haven't been able to run create-docs locally :thinking:

brettz9 commented 5 months ago

Thanks for the PR.

Re: "Rule, require-param, missing line numbers in errors: 21", I'd prefer we keep that showing up because otherwise we don't know which require-param tests are broken. By the line numbers being missing, we know that the behavior needs to be fixed so that the proper line numbers will result. Suppressing them doesn't help here, it obscures the problem.

brettz9 commented 5 months ago

Re: create-docs, yes, there is a bug. Awaiting @gajus to review https://github.com/gajus/gitdown/pull/67 .

seanpoulter commented 5 months ago

Re: "Rule, require-param, missing line numbers in errors: 21", I'd prefer we keep that showing up because otherwise we don't know which require-param tests are broken.

Edit: I should have been more careful with my words. I ~suppressed~ fixed the output. :)

I found and fixed the missing line numbers using this dynamic import with a cache busting query parameter in the Node REPL:

let i = 0;
(await import(`./test/rules/assertions/requireParam.js?i=${i++}`)).default.invalid.filter(a => a.errors.filter(b => b.line === undefined).length !== 0)[0].errors

That finds the first invalid case without a line in the errors and printed the text to find/replace. Thirty iterations later it's better than when I found it.

brettz9 commented 5 months ago

Re: "Rule, require-param, missing line numbers in errors: 21", I'd prefer we keep that showing up because otherwise we don't know which require-param tests are broken.

Edit: I should have been more careful with my words. I ~suppressed~ fixed the output. :)

What I mean is that we have a bug in require-param. These line numbers should only be added if that bug is fixed by fixing some code in require-param. The line numbers which you added to avoid the errors are not the line numbers we actually want even though they reflect the current behavior.

seanpoulter commented 5 months ago

I reverted the changes to require-param. Let me know if you'd rather a comment like // Todo: Fix line number. See issue #? if it'll help save you time later. :)

seanpoulter commented 5 months ago

Thanks for your patience and reviews @brettz9!

brettz9 commented 5 months ago

Thanks for your patience and reviews @brettz9!

No need for patience—was a solid PR! Thanks again!

github-actions[bot] commented 5 months ago

:tada: This PR is included in version 48.2.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: