Skip to content

Record Fields & Comment References #60452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RohitSaily opened this issue Apr 1, 2025 · 12 comments
Open

Record Fields & Comment References #60452

RohitSaily opened this issue Apr 1, 2025 · 12 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-server Issues related to some aspect of the analysis server P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@RohitSaily
Copy link
Contributor

Dart SDK version: 3.8.0-243.0.dev (dev) (Sat Mar 29 05:02:04 2025 -0700) on "macos_x64"

Documentation comment references to record fields are not recognized. It would be useful to have them recognized. This includes the autogenerated names for positional fields e.g. $1.

Example 1 - typedef

/// [a] <- flagged as `comment_references` violation
typedef A=({int a});

/// [b] <- OK
final class B
{     final int b;
}

My conception of records is that they function like an anonymous class with final fields. I expect it to be treated that way with documentation comments in that the fields can be directly referenced through a type definition of it.

Example 2 - Record Field

/// [field.a] <- flagged as `comment_references` violation
extension type A(({int a}) field)
{}

I expect to be able to identify the record field via the extension type field.

Example 3 - Function Return Types

I got this directly from @stereotype441's issue (#59526), copying and pasting their examples here for convenience.

Example 3a

/// Returns a list of [Foo] and a list of [Bar].
///
/// [foos] is the list of [Foo] and [bars] is the list of [Bar].
({List<Foo> foos, List<Bar> bars}) getFoosAndBars();

Example 3b

/// Returns a list of [Foo] and a list of [Bar].
///
/// [$1] is the list of [Foo] and [$2] is the list of [Bar].
(List<Foo>, List<Bar>) getFoosAndBars();

All the references of their examples should be valid.

@RohitSaily RohitSaily added the area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. label Apr 1, 2025
@FMorschel
Copy link
Contributor

FMorschel commented Apr 1, 2025

See #60332

Basically from #60332 (comment):

Thinking about this a bit more, I don't think this is doable right now. I once opened Dart-Code/Dart-Code#5294 for something related to record fields but we still have #53024 opened to support renaming them. While the last one is not closed, I'm not sure this will be possible.

@FMorschel
Copy link
Contributor

Meaning, while we can't rename them (#53024), these references, even when possible (for named cases), are not useful currently.

@bwilkerson
Copy link
Member

My conception of records is that they function like an anonymous class with final fields.

They don't.

An anonymous class is still a class, and classes in Dart define a nominal type. If you define two anonymous classes with the same base class you've defined two distinct classes. That is, the instances of those two classes have different types.

Record types, on the other hand, are structural types. Two records with the same number and types of fields (and the same names for named fields) have the same type. Whether the developer wants them to or not. For example, given

void f((int, int) point, (int, int) range) {}

The parameters point and range have the same type ((int, int)), even though they are probably semantically different.

This would also be true is the fields were named and just happened to have the same names.

Some operations that make perfect sense for nominal types just don't make sense for structural types. For example, renaming the first field in a record type of the form ({int a}) would presumably mean renaming the first field for all records (and record types) with a single field named a whose type is int. But if a developer used the same structure of record for two different purposes they're unlikely to want both "types" of records to be renamed. (I put "types" in quotes because what I really mean is "types that are distinct in the developers mind but are the same according to Dart's semantics".)

@RohitSaily
Copy link
Contributor Author

RohitSaily commented Apr 2, 2025

It makes sense why some operations such as mass renaming does not make sense, as explained in the issues @FMorschel mentioned and based on what @bwilkerson just explained about structural typing.

Meaning, while we can't rename them (#53024), these references, even when possible (for named cases), are not useful currently.

I agree with this in regard to names specified for positional fields but not for fields formally identified through either (1) a custom name declared via curly brace syntax, or (2) the $ prefixed identifiers for positional fields e.g. $1.

Records are a convenient alternative for grouping data when the functionality of classes are not needed. However there are still fields whose purpose needs describing or invariants need specifying. Being able to reference the fields helps make the documentation clear and link to relevant declarations.

