Consider removing From<IpXXAddr> for IpXXNetwork

[sunshowers]

Hi there, thanks for maintaining this crate!

At our workplace we ran into an issue where we were accidentally converting an Ipv6Addr to an Ipv6Network. The conversion seems a bit risky because an address and a network represent different things.

In contrast, From<Ipv4Network> for IpNetwork and From<Ipv6Network> for IpNetwork are reasonable to have.

Would you consider removing these impls and maybe introducing single constructors to represent the single IP use case? That would have made this bug much easier to spot.

Thanks!

[achanda]

Hi 👋 sounds like you want a conversion from Ipv6Addr to Ipv6Network to error out, but that does not happen due to this impl https://github.com/achanda/ipnetwork/blob/master/src/ipv6.rs#L315 so you want to remove those impls. Is that correct? What would the type signature of the single constructor be?

[sunshowers]

Thanks for the quick response! Yes, I'm suggesting removing those impls. It'll be a breaking change, but I think one that's good from an API misuse perspective.

How about just an Ipv6Network::single(addr: Ipv6Addr) constructor? I'm not saying don't provide this, I'm just saying that it would be nice for that to be more explicit.

[faern]

I agree with this suggestion. A impl From<X> for Y should only exist if what the conversion does is obvious. Here I'd say it's not so obvious that IP -> IPnet sets the prefix to the maximum and the result represents a single IP.

A dedicated constructor is more explicit and harder to accidentally misuse.

In contrast, From<Ipv4Network> for IpNetwork and From<Ipv6Network> for IpNetwork are reasonable to have.

I don't agree. They are vague in the same sense IMHO. Converting a network into a single IP can probably be misunderstood what it does. And there is an ip() method already that is more clear.