Code Generation Fails for Single-Field Records

[rubenferreira97]

Describe the bug When generating code for a Riverpod provider that returns a Dart record with a single positional field, the generated code fails due to a missing trailing comma. Dart requires a trailing comma for single-field records to distinguish them from regular parentheses.

To Reproduce

import 'package:riverpod_annotation/riverpod_annotation.dart';

part 'providers.g.dart';

// Works!
// @riverpod
// class Test extends _$Test {
//   @override
//   (int, int) build() {
//     return (1, 2);
//   }
// }

// Fails :(
@riverpod
class Test extends _$Test {
  @override
  (int,) build() {
    return (1,);
  }
}

line 10, column 68 of .: A record type with exactly one positional field requires a trailing comma. 10 │ final class TestProvider extends $AsyncNotifierProvider<Test, (int)> {

Expected behavior

The generated code should handle single-field records correctly with a trailing comma: final class TestProvider extends $AsyncNotifierProvider<Test, (int,)>

[rrousselGit]

It's difficult. It'd require quite a lot of changes to fix that.

[rubenferreira97]

I understand that addressing this issue could be complex, especially if Riverpod is performing in-depth analysis of types and other stuff. However, I’m curious, why can't riverpod_generator capture the return type as a string returnType using something like getDisplayString (in this case, returnType = "(int,)") and then reuse it during the code generation process? I apologize if this is a naive approach, but I’m just looking for some insight on why this might not be feasible.

[rrousselGit]

That's what riverpod is currently doing, and that's what the problem is.

You see, getDisplayString isn't (int,) It's (int) in this case. Hence why there's no comma.

Riverpod would have to stop using getDisplayString and move on to a different approach. The most sensical would be using analyzer_plugin's ChangeBuilder. But that requires significant changes to how types are written.

[rubenferreira97]

You see, getDisplayString isn't (int,)

Oh 😶, now I understand why this is challenging. Isn't this a bug in the analyzer? (int, ) and (int) are two different things. Is there any justification?

[rrousselGit]

Technically not. Analyzer is free to use anything for getDisplayString, even non-valid types. It wasn't supposed to be used to write types. But analyzer had nothing else built-in, so I eneded-up relying on it.

[TekExplorer]

i think you'd just have to special-case records for this.

actually, are you getting the display string of the element or the type?

and whats the difference between getDisplayString() and displayName?

[rrousselGit]

Just passing by to say that this is on my radar. I've been working on a way for code-generator to better write code, which solves import prefixes and many other things