For example, if one uses a specifically structured record throughout a library they will likely have it as a typedef or even represented as a field, which may be discussed by other API. When it is discussed, the linking and formal specification enhances the documentation's utility.

Even without the linking, being able to formally specify the identifiers in documentation comments and have them set apart from regular text, as seen in example 3 of the original post, is useful for semantic and syntactic clarity.

@bwilkerson
Copy link
Member

I understand what you're saying, and I don't disagree that it's useful to add documentation about the way the record is expected to be used, but there are some questions we'd need to answer before we could consider supporting this.

First, dartdoc currently allows you to use [identifier] to refer to an identifier in scope in the body of the function. That includes parameters, but doesn't include fields on the values of the parameters. For example, given

void f(List<int> p) {}

the docs can refer to the parameter as [p], but can't use [length] to reference the length field on the list because it isn't in scope. We also don't support this kind of reference for function types. Given

void f(int Function(int x) g) {}

the docs can refer to the parameter as [g], but can't use [x] to reference the first parameter on the function because it isn't in scope.

I don't think we want to start artificially extending the "scope" used for documentation comments because it will result in portions of parameter types shadowing declarations from the enclosing scope, which would be a breaking change. For example, given

int length = 3;

/// Print a message if the [length] of [p] is an odd number.
void f(List<int> p) {}

the reference to length would start referring to the field of the list p rather than the top-level variable.

Second, we'd have to decide how to deal with conflicts. For example, given

void f((String first, String last) name, (String first, String last) productPreferences) {}

Which record field would [first] refer to?

Or given

void f(List<int> list, Map<String, int> map) {}

would [length] refer to the length of the list or the length of the map?

I'm just not convinced that any of the possible answers are clear enough to not create confusion in some cases.

@FMorschel
Copy link
Contributor

First, dartdoc currently allows you to use [identifier] to refer to an identifier in scope in the body of the function. That includes parameters, but doesn't include fields on the values of the parameters.

I don't think that for parameters we should allow this following your examples. If we do this, we should ask the user to identify:

/// This refers to [list.length] and this to [map.length]
void f(List<int> list, Map<String, int> map) {}

It would be a different conversation when talking about typedefs IMO.

@bwilkerson
Copy link
Member

... If we do this, we should ask the user to identify:

That's effectively allowing arbitrary expressions in doc references (as opposed to allowing identifiers, which is what's currently supported). Then the next question is, what kind(s) of expressions do we allow. This is a slippery slope.

It would be a different conversation when talking about typedefs IMO.

Why?

@FMorschel
Copy link
Contributor

FMorschel commented Apr 2, 2025

... If we do this, we should ask the user to identify:

That's effectively allowing arbitrary expressions in doc references (as opposed to allowing identifiers, which is what's currently supported). Then the next question is, what kind(s) of expressions do we allow. This is a slippery slope.

Agreed, this would be weird. If we ever agree to this, I'd say we should only allow this for structural types since it would mean the value is visible, no reason to reference Map.length from a variable with that type, I don't think.

It would be a different conversation when talking about typedefs IMO.

Why?

This request and #60332 are asking for the same thing (I think), to reference the values inside a typedef for a record type. It is not the same because we can't reference anything (like in your examples, the parameter name) to get that information from a Record "variable".

