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

Possible redundant case caught by Elixir 1.18.0 #29

Open
hchienjo opened this issue Jan 1, 2025 · 1 comment
Open

Possible redundant case caught by Elixir 1.18.0 #29

hchienjo opened this issue Jan 1, 2025 · 1 comment

Comments

@hchienjo
Copy link

hchienjo commented Jan 1, 2025

When running the tests/compiling with Elixir 1.18.0, I get the following warnings:

Compiling 1 file (.ex)
    warning: this clause in cond will always match:

        address_netmask(inet.address)

    since it has type:

        integer()

    where "inet" was given the type:

        # type: dynamic(%Postgrex.INET{})
        # from: lib/ecto_network/inet.ex:55:29
        %Postgrex.INET{} = inet

    typing violation found at:
    │
 59 │         address_netmask(inet.address) -> %{inet | netmask: address_netmask(inet.address)}
    │                                       ~
    │
    └─ lib/ecto_network/inet.ex:59:39: EctoNetwork.INET.load/1

With the following patch, the warning goes away and all the tests pass:

diff --git a/lib/ecto_network/inet.ex b/lib/ecto_network/inet.ex
index 5f87e2c..ce52a52 100644
--- a/lib/ecto_network/inet.ex
+++ b/lib/ecto_network/inet.ex
@@ -57,7 +57,6 @@ defmodule EctoNetwork.INET do
       cond do
         address_netmask(inet.address, inet.netmask) -> inet
         address_netmask(inet.address) -> %{inet | netmask: address_netmask(inet.address)}
-        true -> %{inet | netmask: nil}
       end

     {:ok, inet}

I am not 100% sure if the tests cover all IP address cases so:

  1. With this patch will the code still behave correctly?
  2. Should I go ahead and submit a PR?
@adam12
Copy link
Owner

adam12 commented Jan 2, 2025

Thanks for the report. Let me look and will advise.

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

No branches or pull requests

2 participants