Linter: lint request: enforce recommended dartdoc structure

[pq]

In https://github.com/flutter/flutter/pull/27791#discussion_r255772365, @goderbauer wrote:

That a dartdoc should start with a one-sentence summary as its own paragraph is something I have to frequently point out in code reviews - so I'd prefer it if things like these could be pointed out automatically.

👍

Let's talk details.

1. Enforcing a summary statement lines up w/ this advice from Effective Dart and I think we should just do it: https://www.dartlang.org/guides/language/effective-dart/documentation#do-separate-the-first-sentence-of-a-doc-comment-into-its-own-paragraph

Other thoughts?

[goderbauer]

Yes, yes, yes! Please!

[srawlins]

One thing I'd like to guard against: Pythnon's linter (one of them) enforces a one _line_ summary which I think is a disaster. It limits the summary to something like 75 characters, yuck. One _sentence_ is the Effective Dart recommendation, and we should make sure to stick to that.

I'm not volunteering to write the regex / Markdown parser for this 😁

[pq]

I'm not volunteering to write the regex / Markdown parser for this 😁

Hopefully we can just use the parser from package:markdown for this? Alternatively we could crib some bits from dartdoc?

[pq]

Or is that even over-thinking it? Can we just get away with something simple like

comment.split('\n\n');

?

[goderbauer]

How would you make sure that after splitting the first part is just one sentence though?

[pq]

Yeah. I realized that. A simple scanner might work but then maybe there are complex cases.

@jcollins-g: would it be easy for us to crib this logic from dartdoc?

EDIT: I was assuming there is such logic in dartdoc but I don't know actually why there would be. That is, from dartdoc's perspective, I believe the one liner is just derived from markdown parsed to html. Something like:

var html = markdownToHtml(docString);
var summary = HtmlParser(html).parse().body.children.first.innerHtml;

I'm guessing there's no validation that would notice

This is a bad summary. Because I didn't line-break.

Any more than it would wrongly split

Ms. Marvel is the name of several fictional superheroes appearing in comic books published by Marvel Comics.

Regardless, since performance will be a real issue I'm guessing that if we do want to do this we'd want to roll a regexp or simple scanner to match bad sentences. I have no idea how sloppy that'd be in practice or what the risk of false positive but it'd be pretty easy to do some testing on Flutter.

Anyway, it'd be interesting to get other thoughts here and if there is something we can get from dartdoc all the more awesome.

[jcollins-g]

There is logic here in dartdoc that divides the HTML output into two segments:

https://github.com/dart-lang/dartdoc/blob/9e5a57dd1d0826d5a59fd074d8eb2637466a5c17/lib/src/markdown_processor.dart#L874

As @pq surmised, we take the first node of generated HTML for the summary (modulo a bunch of dartdoc special casing and stripping of script tags, etc). We discussed offline and my recommendation for lints that are likely to produce decent results in dartdoc regardless of localization or personal preference would be:

• Make sure the comment starts with text (no code blocks right at the beginning, or script tags, or raw HTML, etc).
• Make sure the first paragraph break happens in two lines or less.

I think narrowing it down to a "one sentence" summary is probably less general. Maybe a separate lint not enabled by default if we really want it?

[pq]

After chatting w/ @jcollins-g and @bwilkerson, I'm wondering if the following might not get us a lot of the way there:

1. a lint that flags docs whose summary paragraph is longer than some reasonable length. Notice how MethodInvocation wraps below. We could flag that and suggest they break the doc into summary and details.

   

2. a lint that flags doc summaries that contain stuff that wouldn't render well (e.g., doesn't start w/ text, maybe contains links?)

@goderbauer: thoughts?

[goderbauer]

I've never really seen 2 as a problem, so not sure how useful that one will be...

For 1 I'd be curious what we'd consider a reasonable length to not run into Python's annoying limitation of "this has to be 80 chars or less"...

ICU has some heuristics to detect sentence breaks based on Unicode recommendation: http://userguide.icu-project.org/boundaryanalysis#TOC-Sentence-Boundary Maybe that can be useful?

[pq]

I've never really seen 2 as a problem, so not sure how useful that one will be...

I'm not sure either to be honest but then the Flutter docs have benefited from extraordinary care. This may be more valuable to more slap-dash efforts.

For 1 I'd be curious what we'd consider a reasonable length to not run into Python's annoying limitation of "this has to be 80 chars or less"...

Me too. @jcollins-g: I'm curious if you have a gut sense for this?

Thanks for the ICU link. It would be interesting to see how many false positives we'd get with their heuristics. Would that there was a dart package implementation so we could do some quick testing!

[jcollins-g]

I've never really seen 2 as a problem, so not sure how useful that one will be...

I'm not sure either to be honest but then the Flutter docs have benefited from extraordinary care. This may be more valuable to more slap-dash efforts.

For 1 I'd be curious what we'd consider a reasonable length to not run into Python's annoying limitation of "this has to be 80 chars or less"...

Me too. @jcollins-g: I'm curious if you have a gut sense for this?

As above, my gut sense is "two lines" -- of any length. ~150 characters might also work.

Thanks for the ICU link. It would be interesting to see how many false positives we'd get with their heuristics. Would that there was a dart package implementation so we could do some quick testing!