feat: Allow JsonConverter annotations to use constructor arguments an…

[taeilyoon]

#1536 Parameterized JsonConverter

Support for JsonConverter with Constructor Parameters

This PR implements support for JsonConverter classes with constructor parameters, allowing for more flexible and reusable converters.

Implementation details

This PR includes:

• Core Support: JsonConverter classes can now accept constructor arguments in the annotation (e.g., @MyConverter(123)).
• Tests: Added explicit tests for JsonConverter with int, String, List, and named parameters in test/src/json_converter_params_test_input.dart.

Things to discuss / consider

• Const Constructors: The generator assumes that JsonConverter annotations with parameters use const constructors. This is enforced by Dart for annotations generally.
• Complex Parameter Types: Currently verified for primitives and lists. Are there specific complex types (like Maps or other const objects) that might require additional handling or testing?
• Error Handling: Should specific warnings be added if users try to pass unsupported types, or is relying on existing analyzer errors sufficient?

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Summary of Changes

Hello @taeilyoon, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the json_serializable.dart package by introducing support for JsonConverter annotations with constructor parameters. This allows developers to create more dynamic and configurable converters, improving code reusability and flexibility in serialization logic. The implementation includes sophisticated parsing for various Dart literal types and is thoroughly validated with new test cases.

Highlights

• Support for JsonConverter Constructor Arguments: The core change enables JsonConverter annotations to accept constructor parameters, making them more flexible and reusable for various serialization scenarios.
• Robust Argument Parsing Logic: New utility functions (_parseArguments, _dartObjectToString) were introduced to correctly parse and represent various Dart object types (primitives, lists, maps, sets, enums) as constructor arguments, ensuring proper code generation.
• Expanded Test Coverage: Comprehensive tests have been added for different parameter types, including int, String, List, and named parameters, ensuring the new functionality works as expected across various use cases.
• Removal of Previous Restriction: The previous InvalidGenerationSourceError that disallowed JsonConverter annotations from having constructor arguments has been removed, reflecting the new supported functionality.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a significant feature: allowing JsonConverter annotations to have constructor arguments. The implementation correctly parses these arguments and reconstructs them as source code strings for instantiating the converter. This is handled well in the new _parseArguments and _dartObjectToString functions. The changes look solid, and the new test cases for primitive and list parameters are great. I've added one suggestion to also include test cases for Map and Enum parameters to ensure those more complex types are handled correctly, as you mentioned in the PR description.

[gemini-code-assist]

This is a great addition! As you mentioned in the pull request description, it would be beneficial to verify support for more complex parameter types. The implementation in _dartObjectToString includes logic for Map and Enum types, but there are no corresponding tests.

Adding test cases for converters with Map and Enum parameters would increase confidence in the implementation and prevent future regressions for these scenarios.

Here's an example of what you could add to this file (and remember to add the new class names to _expectedAnnotatedTests):

@ShouldGenerate(r'''
JsonConverterMapParams _$JsonConverterMapParamsFromJson(
  Map<String, dynamic> json,
) =>
    JsonConverterMapParams()
      ..value = const _ConverterWithMapParams({
        'a': 1,
        'b': null,
      }).fromJson((json['value'] as num).toInt());

Map<String, dynamic> _$JsonConverterMapParamsToJson(
  JsonConverterMapParams instance,
) =>
    <String, dynamic>{
      'value': const _ConverterWithMapParams({'a': 1, 'b': null})
          .toJson(instance.value),
    };
''')
@JsonSerializable()
@_ConverterWithMapParams({'a': 1, 'b': null})
class JsonConverterMapParams {
  late Duration value;
}

class _ConverterWithMapParams implements JsonConverter<Duration, int> {
  final Map<String, int?> param;

  const _ConverterWithMapParams(this.param);

  @override
  Duration fromJson(int json) => throw UnimplementedError();

  @override
  int toJson(Duration object) => 0;
}

enum _MyEnum { alpha, beta }

@ShouldGenerate(r'''
JsonConverterEnumParams _$JsonConverterEnumParamsFromJson(
  Map<String, dynamic> json,
) =>
    JsonConverterEnumParams()
      ..value = const _ConverterWithEnumParams(_MyEnum.beta)
          .fromJson((json['value'] as num).toInt());

Map<String, dynamic> _$JsonConverterEnumParamsToJson(
  JsonConverterEnumParams instance,
) =>
    <String, dynamic>{
      'value':
          const _ConverterWithEnumParams(_MyEnum.beta).toJson(instance.value),
    };
''')
@JsonSerializable()
@_ConverterWithEnumParams(_MyEnum.beta)
class JsonConverterEnumParams {
  late Duration value;
}

class _ConverterWithEnumParams implements JsonConverter<Duration, int> {
  final _MyEnum param;

  const _ConverterWithEnumParams(this.param);

  @override
  Duration fromJson(int json) => throw UnimplementedError();

  @override
  int toJson(Duration object) => 0;
}