From 45c1324cb37d7a823e83acabd159696f72a1359d Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 29 Sep 2022 19:01:03 +0400 Subject: [PATCH] Improve readability of `CODE_STYLE.md` Former-commit-id: 36f0226751b254e0c70f4f2c8153f09bc254e97f --- CODE_STYLE.md | 279 +++++++++++++++++++++++++++++--------------------- 1 file changed, 163 insertions(+), 116 deletions(-) diff --git a/CODE_STYLE.md b/CODE_STYLE.md index 503844c2..eb0eced3 100644 --- a/CODE_STYLE.md +++ b/CODE_STYLE.md @@ -1,100 +1,101 @@ # Code style -This is a description of a coding style that every contributor must follow. Please, read the whole document before you start pushing code. + +This is a description of a coding style that every contributor must follow. +Please, read the whole document before you start pushing code. ## Generics -Generics are always written with `where`. -Bad: +All trait bounds should be written in `where` ```rust - pub fn new, - T: Into, - P: Into, - E: Into> +// GOOD +pub fn new(user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self +where + N: Into, + T: Into, + P: Into, + E: Into, +{ ... } + +// BAD +pub fn new, + T: Into, + P: Into, + E: Into> (user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self { ... } ``` - -Good: - ```rust - pub fn new(user_id: i32, name: N, title: T, png_sticker: P, emojis: E) -> Self - where - N: Into, - T: Into, - P: Into, - E: Into { ... } +// GOOD +impl Trait for Wrap +where + T: Trait +{ ... } + +// BAD +impl Trait for Wrap { ... } ``` -## Comments - 1. Comments must describe what your code does and mustn't describe how your code does it and bla-bla-bla. Be sure that your comments follow the grammar, including punctuation, the first capital letter and so on. +**Rationale:** `where` clauses are easier to read when there are a lot of bounds, uniformity. -Bad: +## Documentation comments -```rust -/// this function make request to telegram -pub fn make_request(url: &str) -> String { ... } -``` +1. Documentation must describe what your code does and mustn't describe how your code does it and bla-bla-bla. +2. Be sure that your comments follow the grammar, including punctuation, the first capital letter and so on. + ```rust + // GOOD + /// This function makes a request to Telegram. + pub fn make_request(url: &str) -> String { ... } + + // BAD + /// this function make request to telegram + pub fn make_request(url: &str) -> String { ... } + ``` +3. Do not use ending punctuation in short list items (usually containing just one phrase or sentence). + ```md + + - Handle different kinds of Update + - Pass dependencies to handlers + - Disable a default Ctrl-C handling -Good: + + - Handle different kinds of Update. + - Pass dependencies to handlers. + - Disable a default Ctrl-C handling. -```rust -/// This function makes a request to Telegram. -pub fn make_request(url: &str) -> String { ... } -``` + + - Handle different kinds of Update; + - Pass dependencies to handlers; + - Disable a default Ctrl-C handling; + ``` +3. Link resources in your comments when possible, for example: + ```rust + /// Download a file from Telegram. + /// + /// `path` can be obtained from the [`Bot::get_file`]. + /// + /// To download into [`AsyncWrite`] (e.g. [`tokio::fs::File`]), see + /// [`Bot::download_file`]. + /// + /// [`Bot::get_file`]: crate::bot::Bot::get_file + /// [`AsyncWrite`]: tokio::io::AsyncWrite + /// [`tokio::fs::File`]: tokio::fs::File + /// [`Bot::download_file`]: crate::Bot::download_file + ``` - 2. Do not use ending punctuation in short list items (usually containing just one phrase or sentence). Bad: +## Use `Self` where possible -```md - - Handle different kinds of Update. - - Pass dependencies to handlers. - - Disable a default Ctrl-C handling. -``` - -Bad: - -```md - - Handle different kinds of Update; - - Pass dependencies to handlers; - - Disable a default Ctrl-C handling. -``` - -Good: - -```md - - Handle different kinds of Update - - Pass dependencies to handlers - - Disable a default Ctrl-C handling -``` - - 3. Link resources in your comments when possible: - -```rust -/// Download a file from Telegram. -/// -/// `path` can be obtained from the [`Bot::get_file`]. -/// -/// To download into [`AsyncWrite`] (e.g. [`tokio::fs::File`]), see -/// [`Bot::download_file`]. -/// -/// [`Bot::get_file`]: crate::bot::Bot::get_file -/// [`AsyncWrite`]: tokio::io::AsyncWrite -/// [`tokio::fs::File`]: tokio::fs::File -/// [`Bot::download_file`]: crate::Bot::download_file -#[cfg(feature = "unstable-stream")] -pub async fn download_file_stream( - &self, - path: &str, -) -> Result>, reqwest::Error> -{ - download_file_stream(&self.client, &self.token, path).await -} -``` - -## Use Self where possible -Bad: +When referring to the type for which block is implemented, prefer using `Self`, rather than the name of the type. ```rust impl ErrorKind { + // GOOD + fn print(&self) { + Self::Io => println!("Io"), + Self::Network => println!("Network"), + Self::Json => println!("Json"), + } + + // BAD fn print(&self) { ErrorKind::Io => println!("Io"), ErrorKind::Network => println!("Network"), @@ -102,50 +103,96 @@ impl ErrorKind { } } ``` - -Good: ```rust -impl ErrorKind { - fn print(&self) { - Self::Io => println!("Io"), - Self::Network => println!("Network"), - Self::Json => println!("Json"), +impl<'a> AnswerCallbackQuery<'a> { + // GOOD + fn new(bot: &'a Bot, callback_query_id: C) -> Self + where + C: Into, + { ... } + + // BAD + fn new(bot: &'a Bot, callback_query_id: C) -> AnswerCallbackQuery<'a> + where + C: Into, + { ... } +} +``` + +**Rationale:** `Self` is generally shorter and it's easier to copy-paste code or rename the type. + +## Avoid duplication in fields names + +```rust +struct Message { + // GOOD + #[serde(rename = "message_id")] + id: MessageId, + + // BAD + message_id: MessageId, +} +``` + +**Rationale:** duplication is unnecessary + +## Conventional generic names + +Use a generic parameter name `S` for streams, `Fut` for futures, `F` for functions (where possible). + +**Rationale:** uniformity. + +## Deriving traits + +Derive `Copy`, `Clone`, `Eq`, `PartialEq`, `Hash` and `Debug` for public types when possible. + +**Rationale:** these traits can be useful for users and can be implemented for most types. + +Derive `Default` when there is a reasonable default value for the type. + +**Rationale:** `Default` plays nicely with generic code (for example `mem::take`). + +## `Into`-polymorphism + +Use `T: Into` when this can simplify user code. +I.e. when there are types that implement `Into` that are likely to be passed to this function. + +**Rationale:** conversions unnecessarily complicate caller code and can be confusing for beginners. + +## `must_use` + +Always mark a functions as `#[must_use]` if they don't have side-effects and the only reason to call them is to get the result. + +```rust +impl User { + // GOOD + #[must_use] + fn full_name(&self) -> String { + format!("{} {}", user.first_name, user.last_name) } } ``` -
- More examples - -Bad: - +**Rationale:** users will get warnings if they forgot to do something with the result, potentially preventing bugs. + +## Creating boxed futures + +Prefer `Box::pin(async { ... })` instead of `async { ... }.boxed()`. + +**Rationale:** the former is generally formatted better by rustfmt. + +## Full paths for logging + +Always write `log::!(...)` instead of importing `use log::;` and invoking `!(...)`. + ```rust -impl<'a> AnswerCallbackQuery<'a> { - pub(crate) fn new(bot: &'a Bot, callback_query_id: C) -> AnswerCallbackQuery<'a> - where -C: Into, { ... } +// GOOD +log::warn!("Everything is on fire"); + +// BAD +use log::warn; + +warn!("Everything is on fire"); ``` -Good: - -```rust -impl<'a> AnswerCallbackQuery<'a> { - pub(crate) fn new(bot: &'a Bot, callback_query_id: C) -> Self - where -C: Into, { ... } -``` -
- -## Naming - 1. Avoid unnecessary duplication (`Message::message_id` -> `Message::id` using `#[serde(rename = "message_id")]`). - 2. Use a generic parameter name `S` for streams, `Fut` for futures, `F` for functions (where possible). - -## Deriving - 1. Derive `Copy`, `Eq`, `Hash`, `PartialEq`, `Clone`, `Debug` for public types when possible (note: if the default `Debug` implementation is weird, you should manually implement it by yourself). - 2. Derive `Default` when there is an algorithm to get a default value for your type. - -## Misc - 1. Use `Into<...>` only where there exists at least one conversion **and** it will be logically to use. - 2. Always mark a function as `#[must_use]` if its return value **must** be used. - 3. `Box::pin(async [move] { ... })` instead of `async [move] { ... }.boxed()`. - 4. Always write `log::!(...)` instead of importing `use log::;` and invoking `!(...)`. For example, write `log::info!("blah")`. +**Rationale:** uniformity, it's clearer which log crate is used.