unintended_html_in_doc_comment false positives?

[goderbauer]

In the following doc comment the unintended_html_in_doc_comment lint fires on [Tween<Offset>] and Tween<T> even though the docs render correctly.

/// You can also use [Tween<int>].
/// And have you seen `Tween<T>`?
class Tween<T> {
}

(For the correct rendering of these references search for them on https://main-api.flutter.dev/flutter/animation/Tween-class.html).

At least the reference in ` should not trigger the lint (which would be in line with what is documented: https://dart.dev/tools/linter-rules/unintended_html_in_doc_comment).

I was a little surprised that the reference in [] triggers the lint (as documented) since it seems to render totally fine.

[goderbauer]

Slight correction: For the ` case the lint only triggers in one particular case: https://github.com/flutter/flutter/blob/70460854d1831990b5784c91f0982a95012f9722/packages/flutter/lib/src/animation/tween.dart#L224

Maybe because the previous ` section spans over multiple lines?

All other ` instances seem to not fire the lint as expected.

[goderbauer]

Yeah, the lint seems to have a problem with multi-line ` sections. Here is another example where it triggers incorrectly because of that: https://github.com/flutter/flutter/blob/70460854d1831990b5784c91f0982a95012f9722/packages/flutter/lib/src/painting/shader_warm_up.dart#L75

Rendering is fine: https://github.com/flutter/flutter/blob/70460854d1831990b5784c91f0982a95012f9722/packages/flutter/lib/src/painting/shader_warm_up.dart#L75

[goderbauer]

So the two issues are:

• `sections that span over mutliple lines
• references in [] (like [Tween<int>]). We have about 170 of those in flutter. They are kind of a handy and compact way to document the expected type of the generic while also linking to the class.

[srawlins]

I wish we supported [Tween<int>] in the analyzer / an IDE, as I also think it's really useful to be able to write types with type arguments. We have a ticket for this support.

I haven't gotten it in because it's actually a fairly complex parsing feature, like we'd want to support [Map<String, List<int>>], and [String Function(int, String Function(List<int>))].

[bwilkerson]

Correct me if I'm wrong, but I think it's more than a parsing feature. We have a parser that knows how to parse type annotations, so if that's all this required it seems like that wouldn't be too hard. But the feature also requires resolution, which might be the harder part.

[lrhn]

See dart-lang/sdk#56465 for the same issue, and https://dart-review.googlesource.com/c/sdk/+/379519 for fixing the List<int> case when it's on one line.

It still lists [List<int>] as "bad".

To repeat dart-lang/sdk#56465, the issue here is that the lint uses a RegExp to match a language that probably isn't entirely regular (even if its tokens might be). When doing that, it's necessary to correctly match all tokens that can contain something that looks like what you're looking for, but where it means something else.

Here [List<int>] is not Yaml-parsed as a Yaml link. Not unless it's a link shortcut to a declaration of [List<int>]: http://example.com, but let's just assume it isn't. Which means that to DartDoc, the content of the [...] should be rendered as markdown inline content.
That's also true for the content of [...] in [X<sup>2</sup>](https://example.com), but not the content of (...).
And for the first [...] of [X<sup>2</sup>][Banana], which is also a valid link.

That means that just ignoring the content of [...] in the lint (fairly easily doable) isn't correct. That would give false negatives for, say [List<int>] ..... \n[List<int>]: http://example.com.

One option is to ignore the content of a [...] that is not followed by [, or (, and ^[...]:.... It would not detect a [List<int>] with a [List<int>]: ... link, but it would trigger on the link declaration then.

Also tricky because ] has lower precedence than `, so [List < `Dist] banana` should not parse [...] as a link. Yaml-parsing has precedence, and allows nested `s and [...]s.
Basically, to be absolutely correct, this needs to do proper YAML parsing, until then it's approximating a non-regular language with a RegExp, and there will be false positives or negatives to some degeree. We'll just have to decide where we draw the line between realistic and unrealistic cases, and handle the realistic ones.

(The HTML-tag detection would now probably not trigger on [Map<String, int>] because the , after String is not valid HTML.)

[lrhn]

Yeah, the lint seems to have a problem with multi-line ` sections.

It does, since the RegExp is applied to each line independently. Yaml parsing is per-block, so the RegExp has no chance of knowing how something should be tokenized. An unmatched ` is allowed.

To give more correct results, the lines should probably be concatenated, at least per paragraph.

[Leptopoda]

I'm facing a similar bug with:

  /// `<`, `>`
  /// `<>`
  /// `<`
  /// `>`
  final String name;

The first line fires the lint while all other lines do not.

[srawlins]

Sorry @Leptopoda I cannot reproduce. That issue should have been fixed with dart-lang/sdk@df283fd.

If you still face a problem, please open a new issue.