Normalize annotated types

[NelsonVides]

ann_type was not normalised correctly.

Solves #224, and perhaps some more stuff.

[zuiderkwast]

Good finding! I have a suggestion for simplification. Not sure it works (but I assume it does).

[zuiderkwast]

Why not simply this:

normalize({ann_type, _Ann, Type}, TEnv) ->
    normalize(Type, TEnv);

[NelsonVides]

It doesn't work. The absform for an annotated type is {ann_type, _, List}, where that third parameter is a list of things. And then normalize does nothing on a list, it is expecting a tagged tuple, for all other possibilities (like the list), it passes them to the do-nothing clause at the bottom.

And parsing this list is precisely what flatten_type does, so no need to reimplement it 🤷‍♂

[zuiderkwast]

OK, I had a closer look now. The list is a fixed list of arguments, which is typical for these AST nodes. For ann_type it's always 2 arguments, namely the variable name and the type. E.g. the A :: atom() (where 55 is a line number) is represented as:

{ann_type,55,[{var,55,'A'},{type,55,atom,[]}]}

Using flatten_list and then taking the union of the returned types actually give us the type A | atom() instead of the expected atom(). [Edit] Sorry, I missed the merge_union_types.

My revised suggestion then:

normalize({ann_type, _Ann, [_Var, Type]}, TEnv) ->
    normalize(Type, TEnv);

And I suggest adding a line for this in the existing test case for the normalize function. [Edit] Not really needed since there are other tests covering this.

[NelsonVides]

@zuiderkwast hey there, I'm doing as you suggested, and I haven't seen any infinite loop as described in #178, but on the other hand I'm having a hard time to follow all the discussion about annotated types like in #180 as well.
What are your thoughts about this?

[zuiderkwast]

I think it's the right thing to remove these in normalize, but see also #178.

If we instead want to treat these as type variables with a subtype constraint, we should instead replace A :: t() with just A and save a constraint. That could be done already when processing a spec.

But that is not very likely, given that we actually replace the type variables defined using when A :: t() by substituting the type t() for the occurrences of A in a spec. This was implemented in #93. There was a long conversation about that, before it was accepted.

[codecov-io]

Codecov Report

Merging #228 into master will increase coverage by 1.28%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #228      +/-   ##
=========================================
+ Coverage   83.82%   85.1%   +1.28%     
=========================================
  Files          14      16       +2     
  Lines        2380    2498     +118     
=========================================
+ Hits         1995    2126     +131     
+ Misses        385     372      -13

Impacted Files	Coverage Δ
src/typechecker.erl	87.71% <100%> (+0.67%)	⬆️
src/gradualizer.erl	89.06% <0%> (-0.77%)	⬇️
src/gradualizer_highlight.erl	82.43% <0%> (ø)
src/gradualizer_fmt.erl	72.35% <0%> (ø)
src/gradualizer_db.erl	71.62% <0%> (+0.46%)	⬆️
src/gradualizer_cli.erl	89.56% <0%> (+37.08%)	⬆️

---

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 32cbced...bc5c688. Read the comment docs.

[zuiderkwast]

This is good to me. I don't want to merge it however, as long as @josefs opposes it.