massa icon indicating copy to clipboard operation
massa copied to clipboard

Self document style guide RE: format agruments

Open Ben-PH opened this issue 2 years ago • 1 comments

Is your feature request related to a problem? Please describe.

When looking to contribute, I decided to run a cargo clippy -- -W clippy::pedantic, and see what cropped up. A long list on warnings following this pattern was emitted:

warning: variables can be used directly in the `format!` string
   --> massa-node/src/main.rs:236:25
    |
236 |             Err(err) => panic!("critical error detected in the bootstrap process: {}", err)
    |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
note: the lint level is defined here
   --> massa-node/src/main.rs:4:9
    |
4   | #![warn(clippy::uninlined_format_args)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: change this to
    |
236 -             Err(err) => panic!("critical error detected in the bootstrap process: {}", err)
236 +             Err(err) => panic!("critical error detected in the bootstrap process: {err}")
    |

I'm unsure whether or not guidelines specify to inline or not (this lint is quite a subjective one).

Describe the solution you'd like

Either follow the format-args, or add an #![allow(clippy::uninlined-format-args)]

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context

I'll look for style-guide documentation and/or wait for feedback from core team, and then I'll implement the changes needed.

For input, I prefer uninlined, as it keeps consistency with formats that use a struct field:

// doesn't work...
format!("foos bar field is: {foo.bar}");
// ...so this "shouldn't" work (imho)
format("bar variable is: {bar})";

ll go ahead and make a PR with the allow solution if I get any signals there's no preference for the inlined version

Ben-PH avatar Dec 29 '22 10:12 Ben-PH

We are not all agree on this lint so we disabled it for now. As there is no way to have a configuration file saying that you disable a lint for the project we are running clippy with this command in the CI : cargo clippy -- -A clippy::uninlined-format-args

AurelienFT avatar Dec 29 '22 10:12 AurelienFT