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

Rule 'require-description-complete-sentence' is broken #16

Closed
2 of 3 tasks
atakiel opened this issue Jun 6, 2016 · 3 comments
Closed
2 of 3 tasks

Rule 'require-description-complete-sentence' is broken #16

atakiel opened this issue Jun 6, 2016 · 3 comments

Comments

@atakiel
Copy link

atakiel commented Jun 6, 2016

The rule 'require-description-complete-sentence' is in some cases too aggressive, and actually broken in others.

Problems:

  • It disallows use of markdown in most cases. Many times markdown requires starting and ending the block with non letter character, which this rule disallows. [Aggressive]
  • It disallows use of punctuation other than periods as sentence terminating characters in the end of the paragraph. [Aggressive]
  • Inside the paragraph it allows using lower case letters in the beginning of the sentence. [Broken]

The whole rule is somewhat problematic as there are a lot of exemptions to even the basic premise that the rule tries to check for. A sentence may start with non letter, or lowercase letter in case of a variable name as the first word. And sentence might end with a punctuation other than period.

As possible solution, the rule could be made less aggressive by making it only to check description for complete sentences if description consists only of a single sentence in a single line and no special characters are used. Although even in that situation I would see this rule to be too aggressive in certain situations.

Another possible solution would be just to warn if the description would end in a letter. In this case the line is most certainly missing a punctuation.

@dietergeerts
Copy link

Any news about this?

@brettz9
Copy link
Collaborator

brettz9 commented May 16, 2019

Regarding the first two items, the new rule added to master, match-description, might help as it allows a regular expression (as did ESLint's valid-jsdoc). However, it is across the whole description and does not attempt parsing into paragraphs as with this rule.

Regarding #3, the test case has:

         /**
           * Foo.
           *
           * foo.
           */
          function quux () {

          }

and it does give the error "Sentence should start with an uppercase character.".

Are you still seeing the issue with "Inside the paragraph it allows using lower case letters in the beginning of the sentence.". If so, can you give a test case?

I'll mark this issue as an "Enhancement" for now unless we can confirm the bug you mention still exists. If there is no bug remaining, and if match-description with its single regular expression meets your needs, perhaps we can close the issue.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 9, 2019
…ion and exclamation marks at sentence end (fixes part of gajus#16)
@brettz9
Copy link
Collaborator

brettz9 commented Jul 9, 2019

I've marked item #2 in your list as complete as the most recent fix just released fixes question and exclamation mark handling in the fixer. I've also marked "Inside the paragraph it allows using lower case letters in the beginning of the sentence." as fixed since I haven't heard back and can't replicate myself (and may have been fixed since this issue was reported).

I don't think lower-case as variable names at the beginning (or end) ought to be an issue, as good practice would surround them with back-ticks (for which I've added tests, and the current code is passing). The code should only report when upper-casing has an effect (not when lower-casing has no effect or such).

Do you have any other cases of reasonable Markdown use which backticks at the beginning wouldn't solve? (I haven't allowed backticks at the end as I'd think punctuation could still follow.) If not, I will close, as I think this issue should be resolved (though there are some other unresolved require-description-complete-sentence issues).

@gajus gajus closed this as completed Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants