comment_references doesn't recognize named fields in records (false positive?)

[albertms10]

Describe the issue

The comment_references lint does not recognize referenced named fields in records present in function parameter types or return signatures.

It is unclear whether the current behavior is intentional, as functions are not directly responsible for record fields declared in their type signature—which becomes more evident when using typedefs. This prompts the question: how should documentation be handled in this case?

To Reproduce

/// [record] works, but neither does [key] or [value].
//                                    ^^^      ^^^^^ The referenced name isn't visible in scope.
({int value}) retrieveKey(({int key}) record) => (value: 1);

But what should happen here? Where do we expect to document these fields?

/// [key] neither works here.
//   ^^^ The referenced name isn't visible in scope.
typedef KeyRecord = ({int key});
typedef ValueRecord = ({int value});

/// [record] works, but neither does [key] or [value].
//                                    ^^^      ^^^^^
ValueRecord retrieveKey(KeyRecord record) => (value: 1);

Expected behavior

In the reproducible example, both [key] and [value] references should be considered visible in scope—and thus not marked by the lint—as they are present in the function signature.

[bwilkerson]

There are a couple of reasons why I'm thinking that we don't want to support using named record fields in doc comment references.

The first is because record are structurally typed, so the reference can easily be ambiguous. See dart-lang/sdk#53024 (comment) for an example of what I'm talking about.

The second is because record fields are somewhat analogous to fields in objects, and we don't support references to object fields in doc comment references. For example, I wouldn't expect that we'd want to support writing

/// I can reference [c], but not [f].
void f(C c) {}

class C {
  int f;
}

and have the reference to f somehow link to the reference to the field f in the class C.

I can certainly see wanting to document what the record fields represent, or what values they should have, but I don't think we want to try to support navigation / linking (in dart doc) for such comments.

[bwilkerson]

An interesting case that was just brought to my attention:

/// Document this type.
typedef MyRecord = ({int a, int b});

where it does seem reasonable to be able to document the fields of the type in the documentation of the typedef.

[srawlins]

That case is also in the original issue text. I agree with the assessments in each of your comments.

[lrhn]

What [foo] means should be decided by the DartDoc tool, and the lint should then use the same logic to check that all references in the code does point to something, even if it doesn't have to know what. (If it can reuse some of the implementation of that logic, that'd be perfect.)

If [key] and [value] has something they can link to, then they can be links. Even if it can be ambiguous in some cases, it might not be in other, and a heuristic interpretation might be what users want.
For the key, you could probably write [record.key]. That at least tells precisely which key it's referring to (and we can use [someList.length] to refer to List.length of the parameter someList today). There is no similar thing to do for the return type.

In any case:

• Does DartDoc make those into links today? — If so, the lint should just match the actual behavior.
• If not, should it (and to what)? — That should probably be discussed in the DartDoc issue tracker.

[bwilkerson]

(Given that the same team is currently responsible for both tools, I'm fine having the discussion remain here.)

[bwilkerson]

Extending the semantics of references to allow references to members of objects and records is interesting. I wouldn't want us to generalize to include arbitrary expressions as references, and I'd be hesitant to allow multiple member accesses (such as record.key.length), but single level member accesses isn't inconsistent with what we do today for static members.

[srawlins]

I've opened dart-lang/sdk#55584 for the issue of whether the analyzer should be enhanced to recognize such inner identifiers as referable.

This issue is closed though; the comment_references rule correctly calls out these fields as not referable. (Backticks should be used instead of square brackets to refer to the field names.)