Дано

На вашем проекте пишут шахматный AI. Вам прилетела задача. Есть вот такой код:

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub struct Address {
    pub row: u8,
    pub col: u8,
}
  
impl Address {
    pub fn parse(_s: &str) -> Self {
	    !todo!();
    }
}

Суть: доска в движке представлена как двухмерная матрица. Адресация клеток на доске производится координатами (row; col), где row и col находятся в диапазоне [0; 7].

Нас просят заимплементить метод Address::parse(). Он должен парсить человекочитаемый строковый адрес клетки шахматной доски, например "e2" и превращать в объект Address, с которым будет работать движок. Для "e2" это будет (1, 4).

Выполняем поставленную задачу

Окей, скажете вы, тут напрашивается trait FromStr, поскольку мы хотим создать объект из строки. А Address::parse() будет тонкой оберткой вокруг метода from_str. Делаем:

#[derive(Debug, PartialEq, Eq)]
pub struct ParseAddressError;

impl FromStr for Address {
    type Err = ParseAddressError;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let mut res: Self = Address { col: 0, row: 0 };

        let chars = s.chars().collect::<Vec<_>>();

        if chars.len() != 2 {
            return Err(ParseAddressError);
        }

        let col = chars[0].to_ascii_lowercase();
        let row = chars[1].to_ascii_lowercase();

        if let c @ 'a'..='h' = col {
            res.col = (c as u8) - ('a' as u8)
        } else {
            return Err(ParseAddressError);
        }

        if let r @ '0'..='7' = row {
            res.row = (r as u8) - ('0' as u8)
        } else {
            return Err(ParseAddressError);
        }

        Ok(res)
    }
}

Это было несложно, метод простой, понятный. Убеждаемся, что все собирается через cargo check, коммитим.

Потом вспоминаем, что мы забыли про метод parse. Окей, дописываем:

impl Address {
    pub fn parse(s: &str) -> Self {
        Address::from_str(s)
    }
}

Супер — коммитим, пушим, рапортуем о готовности.

Получаем фидбек

Через совсем небольшой промежуток времени нам дают по шапке, потому что "оно даже не компилируется", и нас просят чтобы больше такого не повторялось. Хм, идем смотреть, что там может быть не так, ведь мы делали cargo check. Сделаем его повторно:

error[E0308]: mismatched types
  --> src\chess_address.rs:11:9
   |
10 |     pub fn parse(s: &str) -> Self {
   |                              ---- expected `Address` because of return type
11 |         Address::from_str(s)
   |         ^^^^^^^^^^^^^^^^^^^^ expected `Address`, found `Result<Address, ParseAddressError>`
   |
   = note: expected struct `Address`

Блин, мы забыли проверить parse(), дописанный на скорую руку и пушнутый без проверок. Ндаа, досадная ошибка: поспешили и забыли приписать unwrap(). Перепроверить сборку тоже поленились — спешили запушиться поскорее. Делаем правильно:

impl Address {
    pub fn parse(s: &str) -> Self {
        Address::from_str(s).unwrap()
    }
}

Запускаем cargo check, и в этот раз код компилируется без ошибок. Здорово, снова коммитим и пушим, рапортуем.

Получаем фидбек II

Через совсем небольшой промежуток времени нам дают по шапке, потому что parse работает некорректно, и нас просят починить и впредь писать юнит-тесты. Обидно, но справедливо.

В лучших традициях TDD напишем тесты и с их помощью попробуем выяснить, где мы просчитались:

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn address_parse() {
        for (r_id, r_char) in ('1'..='8').enumerate() {
            for (c_id, c_char) in ('a'..='h').enumerate() {
                let addr_str = format!("{}{}", c_char, r_char);
                let addr = Address::parse(&addr_str);

                assert_eq!(addr, Address { row: r_id as u8, col: c_id as u8 });
            }
        }

        macro_rules! check_neg {
            ($addr:expr) => {
                assert_eq!(Address::from_str($addr), Err(ParseAddressError));
            };
        }

        check_neg!("");
        check_neg!("a");
        check_neg!("f11");
        check_neg!("6e");
        check_neg!("f9");
        check_neg!("j5");
        check_neg!("2");
        check_neg!("2789");
        check_neg!("1f");
        check_neg!("c0");
    }
}

Время найти злосчастный баг, запускаем cargo test:

assertion `left == right` failed
  left: Address { col: 0, row: 1 }
 right: Address { col: 0, row: 0 }

right — это то, что мы ожидали, left — то что вышло по факту. row по какой-то причине на единицу больше, чем должно быть. Идем смотреть, как в методе высчитывается row:

...

if let r @ '0'..='7' = row {
	res.row = (r as u8) - ('0' as u8)
} else {
	return Err(ParseAddressError);
}

...

Wait, OH SHI~ — мы опечатались и начали индексацию шахматных строк с нуля, хотя в шахматах колонки начинаются с числа 1, и зафакапили индексацию. Спешно вносим правки:

...

if let r @ '1'..='8' = row {
	res.row = (r as u8) - ('1' as u8)
} else {
	return Err(ParseAddressError);
}

...

Проверяем сборку, cargo build:

Process finished with exit code 0

Запускаем тесты:

Process finished with exit code 0

Ура! Баг пофикшен, код компилируется, репутация среди коллег подмочена.

Что я делаю не так?

На данном этапе мы понимаем, что в рабочем процессе что-то нужно менять, поскольку уровень нашей невнимательности высок, желание побыстрее запушить свое "я сделяль" — тоже, а в совокупности два эти фактора мешают работать нам и коллегам.

Вот бы автоматизировать процесс и сделать хитрый скрипт, который мог сам запустить cargo check, cargo test, ну и в довесок еще пару cargo-вкусностей, чтобы непосредственно к моменту коммита все было гарантированно рабочим.

Хук слева

Очевидно, что повествование здесь очень жирно намекает на использование git-хуков, которые просто не дадут коммитнуться неработающему коду, если хук правильно приготовить.

Из множества предлагаемых гитом хуков мы выбираем хук pre-commit, поскольку мы хотим делать все наши проверки перед коммитом. pre-push тоже сойдет.

Компиляция и тесты

Идем в папку .git/hooks, создаем там файл pre-commit со следующим содержимым:

#!/bin/sh

cargo check && cargo test

Это оказалось поразительно просто. После некоторых экспериментов с намеренными синтаксическими и логическими ошибками в коде мы убеждаемся, что это еще и действительно работает: код просто отказывается коммититься, если падают тесты или программа имеет ошибки компиляции, т.е. наш минимум выполнен, и мы больше не получим по шапке!

Форматирование

Что еще мы могли бы упихнуть в хук? Ну, например, очень напрашивается cargo fmt — команда, форматирующая ваш rust-код сообразно единому стайл-гайду. Сказано — сделано:

#!/bin/sh

cargo check && cargo test && cargo fmt

Вставим какой-нибудь лишний пробел в код и попробуем закоммитить это изменение, и посмотреть, что сделает cargo fmt. И вот тут все идет не по плану. Первое, что мы видим — коммит с лишним пробелом прошел. Второе, что мы видим — git показывает, что у нас есть not staged changes, смотрим какие:

@@ -57,7 +57,13 @@ mod tests {
                 let addr_str = format!("{}{}", c_char, r_char);
                 let addr = Address::parse(&addr_str);

-                assert_eq!(addr, Address { row: r_id as u8, col: c_id as u8 });
+                assert_eq!(
+                    addr,
+                    Address {
+                        row: r_id as u8,
+                        col: c_id as u8
+                    }
+                );
             }
         }

Ага, окей, нашему коду, оказывается, действительно не хватало немного форматирования по мнению cargo fmt. И форматирование сделано, но мимо кассы — форматирование не попало в наш коммит, и это проблема.

Давайте разберемся, что мы имеем:

• cargo fmt (если команда выполнена именно в такой форме, без флагов) никогда не возвращает ошибку. Она всегда успешно отработает, если не случится какого-то жесткого внутреннего противоречия, чего скорее всего никогда не произойдет. Поэтому эта команда не может прервать коммит, он всегда произойдет

• cargo fmt делает правки в коде, эти правки, очевидно, будут считаться как not staged changes

• Хук pre-commit выполняется перед коммитом

Внимательно анализируем все тезисы и приходим к выводу, что нам нужно тут же в хуке применить изменения в коде, сделанные командой cargo fmt. Хмм, а что если выполнить git add . прямо в хуке? Ничто не мешает нам попробовать:

#!/bin/sh

cargo check \
&& cargo test \
&& cargo fmt \
&& git add .

Откатим тот бардак, что мы сделали командой git reset --hard HEAD~1 и попробуем повторить процедуру снова: вставляем куда-нибудь в код лишний пробел и пытаемся закоммитить. Смотрим, что вышло: git status, показывает, что у нас все чисто; история показывает, что у нас есть наш коммит, diff которого выглядит следующим образом:

@@ -57,7 +57,13 @@ mod tests {
                 let addr_str = format!("{}{}", c_char, r_char);
                 let addr = Address::parse(&addr_str);
 
-                assert_eq!(addr, Address { row: r_id as u8, col: c_id as u8 });
+                assert_eq!(
+                    addr,
+                    Address {
+                        row: r_id as u8,
+                        col: c_id as u8
+                    }
+                );
             }
         }

