Adopt naming conventions that don't repeat names

[thomaseizinger]

At the moment, a lot of names within modules and crates of rust-libp2p repeat the name of the module they are contained in. For example, the libp2p-noise crate exports the type NoiseConfig. From the perspective of the libp2p meta crate, this types is used as:

use libp2p::noise::NoiseConfig;

However, to use this type, one also needs to depend on libp2p::noise::Keypair and libp2p::noise::X25519Spec. Keypair and X25519Spec are too ambiguous to fully import which is why I tend to import the entire libp2p::noise module:

use libp2p::noise;

Now I have to make an annoying trade-off:

1. Do I choose to fully import NoiseConfig because - by itself - its name is unambiguous and doesn't clash with any other symbols in scope but introduce an additional line in the use statements of my file.
2. Have only the use libp2p::noise; import but repeat the word noise upon usage: noise::NoiseConfig. An example of this is here:

   rust-libp2p/examples/chat-tokio.rs

   Lines 68 to 79 in 841b09a

   	// Create a keypair for authenticated encryption of the transport.
   	let noise_keys = noise::Keypair::<noise::X25519Spec>::new()
   	.into_authentic(&id_keys)
   	.expect("Signing libp2p-noise static DH keypair failed.");
   	// Create a tokio-based TCP transport use noise for authenticated
   	// encryption and Mplex for multiplexing of substreams on a TCP stream.
   	let transport = TokioTcpConfig::new().nodelay(true)
   	.upgrade(upgrade::Version::V1)
   	.authenticate(noise::NoiseConfig::xx(noise_keys).into_authenticated())
   	.multiplex(mplex::MplexConfig::new())
   	.boxed();

There are many instances of this over the entire codebase (non-exhaustive):

• libp2p::floodsub::FloodsubConfig
• libp2p::mdns::MdnsEvent
• libp2p::request_response::{RequestResponseCodec, RequestResponseMessage}

My proposal would be to aim for the following:

1. Assume that people use everything through the libp2p crate, i.e. import names should work well if one can assume that people use libp2p::<protocol>. Having things be consistent when depending on sub-crates explicitly is of secondary nature.
2. Importing a dedicated libp2p::<protocol> module should give you types that are short, yet don't clash. In particular, I think most types within a specific protocol do not need to repeat, which module they are contained in: libp2p::request_response::RequestResponseCodec should be libp2p::request_response::Codec, etc.

The benefit of such a structure is that it keeps the amount of imports in a file short. It is common practice to have imports at the top of a file in Rust but it is actually annoying because it is far the least interesting part of code I want to look at. I tend to sort things in files from highest to lowest importance, which makes it easier to find things.

Additionally, within contexts where no ambiguous names exist, like within the libp2p-request-response crate, one can easily fully-import types like Message or Codec, reducing the noise-level of the code.

For reference, there is actually a clippy-lint for detecting such names: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions

[mxinden]

I am in favor of this. I followed the same conventions in #2059 and #2076.

For reference, there is actually a clippy-lint for detecting such names: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions

I am in favor of enabling this in case we do decide to adopt this convention across the whole repository.

[thomaseizinger]

In my experience, the clippy lint is quite excessive and may lead false positives up to the point where you take hoops to satisfy the lint whilst not making things actually more readable. Nevertheless it can be a useful tool in detecting instances of this. I'll play around and see what the results are.

In regards to adopting these new names, would you want to do the rename in one release or shall we include re-exports that are marked as #[deprecated] to allow for a smoother migration? Even if it is just renames, having to adjust a lot of imports and/or code could make a the upgrade to such version quite painful.

[mxinden]

In regards to adopting these new names, would you want to do the rename in one release or shall we include re-exports that are marked as #[deprecated] to allow for a smoother migration? Even if it is just renames, having to adjust a lot of imports and/or code could make a the upgrade to such version quite painful.

I am fine either way.

[thomaseizinger]

Sweet, I'll start with renaming some things then!

[thomaseizinger]

@mxinden what do you think of the following changes in libp2p-swarm:

• swarm::SwarmEvent -> swarm::Event
• swarm::SwarmBuilder -> swarm::Builder
• protocols_handler::ProtocolsHandlerEvent -> protocols_handler::Event (or Action to align with NetworkBehaviourAction?)

A potentially more controversial proposal:

• swarm::behaviour::NetworkBehaviour -> swarm::Behaviour
• swarm::behaviour::NetworkBehaviourEventProcess -> swarm::behaviour::EventProcess
• swarm::behaviour::NetworkBehaviourAction -> swarm::behaviour::Action

[mxinden]

swarm::SwarmEvent -> swarm::Event

👍 that would be in line with #2217.

swarm::SwarmBuilder -> swarm::Builder

I am fine with that.

protocols_handler::ProtocolsHandlerEvent -> protocols_handler::Event (or Action to align with NetworkBehaviourAction?)

Making ProtocolsHandlerEvent and NetworkBehaviourAction consistent in whatever way sounds good to me.

A potentially more controversial proposal:

* `swarm::behaviour::NetworkBehaviour` -> `swarm::Behaviour`

* `swarm::behaviour::NetworkBehaviourEventProcess` -> `swarm::behaviour::EventProcess`

* `swarm::behaviour::NetworkBehaviourAction` -> `swarm::behaviour::Action`

I would as well be fine with these renames.

Before we tackle any of the above, I would suggest doing so right after a release, tackling these and #2217 all at once, doing a single follow-up release with the renames only. What do you think? (Not a strong opinion.)

[thomaseizinger]

Before we tackle any of the above, I would suggest doing so right after a release, tackling these and #2217 all at once, doing a single follow-up release with the renames only. What do you think? (Not a strong opinion.)

The renames are rather tedious to do actually so I think it is a more long-running effort. I am also not sure whether it is nicer if there a big rename release or whether subsequent releases step-by-step rename certain protocols.

I am leaning towards the latter one just so we don't have any pressure in getting them out.

[thomaseizinger]

I've been playing around with using deprecated on renamed imports but haven't gotten very far unfortunately. It is not working as desired :(
Desired being: A deprecation warning when accessing a renamed symbol through its old name.