Skip to content

New lint proposal: end_doc_comments_with_punctuation #58188

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

Open
nscobie opened this issue Jun 16, 2020 · 4 comments
Open

New lint proposal: end_doc_comments_with_punctuation #58188

nscobie opened this issue Jun 16, 2020 · 4 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. customer-google3 devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@nscobie
Copy link

nscobie commented Jun 16, 2020

Related PR: dart-archive/linter#2148

Describe the rule you'd like to see implemented
A simple style rule to help enforce proper grammar in documentation comment blocks by ensuring they end with proper sentence terminating punctuation.

Terminating punctuation would be defined as one of the following: period (.), question mark (?), or exclamation point (!). The rule would effectively only check the very last character of the entire comment block (with the exception of Java-style doc comments), and would not attempt to enforce any punctuation rules for sentences "higher up" in the comment.

The lint would only apply to documentation comments (/// Blah. or /** Blah. */), and would not apply to normal comments (// Blah. or /* Blah. */).

This rule is not specific to any particular framework, and could help anyone utilizing dartdoc.

Examples (test cases from PR)

// BAD

// LINT [+1]
/// This sentence doesn't have any terminating punctuation at the end
void a() => null;

// LINT [+2]
/// This is a long documentation comment composed of multiple sentences.
/// However, it looks like we forgot to terminate the last one, which is bad
void b() => null;

// LINT [+4]
// ignore: slash_for_doc_comments
/**
 * We also flag Java-style documentation comments, such as this long one we have
 * spread across multiple lines here
 */
void c() => null;

// GOOD

// OK
/// This sentence is properly terminated with a period.
void d() => null;

// OK
/// This is a very long sentence spread across multiple lines of a documentation
/// comment, but it still ends with terminating punctuation!
void e() => null;

// OK
// ignore: slash_for_doc_comments
/**
 * This Java-style documentation comment is properly terminated.
 */
void f() => null;

// OK
/// A question mark works too, would you like to see?
void g() => null;

Additional context

Open questions

  • Is the name end_doc_comments_with_punctuation okay, or would something else be preferred?
  • Issue Lint to end the first paragraph of doc comments with a period. #57202 requests a "lint to end the first paragraph of doc comments with a period." Should that functionality be incorporated into this lint as well?
  • Are there any other symbols which could be desired at the end of a documentation comment? Perhaps the colon (:)?
@bwilkerson
Copy link
Member

Thanks for opening an issue to discuss this rule.

I'm concerned about the number of false positives from such a lint. While we do allow a small number of false positives from a lint rule (when they are unavoidable and rare and the value of the rule is sufficient to justify the cost of dealing with those false positives), we strive to have as few false positives as possible. My guess is that this rule would have too many false positives and not enough value to enough users.

In his comment on the PR, Sam gave several good cases where this lint (as written) could validly be violated and I suspect that there are plenty of others. In general, natural language processing is beyond the scope of what the linter is currently able to support (which is part of why #57202 hasn't been addressed), and I think it's better to not introduce lints that require natural language support.

Are there any other symbols which could be desired at the end of a documentation comment?

There are additional punctuation marks used by languages other than English that would probably need to be supported.

@pq
Copy link
Member

pq commented Jun 16, 2020

I'm concerned about the number of false positives from such a lint.

I admit I hadn't thought about some of the concerns Sam raised when I encouraged @nscobie to open this.

For context, this is a rule that is motivated by an internal request.

@nscobie: I'm curious, have you run this across your team's code corpus?

@nscobie
Copy link
Author

nscobie commented Jun 16, 2020

@bwilkerson: You and Sam both raise very valid points about how this could go wrong, and after thinking about your examples I totally understand the desire to not open up this can of worms.

However, I am quite curious now about our discussion on PR dart-archive/linter#2148 regarding parsing Markdown, and would like to briefly investigate your suggestions about measuring false positives and performance before giving up.

Also, regarding additional punctuation marks used by languages other than English: perhaps we could reduce our scope by clearly marking this lint as only supporting natural languages X, Y, and Z? I'm unsure if that would be too exclusionary.

@pq: Unfortunately I cannot test this rule against my team's code corpus, I'll message you with more details. I will look into running it against the Flutter framework and popular packages on pub.dev soon. And while it was motivated by an internal request, my understanding is that it's not high priority.

@bwilkerson
Copy link
Member

I fully support you investigating further. My intuition isn't perfect. :-)

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
@pq pq added the P3 A lower priority bug or feature request label Nov 20, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. customer-google3 devexp-linter Issues with the analyzer's support for the linter package linter-lint-request P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants