Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elixir 1.16 #415

Merged
merged 6 commits into from
Jan 8, 2024
Merged

Elixir 1.16 #415

merged 6 commits into from
Jan 8, 2024

Conversation

angelikatyborska
Copy link
Member

@angelikatyborska angelikatyborska commented Dec 26, 2023

Part of exercism/elixir#1396

Elixir 1.16 is very unhappy about the wild wild west of we have in test_exercise_analysis "..." do where we put solution under test code. That will not compile anymore 🙈. I think we need to wrap every single case in ~S""".

@jiegillet
Copy link
Contributor

Oh boy.
I guess it really was the wild west in those files, I can't blame Jose for putting an end to the madness...

Wrapping everything in ~S is mostly bad for the formatting and syntax highlighting. Is there a way to make a formatter plugin that would format Elixir code in there? Or maybe in a custom sigil ~EX?

@angelikatyborska
Copy link
Member Author

Hmm, ok, maybe it's not that bad... I identified a few files that prevent compilation, commented them out. Then I found that most of the remaining test failures can be fixed with a small tweak to ElixirAnalyzer.ExerciseTestCase. There's one more test failure remaining, and the commented out files need to be brought back. I think it should be possible to modify the code under test in those files to compile.

I pushed my partially finished work in case you want to take a look.

@angelikatyborska
Copy link
Member Author

One more problem - smoke tests fail because not all warnings from a single module are reported.

@angelikatyborska
Copy link
Member Author

Weirdly enough, this problem doesn't appear when invoking Kernel.ParallelCompiler.compile directly (I even tested in the docker container). It only appears when running the smoke test.

docker run \
    -ti \
    --rm \
    --network none \
    --read-only \
    --mount type=bind,src=$(realpath test_data),dst=/opt/analyzer/test_data \
    --mount type=tmpfs,dst=/tmp \
    --entrypoint /bin/sh \
    elixir-analyzer

And in iex:

iex(1)>  paths = ["/opt/analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex"]
["/opt/analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex"]
iex(2)> Kernel.ParallelCompiler.compile(paths)
    warning: Behaviour.defcallback/1 is deprecated. Use the @callback module attribute instead
    │
  4 │   Behaviour.defcallback(init)
    │             ~
    │
    └─ test_data/lasagna/deprecated_modules/lib/lasagna.ex:4:13: Lasagna (module)

    warning: HashDict.new/0 is deprecated. Use maps and the Map module instead
    │
  7 │     HashDict.new()
    │              ~
    │
    └─ test_data/lasagna/deprecated_modules/lib/lasagna.ex:7:14: Lasagna.expected_minutes_in_oven/0

    warning: HashSet.member?/2 is deprecated. Use the MapSet module instead
    │
 12 │     HashSet.member?(HashSet.new(), :foo)
    │             ~
    │
    └─ test_data/lasagna/deprecated_modules/lib/lasagna.ex:12:13: Lasagna.remaining_minutes_in_oven/1

    warning: HashSet.new/0 is deprecated. Use the MapSet module instead
    │
 12 │     HashSet.member?(HashSet.new(), :foo)
    │                             ~
    │
    └─ test_data/lasagna/deprecated_modules/lib/lasagna.ex:12:29: Lasagna.remaining_minutes_in_oven/1

{:ok, [Lasagna],
 [
   {"/opt/analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
    {7, 14},
    "HashDict.new/0 is deprecated. Use maps and the Map module instead"},
   {"/opt/analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
    {12, 13}, "HashSet.member?/2 is deprecated. Use the MapSet module instead"},
   {"/opt/analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
    {12, 29}, "HashSet.new/0 is deprecated. Use the MapSet module instead"},
   {"/opt/analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
    {4, 13},
    "Behaviour.defcallback/1 is deprecated. Use the @callback module attribute instead"}
 ]}

@angelikatyborska
Copy link
Member Author

Another hint: when I use return_diagnostics, I can see that the warnings that do not appear in smoke tests are all runtime warnings, and the ones that do appear, are compile warnings.

Kernel.ParallelCompiler.compile(code_path, return_diagnostics: true) #=> {:ok, [Lasagna],
 %{
   runtime_warnings: [
     %{
       message: "HashDict.new/0 is deprecated. Use maps and the Map module instead",
       position: {7, 14},
       file: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       stacktrace: [
         {Lasagna, :expected_minutes_in_oven, 0,
          [
            file: ~c"test_data/lasagna/deprecated_modules/lib/lasagna.ex",
            line: 7,
            column: 14
          ]}
       ],
       source: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       span: nil,
       severity: :warning
     },
     %{
       message: "HashSet.member?/2 is deprecated. Use the MapSet module instead",
       position: {12, 13},
       file: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       stacktrace: [
         {Lasagna, :remaining_minutes_in_oven, 1,
          [
            file: ~c"test_data/lasagna/deprecated_modules/lib/lasagna.ex",
            line: 12,
            column: 13
          ]}
       ],
       source: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       span: nil,
       severity: :warning
     },
     %{
       message: "HashSet.new/0 is deprecated. Use the MapSet module instead",
       position: {12, 29},
       file: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       stacktrace: [
         {Lasagna, :remaining_minutes_in_oven, 1,
          [
            file: ~c"test_data/lasagna/deprecated_modules/lib/lasagna.ex",
            line: 12,
            column: 29
          ]}
       ],
       source: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       span: nil,
       severity: :warning
     }
   ],
   compile_warnings: [
     %{
       message: "Behaviour.defcallback/1 is deprecated. Use the @callback module attribute instead",
       position: {4, 13},
       file: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       stacktrace: [
         {Lasagna, :__MODULE__, 0,
          [
            file: "test_data/lasagna/deprecated_modules/lib/lasagna.ex",
            column: 13,
            line: 4
          ]}
       ],
       source: "/Users/angelika/Documents/code/exercism/elixir-analyzer/test_data/lasagna/deprecated_modules/lib/lasagna.ex",
       span: nil,
       severity: :warning
     }
   ]
 }}

@angelikatyborska
Copy link
Member Author

I ran out of ideas for debugging and created an issue in elixir-lang: elixir-lang/elixir#13217

@angelikatyborska
Copy link
Member Author

All green thanks to Jose's help in the issue 🚀

@angelikatyborska angelikatyborska marked this pull request as ready for review December 31, 2023 18:59
@angelikatyborska
Copy link
Member Author

@jiegillet Can you take another look at this PR? If it gets approved, I can merge all the other 1.16 tooling PRs too

Copy link
Contributor

@jiegillet jiegillet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I... am simply amazed.
I bow to your incredible Elixir ability 🙇

Everything was broken, how did you get it all to work without so little change and barely even touch the tests?

Just wow, here is your medal: 🥇

@angelikatyborska
Copy link
Member Author

OMG, I wasn't expecting this. I'm moved! Thank you. My mother will be so proud.

7852553

@angelikatyborska angelikatyborska merged commit 01f7f1b into main Jan 8, 2024
@angelikatyborska angelikatyborska deleted the elixir-1-16 branch January 8, 2024 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants