peek-travel / cocktail

Elixir date recurrence library based on iCalendar events
https://hexdocs.pm/cocktail
MIT License
222 stars 30 forks source link

Fix compilation error with Elixir 1.14 #268

Closed maltoe closed 1 year ago

maltoe commented 2 years ago

Since Elixir 1.14, the following results in a compilation error

defmodule Cocktail.Schedule do
  ...
  defimpl Inspect, for: __MODULE__ do
    ...
  end
end

Error looks like this:

==> cocktail
Compiling 21 files (.ex)

== Compilation error in file lib/cocktail/schedule.ex ==
** (CompileError) lib/cocktail/schedule.ex:251: cannot define module
Inspect.Kernel because it is currently being defined in
lib/cocktail/rule.ex:31
    lib/cocktail/schedule.ex:251: (module)

I checked the docs for a few Elixir versions back (at least 1.10), and they always said

When implementing a protocol for a struct, the :for option can be omitted if the defimpl call is inside the module that defines the struct

This patch removes the unnecessary for: __MODULE__ option to fix the compilation error.


Just FYI: It looks to me like a bug in Elixir 1.14, but I failed to reproduce it outside cocktail. It's not just the for: __MODULE__ by itself, at least. Unfortunately I don't really have the time to dig deeper right now. So if you could apply this patch, I'd be very grateful :)

codecov[bot] commented 2 years ago

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

Coverage data is based on head (8c11308) compared to base (3dd3b6e). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #268 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 21 21 Lines 437 437 ========================================= Hits 437 437 ``` | [Impacted Files](https://codecov.io/gh/peek-travel/cocktail/pull/268?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=peek-travel) | Coverage Δ | | |---|---|---| | [lib/cocktail/rule.ex](https://codecov.io/gh/peek-travel/cocktail/pull/268/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=peek-travel#diff-bGliL2NvY2t0YWlsL3J1bGUuZXg=) | `100.00% <ø> (ø)` | | | [lib/cocktail/schedule.ex](https://codecov.io/gh/peek-travel/cocktail/pull/268/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=peek-travel#diff-bGliL2NvY2t0YWlsL3NjaGVkdWxlLmV4) | `100.00% <ø> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=peek-travel). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=peek-travel)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

vanvoljg commented 1 year ago

This was short-lived issue that existed only in Elixir 1.14.0-1.14.1.

maltoe commented 1 year ago

@vanvoljg Not that it matters much, but you could have just merged the PR anyway, as it fixes the issue even for Elixir 1.14.0 and 1.14.1 :rocket: :grimacing: