Дано
На вашем проекте пишут шахматный 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-хуков, решать вам.