Change return value of ValueExpression.eval(...) to ImmutableList<Value>

[jvdb]

This refactoring changes the return value of ValueExpression.eval(...) from ImmutableList<Optional<Value>> to ImmutableList<Value>. Putting optionals in a list doesn't really make sense, instead, there is now a NOT_A_VALUE constant to express results that have no value, such as the result of a division-by-zero.

In the earliest commits (not visible in the full diff now) the return value was changed to Optional<ImmutableList<Value>> however since the original interface already required a non-null return value, wrapping the new return value into an optional appeared unnecessary. Furthermore, it becomes quite tricky to distinguish between return values such as Optional.empty(), an empty ImmutableList, a list containing a single NOT_A_VALUE, etc.

The current semantics should be that:

1. Null is never returned. This was already the case previously.
2. A valid request with no results (such as a ref("a") when no values with name "a" exist) returns an empty list.
3. Any impossible computation will return a NOT_A_VALUE. In the context of processing lists, only the specific computation that results in NOT_A_VALUE will do so, not the surrounding ones. For example, the result of div([1, 1, 1], [1, 0, 1]) will evaluate to [1, NOT_A_VALUE, 1].
4. A comparison involving NOT_A_VALUE results in false, so eq(con(NOT_A_VALUE), con(NOT_A_VALUE)) evaluates to false. This is in line with behaviour defined by IEEE-754.

Some other points to consider:

1. In situations where a SingleValueExpression is required, sometimes an exception is thrown upon encountering a NOT_A_VALUE, but not always (e.g., CurrentIteration and Expand do it, but Ref doesn't). Let's unify this when we implement Add type mechanism for single value list expressions #70.
2. There are quite a lot of error-prone warnings about using reference equality when checking for NOT_A_VALUE. This is currently by design to simplify the size of the diff this issue generates and will be fixed in Create a clean implementation for NOT_A_VALUE #280.

Finally, I've come across some things to consider and perhaps change:

1. If Sub (the Token) or Tie is invoked with an empty list of ValueExpressions (for the offsets and dataExpressions respectively) that doesn't fail the parse, which makes sense, but it does insert an empty ParseGraph. I'm not sure that's a good idea.
2. CurrentIteration returns an empty list when a nonsensical level is provided such as a negative number. It also does this when the requested level does not exist (e.g., 5 when there are only 3 levels). Perhaps it makes more sense to return NOT_A_VALUE for a negative number (i.e., impossible) and 0 for a too high number (i.e., we are technically in iteration 0 for everything above the current high level)?

If we decide to change behaviour, let's make new issues so they don't make this diff larger. Additionally, I will pick up any issues resulting from the analysis tools on this PR.

This PR resolves #279.

[codecov]

Codecov Report

Merging #281 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##             master   #281   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity     1032   1048   +16     
=======================================
  Files            92     94    +2     
  Lines          1394   1406   +12     
  Branches        146    150    +4     
=======================================
+ Hits           1394   1406   +12

Impacted Files	Coverage Δ	Complexity Δ
...ain/java/io/parsingdata/metal/data/ParseValue.java	100% <ø> (ø)	13 <0> (ø)	⬇️
.../parsingdata/metal/expression/value/FoldRight.java	100% <ø> (ø)	3 <0> (ø)	⬇️
...o/parsingdata/metal/expression/value/FoldLeft.java	100% <ø> (ø)	3 <0> (ø)	⬇️
...main/java/io/parsingdata/metal/data/ParseItem.java	100% <ø> (ø)	6 <0> (ø)	⬇️
...io/parsingdata/metal/expression/value/Reverse.java	100% <ø> (ø)	7 <0> (ø)	⬇️
...ingdata/metal/expression/value/reference/Self.java	100% <100%> (ø)	5 <2> (ø)	⬇️
.../src/main/java/io/parsingdata/metal/Shorthand.java	100% <100%> (ø)	131 <33> (ø)	⬇️
...ingdata/metal/expression/value/arithmetic/Div.java	100% <100%> (ø)	3 <0> (ø)	⬇️
.../io/parsingdata/metal/expression/value/Expand.java	100% <100%> (ø)	17 <7> (+2)	⬆️
...ngdata/metal/expression/value/reference/First.java	100% <100%> (ø)	9 <2> (ø)	⬇️
... and 41 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fcda2b...be55ae4. Read the comment docs.

[jvdb]

Issues identified by tools are resolved. Only remaining one that we can ignore for now is the issues that BetterCodeHub raises, since those issues mostly relate to the introduction of NOT_A_VALUE. The current implementation is just a placeholder for when we continue with #280.

[rdvdijk]

Pretty big PR, but it looks pretty good to me!

[jvdb]

I have checked the BetterCodeHub-results. They seem to indicate some worsening of their metrics but everything still seems quite well within their own quality parameters (still scoring 9/10).