feat(validation): Add further checks for enum validation

[volsa]

Previously the enum validation checked whether the datatypes of the left- and right-hand side are equal and if not returned an error. This version adds some further checks to see if an assignment would yield a value that satisfies all the enum's variant values. For example for TYPE Color : (red, green, blue := 5); END_TYPE the requirement would be that the right-hand side of an assignment evaluates to a literal integer of 0, 1 or 5. With that in mind, we can now give more precise error/warning messages.

See this test and snapshot for some further context.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 91.89189% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 95.74%. Comparing base (f58a57f) to head (8b79c14).

Files	Patch %	Lines
compiler/plc_ast/src/ast.rs	38.46%	8 Missing :warning:
src/index.rs	95.65%	1 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1064 +/- ## ========================================== - Coverage 95.75% 95.74% -0.02% ========================================== Files 147 147 Lines 43119 43191 +72 ========================================== + Hits 41290 41353 +63 - Misses 1829 1838 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[volsa]

Nice work! Just wondering if we should also print the enum variant names along with their integer values in the error messages, e.g. something along the lines of [red(1), blue(2), green(3)]

Generally speaking, big fan of this but I'm concerned the error message will be bloated with enums that have many variants, long names or worse both of them - but I'm open for it

[ghaith]

Nice work! Just wondering if we should also print the enum variant names along with their integer values in the error messages, e.g. something along the lines of [red(1), blue(2), green(3)]

Generally speaking, big fan of this but I'm concerned the error message will be bloated with enums that have many variants, long names or worse both of them - but I'm open for it

I'm also open for that. I know it can be bloated but we also warn the user that literals are not recommended, yet we are proposing literals as the solution to this issue.

[volsa]

Nice work! Just wondering if we should also print the enum variant names along with their integer values in the error messages, e.g. something along the lines of [red(1), blue(2), green(3)]

Generally speaking, big fan of this but I'm concerned the error message will be bloated with enums that have many variants, long names or worse both of them - but I'm open for it

I'm also open for that. I know it can be bloated but we also warn the user that literals are not recommended, yet we are proposing literals as the solution to this issue.

Good point, I went ahead and implemented @mhasel suggestion. To avoid bloated messages I'm truncating the amount of enum variants displayed if an enum has more than 3 of them as can be seen here. Not entirely sure if truncating is a good idea but if we feel like we should display all variants I'm open to changing that behaviour again. My thinking was at that point the user knows the internal enum integer value is not in bound and doesn't need to be bothered with a full list of variants.

[mhasel]

Nice work! Just wondering if we should also print the enum variant names along with their integer values in the error messages, e.g. something along the lines of [red(1), blue(2), green(3)]

Generally speaking, big fan of this but I'm concerned the error message will be bloated with enums that have many variants, long names or worse both of them - but I'm open for it

I'm also open for that. I know it can be bloated but we also warn the user that literals are not recommended, yet we are proposing literals as the solution to this issue.

Good point, I went ahead and implemented @mhasel suggestion. To avoid bloated messages I'm truncating the amount of enum variants displayed if an enum has more than 3 of them as can be seen here. Not entirely sure if truncating is a good idea but if we feel like we should display all variants I'm open to changing that behaviour again. My thinking was at that point the user knows the internal enum integer value is not in bound and doesn't need to be bothered with a full list of variants.

Very nice. But I have yet another nit-pick:

7 │                 y : (metallic := 1, matte := 2, neon := 3) := red; // error
  │                                                               ^^^ Value 0 is not bound in [metallic(1), matte(2), neon(3)]

Can we extend the change to also display names to also display Value red(0) is not bound in... in the error message?

[volsa]

Can we extend the change to also display names to also display Value red(0) is not bound in... in the error message?

Sounds good, I'll probably work on this after @ghaith merged his changes but as a reminder to myself this can also be improved

 18 │                 localState := 0;
-   │                               ^ Consider using enums rather than literal integers
+   │                               ^ Consider using enums rather than literal integers, i.e. `localState := State.Idle`

and

 28 │                 color := State.Idle;    // State.Idle == 0 == Color.Red
-   │                          ^^^^^^^^^^ Consider using enums with the same kind
+   │                          ^^^^^^^^^^ Consider using enums of the same kind, i.e. `color := Color.Red`

That is display these if the derived rhs-value is in bound with the lhs

[volsa]

@ghaith @mhasel This PR is ready, it'd be cool if you guys could find some time to review this. Just as a heads-up, the previous discussions are for the most part outdated, the validation message have been slightly modified to be more minimal but this snapshot gives a good overview of the PR as a whole.