Code Review на Rust: что проверять, чтобы код не рассыпался

Почему code review на Rust — это не просто проверка скобок

Rust — язык, который обещает безопасную работу с памятью и высокую производительность. Но даже самый строгий компилятор не убережёт от логических ошибок, неидиоматичного кода или неоправданного использования unsafe. Code review на Rust — это искусство. Вы не просто смотрите, закрыты ли фигурные скобки. Вы проверяете, как разработчик обошёл (или не обошёл) ограничения borrow checker'а, насколько эффективно использует итераторы и не пытается ли «перехитрить» систему с помощью сырых указателей.

В этой статье я расскажу, на какие аспекты стоит обращать внимание при ревью кода на Rust. Без лишней теории — только практические советы, примеры и чеклист, который можно повесить на стену (или сохранить в закладки). Поехали!

1. Владение, заимствование и времена жизни: главный источник боли

Первое, что нужно проверять — правильно ли работает система владения. Даже если код компилируется, это не значит, что он корректен. Типичные проблемы:

• Излишнее клонирование. Когда можно было бы использовать ссылку, разработчик копирует данные. Это убивает производительность.
• Заимствование мутабельного и иммутабельного. Если в одном скоупе есть &mut T и &T одновременно — будет ошибка. Но иногда можно переупорядочить код, чтобы этого избежать.
• Некорректные времена жизни (lifetimes). Особенно в сложных структурах с несколькими ссылками.

Пример: плохой код

fn process(data: &Vec<i32>) -> i32 {
    let mut sum = 0;
    for &x in data.iter() {
        sum += x;
    }
    // Всё ок, но зачем передавать &Vec, если можно &[i32]?
    sum
}

Хороший код

fn process(data: &[i32]) -> i32 {
    data.iter().sum()
}

Что проверять на ревью: не передаётся ли &Vec, &String или &HashMap туда, где достаточно слайса? Используются ли clone() без необходимости? Правильно ли расставлены аннотации времён жизни?

2. Unsafe: красная зона

Блоки unsafe — это как открытый люк в безопасный Rust. Они должны быть оправданы, минимальны и тщательно документированы. На code review обращайте внимание на:

• Почему используется unsafe? Часто можно обойтись безопасными абстракциями.
• Корректность сырых указателей. Не разыменовывается ли нулевой указатель? Не висит ли ссылка после освобождения памяти?
• Инварианты. Код с unsafe должен явно описывать, какие инварианты он поддерживает.

Пример: опасный unsafe

unsafe fn dangerous() {
    let ptr = 0x12345 as *mut i32;
    *ptr = 42; // А если по этому адресу ничего нет?
}

Такой код не должен проходить ревью. Даже если «оно работает» — это не аргумент.

Хороший unsafe

unsafe fn init_buffer(buf: &mut [u8]) {
    // Мы уверены, что buf указывает на валидную память,
    // так как вызывающая сторона гарантирует это.
    std::ptr::write_bytes(buf.as_mut_ptr(), 0, buf.len());
}

Что проверять: есть ли комментарий с обоснованием каждого unsafe? Есть ли тесты, которые проверяют граничные случаи? Не нарушаются ли гарантии безопасности?

3. Замыкания и функциональный стиль

Rust поощряет функциональный подход: итераторы, замыкания, map, filter, fold. Но это может привести к переусложнению. На ревью смотрите:

• Не злоупотребляют ли замыканиями? Если замыкание захватывает много переменных, возможно, стоит вынести логику в отдельную функцию.
• Мутабельное замыкание. Если замыкание изменяет внешнее состояние, это может быть неочевидно.
• Производительность. Итераторы обычно эффективны, но вложенные цепочки могут порождать лишние аллокации.

Пример: запутанный код

let result: Vec<_> = data
    .iter()
    .filter(|x| x % 2 == 0)
    .map(|x| x * 2)
    .collect::<Vec<_>>()
    .into_iter()
    .filter(|x| x > 10)
    .collect();

Здесь лишняя коллекция в середине. Лучше объединить фильтры.

Хороший код

let result: Vec<_> = data
    .iter()
    .filter(|x| x % 2 == 0 && x * 2 > 10)
    .map(|x| x * 2)
    .collect();

Что проверять: не избыточна ли цепочка итераторов? Не захватывает ли замыкание лишние переменные? Есть ли комментарии для сложных лямбд?

4. Обработка ошибок: не глотай их

Rust не использует исключения, но ошибки могут быть тихими. Типичные ошибки на ревью:

• Использование unwrap() или expect(). В production-коде это редко оправдано. Лучше обрабатывать ошибки через match, ? или map_err.
• Игнорирование Result. Если функция возвращает Result, а вызывающий код просто вызывает её и ничего не делает с результатом — это баг.
• Путаница между Option и Result. Option — для отсутствия значения, Result — для ошибок. Не надо использовать None как ошибку.

Пример: плохой код

let file = File::open("config.toml").unwrap(); // Паника, если файла нет

Хороший код

let file = File::open("config.toml")
    .map_err(|e| format!("Не удалось открыть конфиг: {}", e))?;

Что проверять: все ли места с unwrap оправданы? Есть ли обработка Err? Используется ли оператор ? для раннего возврата?