Code Review в Go: что проверять и как давать обраттную связь на практике

Введение: почему code review в Go — это не просто галочка

Code review — это не бюрократическая процедура, а ключевой элемент культуры разработки. В Go, с его лаконичным синтаксисом и строгими правилами, ревью часто становится первым фильтром, который отсеивает проблемы на раннем этапе. Многие команды, переходящие на Golang, сталкиваются с тем, что старые привычки (например, из Python или PHP) не работают: Go прощает меньше ошибок, связанных с типами, но требует внимания к деталям.

На практике code review в Go решает три задачи: обучение команды (новички быстрее вливаются в кодстайл), повышение качества (поиск багов и антипаттернов) и согласованность кода (чтобы проект не превратился в "лоскутное одеяло"). Давайте разберем, на что конкретно смотреть и как формулировать замечания.

1. Что проверять в первую очередь: от синтаксиса до ошибок

Любое ревью начинается с базовых вещей. В Go есть несколько "горячих точек", которые стоит проверять автоматически или вручную.

1.1. Обработка ошибок (error handling)

Go не использует исключения — ошибки возвращаются как значения. Частая проблема: разработчик игнорирует возвращаемую ошибку с помощью _. Это допустимо только в редких случаях (например, fmt.Println).

// Плохо: ошибка игнорируется
resp, _ := http.Get("https://api.example.com/data")

// Хорошо: ошибка обрабатывается
resp, err := http.Get("https://api.example.com/data")
if err != nil {
    log.Printf("не удалось получить данные: %v", err)
    return err
}
defer resp.Body.Close()

Что проверять:

• Все ли ошибки обработаны (не просто _)?
• Используется ли defer для закрытия ресурсов (файлы, соединения)?
• Не "проглатываются" ли ошибки в горутинах?

1.2. Именование и стиль (Go idioms)

Go требует краткости: camelCase для приватных, PascalCase для экспортируемых. Избегайте "венгерской нотации" и длинных имен.

// Плохо
func GetUserDataFromDatabase(userID int) (*UserData, error)

// Хорошо
func GetUser(id int) (*User, error)

Красные флаги:

• Имена переменных типа temp, data, val (слишком общие).
• Функции длиннее 30-40 строк (нужно разбивать).
• Отсутствие комментариев к экспортируемым функциям (это требование golint).

1.3. Горутины и синхронизация

Это самая сложная часть. Ошибки в конкурентном коде — частый источник багов в продакшене.

// Плохо: гонка данных (data race)
var counter int
for i := 0; i < 100; i++ {
    go func() {
        counter++ // небезопасно
    }()
}

// Хорошо: используем мьютекс
var mu sync.Mutex
var counter int
for i := 0; i < 100; i++ {
    go func() {
        mu.Lock()
        counter++
        mu.Unlock()
    }()
}

Что проверять:

• Есть ли sync.Mutex или atomic для общих данных?
• Не забыли ли закрыть канал (close(ch))?
• Не "утекают" ли горутины (нет select с таймаутом)?

2. Архитектурные проверки: от интерфейсов до пакетов

После базового синтаксиса смотрим на структуру кода. Go поощряет композицию, а не наследование.

2.1. Интерфейсы: маленькие и на стороне потребителя

В Go интерфейсы реализуются неявно. Хороший интерфейс содержит 1-3 метода. Проверяйте, не пытается ли автор сделать "всеобъемлющий" интерфейс.

// Плохо: огромный интерфейс
type Storage interface {
    Save(data []byte) error
    Load(id string) ([]byte, error)
    Delete(id string) error
    List() ([]string, error)
    Connect() error
    Disconnect() error
}

// Хорошо: разделено на мелкие интерфейсы
type Saver interface {
    Save(data []byte) error
}
type Loader interface {
    Load(id string) ([]byte, error)
}

Совет: Если интерфейс имеет более 3-х методов, скорее всего, его можно разбить.

2.2. Структура пакетов

Go не любит глубокую вложенность. Избегайте пакетов utils, common или helpers — это "свалка". Каждый пакет должен отвечать на вопрос "что делает этот код?".

• Нет циклических зависимостей (это ошибка компиляции).
• Пакеты названы существительными (user, payment), а не глаголами (processing).
• Один пакет — одна ответственность.

2.3. Тестирование

Code review без проверки тестов — половина дела. В Go тесты пишутся в том же пакете (или с суффиксом _test).

// Плохо: тест не проверяет ошибку
func TestAdd(t *testing.T) {
    result := Add(2, 3)
    if result != 5 {
        t.Errorf("ожидалось 5, получено %d", result)
    }
}

// Хорошо: table-driven тест
func TestAdd(t *testing.T) {
    tests := []struct {
        a, b, expected int
    }{
        {2, 3, 5},
        {-1, 1, 0},
        {0, 0, 0},
    }
    for _, tt := range tests {
        result := Add(tt.a, tt.b)
        if result != tt.expected {
            t.Errorf("Add(%d, %d) = %d; want %