Update to Elixir 1.20

[jiegillet]

In this PR:

• update to Elixir 1.20
• drop Elixir 1.15 (following https://elixir.hexdocs.pm/compatibility-and-deprecations.html)
• fix type and runtime warnings, tweak or remove some tests

I think this is ready, the test runner CI fails, because it needs this branch.
See my comment below for more thoughts.

[github-actions]

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

[github-actions]

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:

• General steps

  • 🏆 Does this PR need to receive a label with a reputation modifier (x:size/{tiny,small,medium,large,massive})? (A medium reputation amount is awarded by default, see docs)

• Any exercise changed

  • 👤 Does the author of the PR need to be added as an author or contributor in <exercise>/.meta/config.json (see docs)?
  • 🔬 Do the analyzer and the analyzer comments exist for this exercise? Do they need to be changed?
  • 📜 Does the design file (<exercise>/.meta/design.md) need to be updated to document new implementation decisions?

• Practice exercise changed

  • 🌲 Do prerequisites, practices, and difficulty in config.json need to be updated?
  • 🧑‍🏫 Are the changes in accordance with the community-wide problem specifiations?

• Practice exercise tests changed

  • ⚪️ Are all tests except the first one skipped?
  • 📜 Does <exercise>/.meta/tests.toml need updating?

• Concept exercise changed

  • 🌲 Do prerequisites and practices in config.json need to be updated?
  • 📖 Does the concept introduction provide all necessary information to solve this exercise?

• Concept exercise tests changed

  • ⚪️ Are all tests un-skipped?
  • 🔢 Are all tests annotated with @tag task_id?
  • 🐈 Can all tests be understood by reading the test's block only (e.g. no module attributes, no setup functions)?

---

Automated comment created by PR Commenter 🤖.

[jiegillet]

I did this PR in a couple of passes, and I think it's worth breaking down the categories of changes I made.

TLDR: there are classes of warnings that our CI doesn't catch, and we have some tests that awkwardly overlap with the compiler's job.

Failing tests

This one is pretty basic, the compiler changed, so some tests started failing (top-secret).

Examploid type warnings

Also basic, when compiling an exercise (or rather its exemploid solution) the compiler found some type warnings (paint-by-number, intergalactic-transmission).

Runtime warnings

These are things like Enum.slice([1, 2, 3, 4, 5], 3..2//-1) that are not caught at compile time but log a warning at runtime (kindergarten-garden, pascals-triangle, pig-latin, rail-fence-cipher).

These are tricky, because they don't make the tests fail, even when using mix test --warnings-as-errors. I only noticed them from the logs from a test-runner CI failure. They have probably been there for a long time and are not related to the 1.20 update.

I'm not sure what we need to do to catch these errors in the CI, but it would be worth doing, let's open an issue if you agree.

Tests type warnings

These are type warnings in test files (city-office, kitchen-calculator, beer-song, collatz-conjecture, list-ops, two-fer). Some were obvious fixes, some were not, more on that in two paragraphs.

Those are tricky as well because they won't get caught by MIX_ENV=test mix compile --warnings-as-errors even though it really feels like they should. You can catch them with mix test --warnings-as-errors (you can add --exclude test if you don't want to run the tests).
Again, many of these predate 1.20, we just didn't notice them.

These are easy to catch, just add the flag, however I didn't do that because it would also break two other concept exercises bread-and-potions and remote-control-car. Those two explicitly pass in arguments of the wrong type to show that an error will be raised, but now of course the compiler complains about it. In collatz-conjecture and two-fer I felt comfortable removing tests like that because they weren't in the canonical tests, but for concept exercises we need to have a broader conversation about this practice now that Elixir if officially a gradually typed language.

[angelikatyborska]

Thanks for taking care of Elixir 1.20. It was on my radar too, but I'm glad I can just review a PR instead of doing all of this myself 😁 . I'll do the review in chunks, for now I'm leaving my thoughts about "Tests type warnings"

Those are tricky as well because they won't get caught by MIX_ENV=test mix compile --warnings-as-errors even though it really feels like they should.

I think it's because test files are .exs and those never get compiled when running mix compile which would produce artifacts (beam files), but they do get complied when running mix test without producing artifacts.

These are easy to catch, just add the flag,

Yeah, agreed. Let's add it: #1636

it would also break two other concept exercises bread-and-potions and remote-control-car

This is an interesting problem. I looked at those two tests and asked myself: would a typical learner mess up their code in a way that those tests would catch?

For bread-and-options, the answer is not really:

    test "cannot eat a completely new item" do
      assert_raise Protocol.UndefinedError, fn ->
        Edible.eat(%NewItem{}, %Character{})
      end
    end

To make this test fail, I think they would have needed to implement a fallback to any: https://elixir.hexdocs.pm/main/protocols.html#fallback-to-any

For remote-control-car, the answer is definitely yes:

  test "display distance raises error when not given struct" do
    fake_car = %{
      battery_percentage: 100,
      distance_driven_in_meters: 0,
      nickname: "Fake"
    }

    assert_raise(FunctionClauseError, fn ->
      RemoteControlCar.display_distance(fake_car)
    end)
  end

All it takes is not using the struct name in the pattern in the function's head: display_distance(car) instead of display_distance(%RemoteControlCar{}). That "mistake" is already present in the boilerplate code because they're meant to figure out this detail themselves. It's even in the instructions:

Make sure the function only accepts a `RemoteControlCar` struct as the argument.

Would it make sense to move this check from a unit test to the analyzer? Or maybe we should find a mechanism to exclude this specific compilation warning from failing the CI?

I'll address the rest of the PR later.

[angelikatyborska]

All makes sense 💯 I'm lazy and I want to see green CI on the tooling PRs, so I'll let you update all the tooling PRs before reviewing.

[angelikatyborska]

This one doesn't cause compilation warnings? Even though the typespec is pos_integer?

[angelikatyborska]

I'm very happy that 1.20 is finding those bugs 💯