Ура, наш git add . внутри хука сработал, и мы стали еще неуязвимее — теперь наш код будет всегда отформатированным, даже если мы в процессе разработки не будем соблюдать правила форматирования от слова совсем. Это ли не чудо?

Линтер

Какие еще интересные cargo-утилиты мы знаем, которые могут сделать наш код лучше и чище? cargo clippy — это линтер, что-то вроде утилиты статического анализа кода для выявления и предупреждения сомнительных или неоптимальных мест в коде.

Памятуя об особенностях выполнения cargo fmt, сразу идем в документацию cargo clippy, чтобы узнать, какие подводные камни нас ждут. Узнаем, что в обычной ситуации clippy возвращает код 0 (успешное выполнение), даже если он нашел и вывел на экран warning'и. Нам это не подходит — так мы увидим warning'и на экране, но наш поезд уже уйдет, и коммит будет сделан вопреки наличию warning'ов. Нужно сделать так, чтобы clippy серьезнее воспринимал warning'и и возвращал неуспех, и коммит был отклонен хуком.

Тут же в документации находим устраивающий нас подход:

For CI all warnings can be elevated to errors which will inturn fail the build and cause Clippy to exit with a code other than 0.

`cargo clippy -- -Dwarnings`

Окей, дописываем в наш хук:

#!/bin/sh

cargo check \
&& cargo test \
&& cargo fmt \
&& git add . \
&& cargo clippy -- -D warnings

Пытаемся закоммитить что-нибудь в наш код. Интересно, поймает ли clippy хоть что-то?

error: casting a character literal to `u8` truncates
  --> src\chess_address.rs:34:35
   |
34 |             res.col = (c as u8) - ('a' as u8)
   |                                   ^^^^^^^^^^^ help: use a byte literal instead: `b'a'`
   |
   = note: `char` is four bytes wide, but `u8` is a single byte
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8
   = note: `-D clippy::char-lit-as-u8` implied by `-D warnings`

error: casting a character literal to `u8` truncates
  --> src\chess_address.rs:40:35
   |
40 |             res.row = (r as u8) - ('1' as u8)
   |                                   ^^^^^^^^^^^ help: use a byte literal instead: `b'1'`
   |
   = note: `char` is four bytes wide, but `u8` is a single byte
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8

error: could not compile `git_hooks` (bin "git_hooks") due to 2 previous errors

Вот это я понимаю сервис — линтер показал нам, как сделать код не только правильнее, но и короче, и теперь мы можем заменить:

res.col = (c as u8) - ('a' as u8)

на

res.col = (c as u8) - b'a'

Итоги

С таким хуком мы можем надеяться на то, что мы не получим по шапке и в целом будем иметь автоматизированную проверку качества нашего кода с минимальной возможностью дать себе слабину. Все, что останется — не забывать честно обмазывать наш код юнит-тестами.

Итак, итоговый вид нашего хука:

#!/bin/sh

cargo check \
&& cargo test \
&& cargo fmt \
&& git add . \
&& cargo clippy -- -D warnings

Помним, что хук pre-commit локальный. Это значит, что нам не нужно бояться, что мы заупшим его в репозиторий.

Я не пишу на Rust, я тут за идеей

Хук для вашего стека по смыслу не будет принципиально отличаться от того, что здесь написано. Вы должны лишь найти способ, как через хук-скрипт запустить ваши компилятор, тестовую утилиту, форматтер, линтер. Чего-то у вас может не присутствовать, а, возможно, добавится и дополнительная утилита, специфическая для вашего стека или пайплайна.

Могу предположить, что ваш хук может выглядеть сложнее, т.к. не каждый язык/фреймфорк имеет такой удобный пакетный менеджер как cargo (если вообще имеет пакетный менеджер — привет, C++).

Хук можно писать не только на башике, но и на питоне или, прости господи, перле. Тогда в шапке хука напишите #!/usr/bin/env python или #!/usr/bin/perl. С питоном на Windows, правда, могут возникнуть проблемы — подробнее о них можно почитать в этой хабра-статье.

Я пишу на Rust, помогите

Хук, описанный в статье, можно вообще организовать через специально сделанную для этого утилиту rusty-hook. Все, что нужно сделать, это создать в корне проекта файл .rusty-hook.toml со следующим содержимым:

[hooks]
pre-commit = "cargo check && cargo test && cargo fmt && git add . && cargo clippy -- -D warnings"

[logging]
verbose = true

Тонкостей утилиты не знаю, и стоит ли ее использовать, имеет ли она какие-то преимущества или недостатки перед идиоматическим созданием git-хуков, решать вам.