Refactor operator patterns

[gomoripeti]

• use erl_eval:partial_eval/1 exactly what the compiler also uses
• remove duplicated code of handling integer/float pattern within op patterns

just a bit of ad hoc refactoring

[codecov-io]

Codecov Report

Merging #174 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
- Coverage      84%   83.95%   -0.05%     
==========================================
  Files          12       12              
  Lines        2169     2163       -6     
==========================================
- Hits         1822     1816       -6     
  Misses        347      347

Impacted Files	Coverage Δ
src/typechecker.erl	84.92% <100%> (-0.06%)	⬇️

---

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 afc1b1a...e4bc114. Read the comment docs.

[zuiderkwast]

Looks good to me! Just one minor comment.

[zuiderkwast]

Should not happen for what reason? I guess only hand-coded ASTs can include this. Perhaps we don't need this clause but instead just Literal when Literal =/= Pat in the other clause? Then Pat gives a case_clause error and code coverage can increase. (I assume this is an uncovered clause.)

[gomoripeti]

I was thinking about exactly the same when I saw codecov complaining. However at the undefined-records (somewhere in this thread #147 (comment)) we agreed that we assume the least from the AST and (although proactively don't look for such errors) when we hit it, we show a meaningful message.

So I will just add a little test to cover this case similar to this https://github.com/josefs/Gradualizer/blob/master/test/typechecker_tests.erl#L546

[zuiderkwast]

Sure, if you add a test for this, at least it's not completely dead code. I do think there's a difference to undefined records though.

For the undefined records, it's still valid syntax, so it's still a realistic scenario to hit when we're type checking source files just using the parser, which is something we do support. An AST that the parser cannot produce is not the same; it's not realistic IMO.

We were talking about calling the compiler for the validation step when checking source files, which would catch undefined records, but we didn't implement it yet, did we? If we do, I think we don't need the code for handling undefined records either.

[gomoripeti]

Actually the parser successfully parses this illegal patter (see the added testcase) only the linter catches it.
If gradualizer will ever support running as a parse_transform (to plug it straight into the compilation pipeline) that is the only scenario when we might receive such unchecked AST, and I think in that case we cannot call out to the compiler/linter. This use case (which we might decide to never implement for some reason or the other) stopped me to implement the erl_lint check for source code, and convinced me that we need to handle every case the parser can possibly spit out.