linter icon indicating copy to clipboard operation
linter copied to clipboard

proposal: `unintented_html_in_doc_comment`

Open srawlins opened this issue 8 months ago • 4 comments

unintented_html_in_doc_comment

Description

Sometimes a developer may neglect to put a code reference with angle brackets in backticks, within a doc comment, and write something like /// Returns a List<int>. The angle brackets are interpreted as HTML, and so the type argument(s) generally get swallowed by the browser, and not displayed.

The lint rule would catch angle brackets not contained within a code block, contained within a code span, or used as the delimiters in an autolink.

Details

I am recommending a linter rule rather than a static warning because there is a fair bit of opportunity for false positives, unless linter parses with the markdown package. This may or may not be on the table...

Without the markdown package, and rigorous parsing of doc comments, things will slip through.

In any case, I think the lint rule would catch any < which was determined to not be in a code block, code span, or autolink. We might need to allow HTML blocks and raw HTML, which are allowed by Markdown broadly, and I believe we allow some raw HTML; it might be an allowlist of tag names.

Kind

Guards against errors in documentation.

Bad Examples

/// Text List<int>.
/// Text [List<int>].
/// <assignment> -> <variable> = <expression> 

Good Examples

/// Text `List<int>`.
/// `<assignment -> <variable> = <expression>`

Discussion

Discussion checklist

  • [ ] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • [ ] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
  • [ ] If there's any prior art (e.g., in other linters), please add references here.
  • [ ] If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
  • [ ] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.

srawlins avatar Oct 25 '23 06:10 srawlins

I think this is a great idea!

You mention a potential allow list of HTML tags. What tags are you thinking? I personally think doc comments shouldn't have any raw HTML (without some very specific opt-in such as the {@inject-html} directive or synax that can be converted to another format) as they can be and are rendered in non-HTML environments.

Note on the name. I guess you mean unintended_html_in_doc_comment :)

parlough avatar Oct 25 '23 17:10 parlough

You mention a potential allow list of HTML tags. What tags are you thinking?

I don't think there is any design or decision-making here, just some investigation:

  • If GitHub allows some tags to be used in an HTML block, then pub.dev should allow the same for package READMEs, and dartdoc should allow the same for doc comments; I think this is implemented already.
  • Same with inline HTML. It might be the same allowlist.
  • I think all of this might be implemented in package:markdown...

Given that state, the lint rule should just report on text that looks like an HTML tag, which does not match the allowlist. So presumably <script> won't pass, and <int> in List<int> won't pass.

srawlins avatar Oct 25 '23 19:10 srawlins

Bumping this up again. If anyone has any objections, let us know. Otherwise, will try and see if I can work on this the upcoming week.

cc @srawlins @pq

kallentu avatar Feb 27 '24 23:02 kallentu

Awesome. Thanks, @kallentu!

pq avatar Feb 28 '24 17:02 pq

Fixed by @kallentu with https://github.com/dart-lang/sdk/commit/cd4b418628e5ef8db6b7bc11e1619def48313e3e and https://github.com/dart-lang/sdk/commit/be0cb54a9e5e801a2f54972f2d3c06550cf6df84

srawlins avatar Apr 23 '24 16:04 srawlins