Adding case insensitive to enum based on loading JsonEnum config

[matanshukry]

Fixes issue #927

Problem

Currently when parsing enum we compare thme using the equal operator. Enum values that differ only in cases (e.g. 'sunday' and 'SuNDaY') are not equal, and result in parsing error.
The user should have the possibiliy to decide on the comparator, at least in terms of case insensitive.

Solution

The solution suggested in this PR is:

1. Add caseInsensitive flag to the JsonEnum class.
2. Instead of comparing using == operator, we use a comparator function.
3. Create 2 comparators; the default one, using == operator, and a second one that assumes the value is String, and implementation check if the other object is also string and the .toLowerCase() on both of them is equal.
4. the comparator function defaultt, if null is provided, is using the == operator.
5. When creating the call to the enum deserializer method, $enumDecodeNullable or $enumDecode, we add an argumnet to use the 2nd case-insensitive operator, if the caseInsensitive flag exists.

Improvements

1. Optimization. Currently we go over each value in the map and check it. We can easily use a map, for both the regular case and case-insensitive checks. Not implemented here though as using a map isn't used regardless of the case insensitive case either way.
2. Extra check for type: currently we don't check the value type is String, but we assume it is like that. We could possibly add that type check, although not sure if that's necessary.

Extra notes

• I tried adding a test to this PR as well, but I couldn't check that. Specifically it looks like the generated files are bundled and I'm not sure how to generate them. I tried to simply run the generator in the directory, but it results in bigger differences, and I didn't wan to change it manually. I did verify it's working by running this changed code against my own external repo, and ran the generated code.

[matanshukry]

@kevmoo ?

[kevmoo]

Happy to look at this again. Will need a rebase! See my other comments!

[matanshukry]

@kevmoo merged with master and fixed the comments. Please take a look again

[kevmoo]

@matanshukry – you'll need to do a rebase and look at the analyzer errors

[kevmoo]

I think this is headed in a good direction, though!

[matanshukry]

@matanshukry – you'll need to do a rebase and look at the analyzer errors

Currently there are 347 issues - most of which are errors - when I analyze the master branch 😓 (flutter analyze)

Any specific errors you want me to take a look at, that's related to this PR?

[kevmoo]

@matanshukry – you should fix all of them. 😄

[matanshukry]

@matanshukry – you should fix all of them. 😄

What 🤔

Why should I fix all of the errors that exists today in the repo? this PR is about introducing a new fix, not fixing the issues the repo already has.

[kevmoo]

@matanshukry – we are green at HEAD - https://github.com/google/json_serializable.dart/actions/runs/4995588862

[matanshukry]

@matanshukry – we are green at HEAD - https://github.com/google/json_serializable.dart/actions/runs/4995588862

@kevmoo locally I still a lot of issues, even on master. I'm using dart 3.0.0 so that should be the same, but I am using windows (vs linux on the machines).

You've now let the CI run so I can see the errors through that (and working on it now), but if you have a better way of checking it locally on windows that would be great 🙏

[kevmoo]

@matanshukry – just need to check analyzer, formatter, and run tests locally

[matanshukry]

@kevmoo looks like the result locally are different than the CI though, at least for the analyzer.

For the testing - how do you generate the .g file, specifically for json_test_example.dart ? since it's in the test directory, using the regular build runner doesn't work. Unless I'm missing something.

[matanshukry]

For anyone interested: So apparently the example lib has overrides for path (I tried to just change the version without the override to path, it didn't work). I then copied the example lib (and another common file that's needed), and ran build on that directory.

@kevmoo Regardless, the tests in the annotation won't succeed because it doesn't even recognize the existence of the field caseInsensitive, since it uses the published version and not the path one. I'm assuming that means we need to first publish the annotation version, and then upgrade the serializable to use the annotation.

Hence I've created a separate PR for the annotation only:
#1323

Let me know if there's another method you would like to do that.

[kevmoo]

Just add (path) dependency overrides. You want to test EVERYTHING before you publish ANYTHING.

[matanshukry]

Just add (path) dependency overrides. You want to test EVERYTHING before you publish ANYTHING.

Already did that locally, that's not the issue. All tests passed when I do that.

I'm assuming you don't want that added to this PR though.

[matanshukry]

@kevmoo I can see you allowed the build to run.
As expected we can see it failed on the tests that require data from the 2nd repo.

So next step would be to review merge and publish the 2nd PR - #1323 .

Let me know if there's anything else you want me to do in the meantime.

[matanshukry]

@kevmoo fyi - there's no reason to run the builds until the other PR is merged and the versions are updated; it will always fail until then.

[petrnymsa]

Hi @matanshukry @kevmoo Do you plan to continue on this? 🙏 I'd love to have support for enum case insensitivity.

I am open to help with finishing this PR as well.