josefs / Gradualizer

A Gradual type system for Erlang
MIT License
613 stars 35 forks source link

Expand erlang.++ function specs #485

Closed luk-pau-es closed 2 years ago

luk-pau-es commented 2 years ago

Expand function specs for erlangs ++ operator.

erszcz commented 2 years ago

@luk-pau-es awesome! Could you rebase this on top of https://github.com/erszcz/Gradualizer/tree/split-list-concat-op-tests and add a test to test/should_pass/list_concat_op_pass.erl which exercises the new spec? I.e. a test which fails with the current spec, but works with the new one.

erszcz commented 2 years ago

@zuiderkwast

Wow! Lots of known problems fixed. This looks awesome.

It looks impressive, but it's actually code already present in master. Most of the tests in test/should_pass/list_concat_op_pass.erl already passed, but were previously under known_problems. Here we split the ones passing from the ones that actually are known_problems.

@luk-pau-es We have to add a spec to generate_list() - otherwise it doesn't exercise the new spec clauses. You can verify that by running the new test, but keeping the old spec - the test will not fail. A spec to add could be:

diff --git a/test/should_pass/list_concat_op_pass.erl b/test/should_pass/list_concat_op_pass.erl
index 8af53a9..8b8c3db 100644
--- a/test/should_pass/list_concat_op_pass.erl
+++ b/test/should_pass/list_concat_op_pass.erl
@@ -31,4 +31,5 @@ nonempty_concat_fun_nonempty_gives_proper() ->
 nonempty1_concat_fun_nonempty2_gives_proper() ->
     erlang:'++'(generate_list(), generate_list()).

+-spec generate_list() -> [integer()].
 generate_list() -> lists:seq(1, 2).
zuiderkwast commented 2 years ago

Most of the tests in test/should_pass/list_concat_op_pass.erl already passed, but were previously under known_problems.

Oh, that's a problem in our test framework in that case. I thought a passing test in known_problems/should_pass would fail the test run. That's what I'd expect anyway.

erszcz commented 2 years ago

@zuiderkwast It seems that if any test in a known_problems/should_pass file fails, the entire file is treated as a known problem, so some passing tests might hide in such a file. If no test fails in a known_problems/should_pass file, then it's reported as solved.