New warnings with `json_serializable>=6.11.0` (conflicting parameters)

[walsha2]

Describe the bug

This only started happening since json_serializable>=6.11.0. A whole host of warnings are now reported when running code generation. No other changes. Currently using freezed==3.2.0

Nothing breaks, the code still generates the same way, but there are now lots of warnings.

To Reproduce

Take the following:

@freezed
abstract class MyResponse with _$MyResponse {
  const MyResponse._();

  const factory MyResponse({
    @JsonKey(name: 'docs_url')
    @Default('http://localhost:8000/docs')
    String docsUrl,
  }) = _MyResponse;

  factory MyResponse.fromJson(Map<String, dynamic> json) =>
      _$MyResponseFromJson(json);
}

Then generate code:

dart run build_runner build --delete-conflicting-outputs

Will yield the following warning:

W Field `docsUrl` has conflicting `JsonKey.name` annotations: 
both constructor parameter and class field have this annotation. 
Using constructor parameter value.

Here is the generated code:

mixin _$MyResponse {

@JsonKey(name: 'docs_url') String get docsUrl;
/// Create a copy of MyResponse
/// with the given fields replaced by the non-null parameter values.
@JsonKey(includeFromJson: false, includeToJson: false)
@pragma('vm:prefer-inline')
$MyResponseCopyWith<MyResponse> get copyWith => _$MyResponseCopyWithImpl<MyResponse>(this as MyResponse, _$identity);

  /// Serializes this MyResponse to a JSON map.
  Map<String, dynamic> toJson();


@override
bool operator ==(Object other) {
  return identical(this, other) || (other.runtimeType == runtimeType&&other is MyResponse&&(identical(other.docsUrl, docsUrl) || other.docsUrl == docsUrl));
}

@JsonKey(includeFromJson: false, includeToJson: false)
@override
int get hashCode => Object.hash(runtimeType,docsUrl);

@override
String toString() {
  return 'MyResponse(docsUrl: $docsUrl)';
}
}

This is true for other keys as well, e.g. JsonKey.includeIfNull. The new version of json_serializable is now reporting this as a warning.

Expected behavior

No warnings to be emitted.

[geekswamp]

+1

[liasica]

same issue here

[rrousselGit]

This is ultimately pretty similar to invalid_annotation_target that already happens when using json_serializable.
Overall json_serializable just doesn't like what Freezed is doing with factory constructors.

[matteo-convert]

putting invalid_annotation_target: ignore in analysis_option.yaml change Nothing, it's not really serious but it will be better if it not appear

[matanshukry]

@rrousselGit Just wondering - why does freezed need to put the annotation on both the field and the constructor? why can't freeze "listen" to the json_serializable warning and put it on just one of them?

[rrousselGit]

Freezed's annotation logic is independent from JsonSerializable.
The specs are that any annotation placed on a constructor also applies it to the generated copyWith/field/...

This is most likely how Primary Constructors would work in Dart, and enables things like:

factory Person(
  @deprecated String name,
);
....

Person p;
p.name; // deprecated;
p.copyWith(name: // deprecated too

The behavior is as such technically correct.
The issue here is more so that we need a way of silencing the warning. I'm not aware of one.

---

Lately I've been considering this:

@Apply(annotation)

For example:

@freezed
class Person with _$Person {
  @Apply(JsonSerializable());
  factory Person() = _Person;
}

As a syntax for "apply this annotation to the generated code".

It doesn't seem to trigger the warning. The main issue is that tomorrow JsonSerializable could decide that this too is illegal, and we'd be back to square one.

The only safe bet would be to rely on Strings instead:

@Apply('JsonSerializable())'

But then we loose syntax highlighting, autocomplete, compilation errors, and much more.

[rrousselGit]

Alternatively an extension type seems to silence the warning.

You could maybe do:

class Person {
  @Custom(JsonSerializable())
  factory Person(String value) = _Person;
}

extension type const Custom(JsonSerializable annotation)
    implements JsonSerializable {}

[matanshukry]

Freezed's annotation logic is independent from JsonSerializable. The specs are that any annotation placed on a constructor also applies it to the generated copyWith/field/...

This is most likely how Primary Constructors would work in Dart, and enables things like:

factory Person(
@deprecated String name,
);
....

Person p;
p.name; // deprecated;
p.copyWith(name: // deprecated too
The behavior is as such technically correct. The issue here is more so that we need a way of silencing the warning. I'm not aware of one.

In the specific annotation/use-case you provided yes, but honestly I'm not convinced that's the case for all annotations.
I can see for example annotations that are for validations, where one would want them to have a value in the constructor, but not on the field itself. possibly even different ones for differnt constructors.

Perhaps we can utilize the custom thing you mentioned in your last comment, and also add a field on it describing if user wants to copy the annotation to both the ctor and the field?

Although bottom line - It's just a warning, and at this point the solution isn't simple enough to worth spending time on.

[amrgetment]

Maybe you could ask the json_serializable team for support or open an issue explaining your problem.

@davidmorgan Sorry for tagging you — do you have any suggestions?

[davidmorgan]

For the json_serializable warnings: it could skip the warning if both annotations have the same value?

Personally I don't like build warnings, I would prefer if it fails if it's impossible to generate valid code and succeeds if it is possible.

If you want to open a PR against json_serializable I can add my thumbs up :) ... I could take a look myself at some point but I'm not sure when I can get to it.

[biplob-6amtech]

Is there any update?