# `_procedure_name_(arguments)` naming convention for procedures

As we know, functions return values while procedures don't. That's basically the difference.

I've renamed procedures in my crates as title suggests and it seems there are some improvements in readability.

Below are some code chunks:

fn main() {
    let terminate = Arc::new(AtomicBool::default());

    _register_signal_handling_(&terminate);

    let conn = swayipc::Connection::new()
        .expect("Cannot connect to Sway!");

    let mut libinput = Libinput::new_from_path(Interface);

    let args = get_arguments();

    let pointer_device = libinput
        .path_add_device(&args.device_path)
        .expect("Cannot open pointer device");

    handle_events(conn, libinput, terminate, pointer_device, args)
        .unwrap()
}
#[tokio::main(worker_threads=2)]
async fn main() {

    _init_logger_();

    let env_ctx = context::gather_env::enter();

    _warn_if_incompatible_options_(&env_ctx.opt);

    validate_files(env_ctx).await;
}
async fn connect_neovim(cli_ctx: context::UsageContext) {
    log::info!(target: "context", "{cli_ctx:#?}");

    _init_panic_hook_();

    let mut nvim_conn = connection::open(
        &cli_ctx.tmp_dir,
        &cli_ctx.page_id,
        &cli_ctx.opt.address,
        &cli_ctx.opt.config,
        &cli_ctx.opt.config,
        cli_ctx.print_protection
    ).await;

    let mut nvim_ctx = context::connect_neovim::enter(cli_ctx);
    if nvim_conn.nvim_proc.is_some() {
        nvim_ctx
            .child_neovim_process_has_been_spawned()
    }

    manage_page_state(&mut nvim_conn, nvim_ctx).await
}

In all chunks last function calls in main function are technically procedures, but since they're continuation of main function I've decided to keep their names as is. We can think about it as about "color", so there's primary regular color and secondary with underscores to mark side-effects.

So, for me it seems in that way it's easier to concentrate on how functions are composed.


What community thinks about that? Can we turn it into convention, pattern or a good practice? Perhaps it would be possible to detect such side-effect procedures and rename them accordingly on a next edition change? Or should I just stop inventing and must rename functions as they were originally?

Note that prefixing a function name with _ will prevent the dead code lint from triggering on it.

10 Likes

Rather than returning values or not a more common distinction is whether a function has side effects or not. An example of a side-effecting but value returning function is Vec::pop, sometimes you may call it for the side-effect without wanting to use the result.

Non-side-effecting functions can be marked with #[must_use] to denote that there’s no point in calling it without doing something with the result, then you’ll get a warning for not using it.

4 Likes

That's a good argument against turning this into a common convention.

Perhaps a common prefix like do_ would be a better solution.

This is about functions, not methods.

Methods are just functions with a first "self" argument that gives an additional special call syntax, you can invoke them just like functions.

let v = vec![1, 2];
Vec::pop(&mut v);

My point didn't really have anything to do with Vec::pop though. Another example of a side-effecting and value-returning free-function is std::fs::copy (slightly complicated because it returns two layers of values, one of which is #[must_use] itself, but the inner u64 result is not #[must_use] because of the side-effect, if it was a pure function then it should also be marked).

Rather than a naming convention, how about reaching this goal by implementing an optional Clippy restriction lint for any occurrences of discarding a return value? That is, treat everything except () as if it was #[must_use], so that 'procedures' are recognizable by being the ones that are allowed to appear in statement-expression position, and functions always appear with an = or something.

4 Likes

That lint already exists in rustc:

unused-results  allow    unused result of an expression in a statement
2 Likes

I'm thinking about creating a module with the same name as main function which contains procedures that main function uses, for example:

fn main() {
    let terminate = Arc::new(AtomicBool::default());

    main::register_signal_handling(&terminate);

    let conn = swayipc::Connection::new()
        .expect("Cannot connect to Sway!");

    let mut libinput = Libinput::new_from_path(Interface);

    let args = get_arguments();

    let pointer_device = libinput
        .path_add_device(&args.device_path)
        .expect("Cannot open pointer device");

    handle_events(conn, libinput, terminate, pointer_device, args)
        .unwrap()
}