This would also mean that considering (copied example from #60332 but with different names):

/// - [a] BAD: The referenced name isn't visible in scope.
/// - [b] BAD: The referenced name isn't visible in scope.
/// - [T] GOOD: The referenced name is visible in scope.
typedef PositionalRecord<T> = (
  int a,
  String b,
);

/// - [a] BAD: The referenced name isn't visible in scope.
/// - [b] BAD: The referenced name isn't visible in scope.
/// - [T] GOOD: The referenced name is visible in scope.
typedef NamedRecord<T> = ({
  int a,
  String b,
});

The PositionalRecord would have no differentiation (see dart-lang/language#4309 for a request concerning typedef differentiation) from a simple (int b, String a) anywhere else, similar to a FunctionType with the same positional parameter types (e.g.: void Function(int a) and void Function(int b)).

IMO, this is very similar to what we currently have with typedefs for FunctionTypes and it would be more consistent if we added this behaviour. Another request would be to add the behaviour you mentioned above.

@RohitSaily
Copy link
Contributor Author

@bwilkerson:

I don't think we want to start artificially extending the "scope" used for documentation comments because it will result in portions of parameter types shadowing declarations from the enclosing scope, which would be a breaking change. For example, given [...]

I agree that would definitely be bad and should not be done.


@FMorschel:

I don't think that for parameters we should allow this following your examples. If we do this, we should ask the user to identify:

/// This refers to [list.length] and this to [map.length]
void f(List list, Map<String, int> map) {}
It would be a different conversation when talking about typedefs IMO.

@bwilkerson:

That's effectively allowing arbitrary expressions in doc references (as opposed to allowing identifiers, which is what's currently supported). Then the next question is, what kind(s) of expressions do we allow. This is a slippery slope.

@FMorschel:

Agreed, this would be weird. If we ever agree to this, I'd say we should only allow this for structural types since it would mean the value is visible, no reason to reference Map.length from a variable with that type, I don't think.

Semantically list.length identifies the field on the instance instead of the class so it is an identifier in a sense. For that reason, I believe that should be permitted. This would allow referencing record fields via $ prefixed fields or formally named fields, and make documentation less ambiguous and more concise at the same time.

/// [a.length] must be strictly less than [b.length].
void f(List<int> a, List<int> b)
{	assert(a.length<b.length, 'Obey the docs!')
}

References such as Map.length should be used when the field needs to be referenced, but there is nothing to refer to it. It's kind of like a static reference i.e. referencing the definition/declaration and not the value of an instance.

Optionals would have to be considered to prevent ambiguity and keep syntax consistent but I believe it can end there. So . and ?. access would be permitted and nothing else. With that references are still pretty strict and things become more intuitive and expressive.

/// [r.count] can be referenced, and so can [r.point.x]!
void f(({int count, ({int x, int y}) point})) r)
{}

/// [Alias.$1] is an integer and so is [Alias.b]!
typedef Alias=(int a, {int b});

@bwilkerson
Copy link
Member

Semantically list.length identifies the field on the instance instead of the class so it is an identifier in a sense.

It isn't an identifier in Dart, it's the invocation of a getter, which is an expression. And I don't think we want to allow expressions in reference blocks.

References such as Map.length should be used when the field needs to be referenced, but there is nothing to refer to it. It's kind of like a static reference ...

It is a static reference, and because there's no static member named length it would be considered to be an invalid reference. I don't think we want to make comment references not adhere to the normal rules for Dart.

@lrhn
Copy link
Member

lrhn commented Apr 2, 2025

And I don't think we want to allow expressions in reference blocks.

You don't need to allow any expression, just identifier selector*.

Today I have to write something like [`list.length`](List.length) (if that even works) or just "the [List.length] of list".

If DartDoc knows the static type of list, then it could treat [list.length] as a link to List.length, and make even further selectors. (Already allow [List.add()] to refer to List.add.)

(Is there any way to make a DartDoc link to a declaration and control the link text? [`foo.bar`][Foo.bar] or something, out does that not work because DartDoc links are not passed through markdown?)

@bwilkerson
Copy link
Member

Is there any way to make a DartDoc link to a declaration and control the link text?

As far as I'm aware, no. Not without an ugly hack based on figuring out the link that DartDoc would have produced and using it explicitly, but that's too fragile so I wouldn't recommend it.

@srawlins srawlins added devexp-server Issues related to some aspect of the analysis server type-enhancement A request for a change that isn't a bug P3 A lower priority bug or feature request labels Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. devexp-server Issues related to some aspect of the analysis server P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants