[doc_imports] Differences with analyzer and Dartdoc's comment resolver

[kallentu]

General Problem

When looking for a reference with the same name as other references in scope, which should the comment reference resolver pick?

For example, this can happen when a class member doc comment references a name that is both a field and a parameter.

Resolution today

Dartdoc has its own algorithm to search through children nodes and parent nodes (universal reference resolving).

This algorithm can be seen in CommentReferable.referenceBy.

Resolution tomorrow

The analyzer has a different algorithm with Scope that makes for (IMO) better reference resolving, but this does mean that there are a couple of differences and changes.

I'll list out a bunch of examples where this happens with output from some dummy warnings I've generated.

Case 1: Analyzer prefers parameters over fields.

This should be an acceptable change that the analyzer prefers the Parameter. If we want the Field, we should have doc writers write [Class.field] as an escape hatch.

We might need to migrate the references that intend to be referencing fields.

An example of this is below:

https://github.com/dart-lang/sdk/blob/51d7b8477d4abe6cc54b203dcf6fae3a45f0c7db/sdk/lib/convert/html_escape.dart#L141

build-sdk-docs:   error: [analyzerRef DIFFERENT Parameter escapeSlash null other result is Field escapeSlash %%__HTMLBASE_dartdoc_internal__%%dart-convert/HtmlEscapeMode/escapeSlash.html]
build-sdk-docs:     from dart-convert.HtmlEscapeMode.HtmlEscapeMode: (file:///usr/local/google/home/kallentu/dart-sdk/sdk/out/ReleaseX64/dart-sdk/lib/convert/html_escape.dart:142:9)

Case 2: Analyzer prefers a field over parameter.

/flutter/packages/flutter/lib/src/material/theme_data.dart for [applyElevationOverlayColor]

• This chooses the right field over a parameter. I think this parameter actually come from the ThemeData factory constructor which makes no sense.

flutter-docs:   error: [analyzerRef DIFFERENT Field applyElevationOverlayColor material/ThemeData/applyElevationOverlayColor.html other result is Parameter applyElevationOverlayColor null]
flutter-docs:     from material.ThemeData.from: (file:///tmp/flutterOOFHUB/packages/flutter/lib/src/material/theme_data.dart:781:21)

/flutter/packages/flutter/lib/src/material/colors.dart for [value]

• This is picking a field of a super class which makes sense.

flutter-docs:   error: [analyzerRef DIFFERENT Field value dart-ui/Color/value.html other result is Parameter value null]
flutter-docs:     from material.MaterialColor.MaterialColor: (file:///tmp/flutterOOFHUB/packages/flutter/lib/src/material/colors.dart:100:9)

/flutter/packages/flutter/lib/src/foundation/diagnostics.dart for [hashCode]

• This is choosing the hashcode of Object which makes more sense than this unrelated RadioThemeData hashcode.

flutter-docs:   error: [analyzerRef DIFFERENT Field hashCode dart-core/Object/hashCode.html other result is Field hashCode material/RadioThemeData/hashCode.html]
flutter-docs:     from material.RadioThemeData.toStringShort: (file:///tmp/flutterOOFHUB/packages/flutter/lib/src/foundation/diagnostics.dart:3013:10)

Bonus Case 3: Analyzer finding references where Dartdoc couldn't before

This is more of a general win.

[File] in this doc comment wouldn't be linked, but is now linked to the correct class.

• Not completely sure why Dartdoc couldn't resolve this with a proper reference to the File class...
• https://github.com/dart-lang/sdk/blob/c09cb46304325cc59890ef685d33f5e022da047e/sdk/lib/io/file.dart#L602
• https://api.dart.dev/stable/3.3.4/dart-io/File/writeAsString.html

build-sdk-docs:   error: [analyzerRef DIFFERENT Class File %%__HTMLBASE_dartdoc_internal__%%dart-io/File-class.html other result is File null]
build-sdk-docs:     from dart-io.File.writeAsString: (file:///usr/local/google/home/kallentu/dart-sdk/sdk/out/ReleaseX64/dart-sdk/lib/io/file.dart:665:16)

[bwilkerson]

The analyzer has a different algorithm with Scope that makes for (IMO) better reference resolving ...

I agree. Using the language-defined scope lookup makes more sense, especially given that users already have to learn about scopes and shouldn't have to learn a different mechanism for deciding which names will be found.