Add aliases to JsonValue to enum value to be decoded from different JSON values

[mateusfccp]

Solves #1380.

Adds a parameter to JsonValue called aliases, which accepts a list of values that can be alternatively used to decode a JSON.

Example:

enum StatusCode {
  @JsonValue(200, aliases: [201, 202])
  success,
  @JsonValue(301)
  movedPermanently,
  @JsonValue(302)
  found,
  @JsonValue(500)
  internalServerError,
}

With the code above, all of 200, 201 and 202 would be decoded as StatusCode.success.

When calling toJson, however, the value parameter is used. In this case, 200.

Notes

• The $enumDecode and $enumDecodeNullable were not changed because they are public and there are packages that depend on them. Instead, $enumDecodeWithDecodeMap and $enumDecodeNullableWithDecodeMap were introduced. We may want to deprecate the former at some point and remove them in a major version.

[kevmoo]

Ideally we'd have a couple of tests to validate the failure cases, but I get that those can be tricky to add.

[mateusfccp]

@kevmoo What would be the failure cases for this PR? Can you give an example?

[kevmoo]

@kevmoo What would be the failure cases for this PR? Can you give an example?

Unsupported types in the set is the big one.
null isn't supported, right?
Those types of things.

[mateusfccp]

@kevmoo What would be the failure cases for this PR? Can you give an example?

Unsupported types in the set is the big one. null isn't supported, right? Those types of things.

Ah, yes, the idea is to mimic value, so until we have proper union types in Dart, we use Object? but should only accept String and int.

I am going to work on this test.

[kevmoo]

You'll need to rebase this fix.

[mateusfccp]

You'll need to rebase this fix.

I rebased to master and included the requested test.

In my local machine, a test was failing, but I didn't change anything related to it (the *ToJson generator).

[mateusfccp]

Mmm, I had made a regular merge, but GitHub is saying to me that "this branch must not contain merge commits", so I rewrote the commits using a rebase instead, but it is still giving me this message.

[mateusfccp]

Well, the test that did not pass locally passed here, so we are good with it.

[mateusfccp]

@kevmoo Is there something preventing the code review from proceeding?

[kevmoo]

I don't like the repeated map EVERYWHERE for one-off feature.

Could you noodle on that a bit?

[kevmoo]

Undo this rename. Makes reviewing more tricky.

[mateusfccp]

Done.

[kevmoo]

So we're changing the generated code and creating a NEW map, even if the dev doesn't use the new feature?

[mateusfccp]

@kevmoo Yes, basically.

The old map is left as is. However, in the current code, the map is used both for encoding and decoding. Decoding is done by linearly checking if an entry has the expected value for that key. It works similar to a Bimap.

By using two maps, we have O(1) access to the expected decoded value. But this is only a consequence of this approach, and I don't even think it will make any difference in practice unless someone decodes an outstanding quantity of Json values.

The main point is that if we introduce aliases, we have an asymmetric relationship between the two sets (the sets of encoded and decoded values). We want encoding to have a single resulting value (it is the one defined by the first parameter of the JsonValue constructor), but it can be decoded by more than one value (which is this same value + the ones declared in the alias parameter).

Because of this, we can't work with a single map anymore unless:

• We revert the map and key values, so we avoid duplicated keys.
• We guarantee that the first value is always the one that is expected when encoding.

Another approach could be to use two maps, the one we already have and an optional map aliases that will only be generated if aliases are used. Then, in the $enumDecodeWithDecodeMap function, we accept an optional aliases map as parameter, which, if passed, will first check for the entries like it is doing today and, if not found, it will try to find it in the passed aliases map (if any), falling back to a runtime error as it is today.

This could work, but I don't think it is a better approach than what I chose to use on this PR. I just splits the map into two, which guarantees O(1) access and a clear distinction between encoding/decoding expectations.

Maybe you have a third idea about how to tackle this issue?

[mateusfccp]

(Ps. the $enumDecode function is not even used anymore, I just didn't remove it entirely because I don't know if it would constitute a breaking change. Supposedly not, because we don't expose it in the public API, but I wanted to check with you before. If we are good, $enumDecodeWithDecodeMap can be renamed to $enumDecode, as it replaces its purpose completely with this approach)