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

require-description-complete-sentence false positives #337

Closed
maxmilton opened this issue Jul 13, 2019 · 4 comments · Fixed by #338
Closed

require-description-complete-sentence false positives #337

maxmilton opened this issue Jul 13, 2019 · 4 comments · Fixed by #338

Comments

@maxmilton
Copy link

maxmilton commented Jul 13, 2019

Looks like commit 78f9b61 to fix #333 has introduced a regression.

@example tags contain code and should not be treated as a sentence. Since it looks like the rule runs on all JSDoc tags, maybe an option to disable the check for certain tags would be useful.

The autofix for the rule is quite annoying so a way to disable the autofix would be great too. It breaks things like changing /** @jest-environment node */ to /** @jest-environment Node */, causing jest to fail.

Starting a line with a capital character also often trigger a false positive. This typically happens when using words which should be capitalised, e.g. names; JavaScript, TypeScript, ProductName. Could the logic be relaxed to only trigger if the previous line ends with a . or is a line break?

Abbreviation which contain .s also trigger false positives, e.g. e.g. Text.

@brettz9 brettz9 mentioned this issue Jul 14, 2019
16 tasks
@brettz9
Copy link
Collaborator

brettz9 commented Jul 14, 2019

Regarding making the fixer optional and changing the logic, I've added a notation on #336, so that can be tracked there.

Regarding abbreviations, have you tried adding a comma, as in e.g.,? That is the standard way to write it anyways (and code using dots should be encapsulable within backticks).

As far as the first issue mentioned re: @example, I plan to add some logic shortly to avoid that by default, and I agree a blacklist option for other tags such as custom ones is a good idea.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 14, 2019

Actually, on further examination of the tags, I think we should implement with a tags whitelist (as we do with other rules), since there are many tags for which a description is not appropriate, e.g., @author, @defaultvalue, etc., though we can default to support some like @classdesc, @summary where it is likely desirable.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 14, 2019
…n default tags likely to have descriptions or by `tags` array for additional choices; fixes gajus#337

Tags that are checked for description by default are: 'param', 'arg', 'argument', 'property', 'prop', 'returns', 'return', 'summary', 'file', 'fileoverview', 'overview', 'classdesc', 'todo', 'deprecated', 'throws', 'exception', 'yields', 'yield'. Note that `see` and `copyright` are not included by default because of potentially allowing a non-description or potential sensitivity, respectively.
golopot pushed a commit that referenced this issue Jul 14, 2019
…n default tags likely to have descriptions or by `tags` array for additional choices; fixes #337

Tags that are checked for description by default are: 'param', 'arg', 'argument', 'property', 'prop', 'returns', 'return', 'summary', 'file', 'fileoverview', 'overview', 'classdesc', 'todo', 'deprecated', 'throws', 'exception', 'yields', 'yield'. Note that `see` and `copyright` are not included by default because of potentially allowing a non-description or potential sensitivity, respectively.
@gajus
Copy link
Owner

gajus commented Jul 14, 2019

🎉 This issue has been resolved in version 15.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gajus gajus added the released label Jul 14, 2019
brettz9 added a commit to interfaced/eslint-plugin-jsdoc that referenced this issue Jul 14, 2019
* master: (46 commits)
  feat(require-description-complete-sentence): limit checking to certain default tags likely to have descriptions or by `tags` array for additional choices; fixes gajus#337
  docs(newline-after-description): indicate applies on doc block
  docs(match-description): add alias `desc` to separate column
  docs(match-description): indicate application by default to `description`/`desc` and allowance for `property`/`prop`; clarify
  fix(match-description): ensure `prop` and `property` matching excludes name
  testing(require-param): fix test source (part of gajus#332)
  testing(require-param): fix test expectation (part of gajus#332)
  docs: generate docs
  chore: update devDeps (eslint-config-canonical, gitdown)
  fix(no-undefined-types): ensure working in all contexts; fixes gajus#324
  refactor(iterateJsdoc): reduce retrieval calls
  docs(check-examples): allow for whitespace at end
  feat(check-examples): add `paddedIndent` option
  fix(check-examples): preserve whitespace so as to report issues with whitespace-related rules such as `indent` (fixes gajus#211)
  chore(travis): fix Travis to use older unicorn version that supports Node 6 (and the canonical config that requires the earlier unicorn version)
  docs(require-returns, require-returns-check): indicate that these will report if multiple `returns` tags are present
  refactor: use `String.prototype.repeat` over lodash `repeat`
  refactor: apply (jsdoc-related) eslint rule fixes
  chore: add `lint-fix` script
  fix(newline-after-description): avoid matching duplicate or partial matches within tag descriptions after the block description; fixes gajus#328
  ...
@gajus
Copy link
Owner

gajus commented Sep 1, 2019

🎉 This issue has been resolved in version 15.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
3 participants