mod main {
    pub fn register_signal_handling(terminate: &Arc<AtomicBool>) {
        for sig in signal_hook::consts::TERM_SIGNALS {

            let sig_handler = Arc::clone(terminate);
            signal_hook::flag::register(*sig, sig_handler)
                .expect("Cannot register signal handler");
        }
    }
}

Perhaps I like it even more than _procedure_name_(arguments) convention, however there's a problem: this module requires something like use std::sync::Arc; or use super::Arc; or even use super::*; at the top which is a distraction and it's really unclear which variant is better.

What if modules that have function with the same name in the same scope would imply use super::*;?

Also with use super::*; I'm getting can't leak private type error when trying to hide another function under such module. It returns private struct in parent module, so that struct should be made pub in order to make code compile, which then will make it public to the rest of code.

I think there's an opportunity to remove that error unless the module is pub.

And perhaps to include a warning when such module is used in a function with distinct name.

Rather than pub (which implies pub-to-world), you can use pub(super) to say the item is only public to the parent module. This will allow you to use non-pub types from the parent module without the priv-in-pub error.

Note that a pub item in a non-pub module can still be exported if the item is pub used. This is done a lot in practice to allow splitting large modules into multiple files without exposing the implementation module hierarchy to the crate's consumer.

It does seem reasonable to disable or at least weaken the priv-in-pub lint for binary crates, though. However, I do think it should still warn (and suggest restricting the pub item's visibility to the less visible item's) as it's still a likely error, and using the pub(in path) syntax is clearer on intent.

But rather than a mod alongside the fn with the same name, it's more common to define the helpers inside the fn scope instead,

e.g.
fn main() {
    fn register_signal_handling(terminate: &Arc<AtomicBool>) {
        for sig in signal_hook::consts::TERM_SIGNALS {

            let sig_handler = Arc::clone(terminate);
            signal_hook::flag::register(*sig, sig_handler)
                .expect("Cannot register signal handler");
        }
    }

    let terminate = Arc::new(AtomicBool::default());

    register_signal_handling(&terminate);

    let conn = swayipc::Connection::new()
        .expect("Cannot connect to Sway!");

    let mut libinput = Libinput::new_from_path(Interface);

    let args = get_arguments();

    let pointer_device = libinput
        .path_add_device(&args.device_path)
        .expect("Cannot open pointer device");

    handle_events(conn, libinput, terminate, pointer_device, args)
        .unwrap()
}

(This only works for top-level items, not methods. You could write and implement an extension trait, but that level of nesting and rigamarole just in order to make a helper more private isn't really worth it. If you're willing to give up method syntax, you can make it a free function, or if you're insistent, put the entire impl with helpers into a mod to get a privacy edge. IMHO: going to such ends is worth it only if it's to improve unsafe isolation, e.g. polyfilling unsafe field access.)


This doesn't need a use super::*, as the fn inherits the parent module's namespace implicitly. This is a somewhat common way to implement manual polymorphism, delegating to a less-generic version of the function. However, use super::*; is a common idiom for inline modules (most often seen in a #[cfg(test)] mod), so it's not that big of a deal imho.

If I understand you correctly this requires moving the struct into child module which is an additional churn. In addition use super::* is either required in child module, which is even more churn.

Alternatively I can use pub(in child_module) struct .. as you've also suggested. But still use super::* is required in child module, so there's still two places to modify which isn't very ergonomic.

Problem with that is that procedures are hard to discern from functions. Here register_signal_handling(&terminate); looks like any other function, so I want it to be main::register_signal_handling(&terminate); instead.

That's the main motivation behind this proposal.

BTW, I like how code is organized in outline window. It's the same structure as with Rust modules in filesystem!

I'm ready to write RFC for this.