Skip to content

Latest commit

 

History

History
302 lines (227 loc) · 8.08 KB

quick-reference.md

File metadata and controls

302 lines (227 loc) · 8.08 KB

Quick Reference - Шпаргалка по Code Review

Быстрая справка для ежедневного использования.

🚨 Критические проблемы (блокируют merge)

Безопасность

# Поиск секретов
rg "(?i)(password|secret|api_key|token|private_key).*=.*['\"]" --type rust

# Поиск SQL инъекций
rg "format!.*SELECT|execute.*&format!" --type rust

# Поиск command инъекций
rg "Command::new.*format!" --type rust

Чек-лист:

  • Нет hardcoded credentials
  • SQL запросы параметризованы
  • Валидация всех входных данных
  • auth_date проверяется для защиты от replay attacks
  • Нет слабых алгоритмов (MD5, SHA1)

Panic в production

# Найти все unwrap/expect/panic
rg "\.unwrap\(\)|\.expect\(|panic!" --type rust --glob '!tests'

Правило: Нет panic в production коде (только Result).

let-else vs map_err цепочки

// ❌ Длинные цепочки
let value = parse(input).map_err(E::A)?.validate().map_err(E::B)?;

// ✅ Читаемый код
let Ok(value) = parse(input) else { return Err(E::A); };
let Ok(value) = value.validate() else { return Err(E::B); };

Правило: let-else для нескольких шагов, map_err — для одного.

⚡ Проблемы производительности

Быстрые проверки

# Вложенные циклы (потенциальный O(n²))
rg "for.*\{[\s\S]{1,200}for" --type rust

# Vec::new без with_capacity
rg "Vec::new\(\)" --type rust

# contains в цикле
rg "\.contains\(" --type rust

# Лишние clone
rg "\.clone\(\)" --type rust

Вопросы:

  • Парсинг происходит один раз?
  • Используется with_capacity для Vec?
  • Нет O(n²) где можно O(n)?
  • Используются итераторы вместо промежуточных Vec?

📋 Чеклист на 5 минут

1. Безопасность (2 мин)

  • Нет секретов в коде
  • Нет unwrap/expect
  • Входные данные валидируются
  • Нет SQL/Command инъекций

2. Производительность (1 мин)

  • Нет очевидных O(n²)
  • Нет дублирования операций
  • Vec::with_capacity где нужно

3. Качество (2 мин)

  • Нет дублирования кода (> 3 раз)
  • Функции < 50 строк
  • Нормальные имена переменных
  • Есть тесты на новую логику

🔍 Частые паттерны проблем

1. Replay Attack

// ❌ ПЛОХО
fn validate(raw: &str, hash: &str) -> bool {
    compute_hash(raw) == hash  // Нет проверки времени!
}

// ✅ ХОРОШО
fn validate(raw: &str, hash: &str, max_age: u64) -> bool {
    check_timestamp(raw, max_age) && compute_hash(raw) == hash
}

2. Double Parsing

// ❌ ПЛОХО - парсим дважды
fn validate(raw: &str) -> bool {
    let data = parse(raw);  // 1
    check(data);
    hash(raw)  // parse() вызывается внутри снова! 2
}

// ✅ ХОРОШО
fn validate(raw: &str) -> bool {
    let data = parse(raw);  // 1 раз
    check(&data);
    hash(&data)  // передаем ссылку
}

3. N+1 Query

// ❌ ПЛОХО
for id in ids {
    let user = db.get(id).await;  // N запросов
}

// ✅ ХОРОШО
let users = db.get_many(ids).await;  // 1 запрос

4. Unnecessary Clone

// ❌ ПЛОХО
fn process(data: Vec<String>) -> usize {
    data.len()  // Забирает ownership зря
}

// ✅ ХОРОШО
fn process(data: &[String]) -> usize {
    data.len()
}

🛠️ Быстрые команды

Поиск проблем

# Все unwrap/expect
rg "\.unwrap\(\)|\.expect\(" --type rust --glob '!tests'

# Magic numbers
rg "\b[0-9]{3,}\b" --type rust

# TODO/FIXME
rg "TODO|FIXME" --type rust

# Длинные функции (>50 строк)
# (нужен скрипт или ручная проверка)

Метрики

# Покрытие тестами
cargo tarpaulin --out Xml

# Clippy
cargo clippy -- -D warnings

# Форматирование
cargo +nightly fmt --check

# Тесты
cargo test

# Бенчмарки
cargo bench

📊 Приоритизация

MUST FIX (блокирует merge)

  1. Уязвимости безопасности
  2. Panic в production
  3. Логические ошибки
  4. Сломанные тесты

SHOULD FIX (перед merge)

  1. Проблемы производительности (>10% деградация)
  2. Дублирование кода (>3 раз)
  3. Отсутствие тестов для критичной логики

NICE TO HAVE (можно отдельным PR)

  1. Рефакторинг для читаемости
  2. Дополнительные тесты
  3. Улучшение документации

💬 Как писать комментарии к review

❌ Плохо

  • "Это плохо"
  • "Переделай"
  • "Тут проблема"

✅ Хорошо

  • "Здесь возможна SQL-инъекция в строке 42. Используй параметризованный запрос: query_as!(...)"
  • "Эта функция вызывается в цикле, что дает O(n²). Можно использовать HashMap для O(n)"
  • "Нет проверки auth_date - уязвимость к replay attack. Добавь валидацию времени по документации Telegram"

Формула:

  1. Что не так
  2. Почему это проблема
  3. Как исправить (или предложить варианты)

🎯 Фокус по времени

5 минут - Критичное

  • Безопасность
  • Panic/unwrap
  • Очевидные баги

15 минут - Добавить производительность

  • Дублирование операций
  • Неэффективные алгоритмы
  • Лишние аллокации

30 минут - Добавить качество

  • Дублирование кода
  • Читаемость
  • Тесты
  • Документация

45 минут - Тщательный review

  • Архитектура
  • Edge cases в тестах
  • Долгосрочная поддерживаемость

📝 Шаблон комментария

## Security
- [ ] auth_date validation missing (replay attack) - line 42
- [ ] SQL injection in `get_user()` - line 156

## Performance
- [ ] Double parsing of initData - lines 50, 75
- [ ] O(n²) in `remove_duplicates()` - line 200

## Quality
- [ ] Hash computation duplicated 3 times - suggest extract function
- [ ] Missing tests for error cases

## Suggestions
- Consider using HashMap instead of Vec::find for O(1) lookups
- Add doc comments for public API

🚀 Быстрый старт

  1. Открой PR → смотри diff
  2. Безопасность (5 мин):
    • rg секреты, unwrap, инъекции
  3. Производительность (5 мин):
    • Дублирование, O(n²), аллокации
  4. Качество (5 мин):
    • DRY, читаемость, тесты
  5. Пиши комментарии - конкретно и с решениями
  6. Approve или Request Changes

Итого: 15-20 минут на обычный PR

🔗 Полная документация

Подробности смотри в: