Привет, Хабр. Меня зовут Сергей Рудаченко, я техлид в компании Roistat. Последние два года наша команда переводит различные части проекта в микросервисы на Go. Они разрабатываются несколькими командами, поэтому нам понадобилось задать жесткую планку качества кода. Для этого мы используем несколько инструментов, в этой статье речь пойдет об одном из них — о статическом анализе.


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


В статьях по этой теме часто встречается термин «линтер». Для нас это удобное название простых инструментов для статического анализа. Задача линтера — поиск простых ошибок и некорректного оформления.


Зачем нужны линтеры?


Работая в команде, вы, скорее всего, выполняете ревью кода. Ошибки, пропущенные на ревью, — это потенциальные баги. Пропустили необработанный error — не получите информативного сообщения и будете искать проблему вслепую. Ошиблись в приведении типов или обратились к nil map — еще хуже, бинарник упадет с panic.


Описанные выше ошибки можно добавить в Code Conventions, но найти их при чтении пулл реквеста не так просто, ведь ревьюеру придется вчитываться в код. Если в вашей голове нет компилятора, часть проблем все равно пройдет на бой. Кроме того, поиск мелких ошибок отвлекает от проверки логики и архитектуры. На дистанции поддержка такого кода станет дороже. Мы пишем на статически типизированном языке, странно этим не воспользоваться.


Популярные инструменты


Большинство утилит для статического анализа используют пакеты go/ast и go/parser. Они предоставляют функции для разбора синтаксиса .go файлов. Стандартный поток выполнения (например, для утилиты golint) такой:


  • загружается список файлов из требуемых пакетов
  • для каждого файла выполняется parser.ParseFile(...) (*ast.File, error)
  • запускается проверка поддерживаемых правил для каждого файла или пакета
  • проверка проходит по каждой инструкции, например, вот так:

f, err := parser.ParseFile(/* ... */)

ast.Walk(func (n *ast.Node) {
    switch v := node.(type) {
    case *ast.FuncDecl:
        if strings.Contains(v.Name, "_") {
            panic("wrong function naming")
        }
    }
}, f)

Помимо AST существует Single Static Assignment (SSA). Это более сложный способ анализа кода, который работает с потоком выполнения, а не синтаксическими конструкциями. В этой статье мы не будем рассматривать его подробно, можете почитать документацию и взглянуть на пример утилиты stackcheck.


Далее будут рассмотрены только популярные утилиты, которые выполняют полезные для нас проверки.


gofmt


Это стандартная утилита из пакета go, которая проверяет соответствие стилю и может автоматически его исправлять. Соответствие стилю для нас обязательное требование, поэтому проверка gofmt включена во всех наших проектах.


typecheck


Typecheck проверяет соответствие типов в коде и поддерживает vendor (в отличие от gotype). Ее запуск обязателен для проверки компилируемости, но не дает полных гарантий.


go vet


Утилита go vet — часть стандартного пакета и рекомендована к использованию командой Go. Проверяет ряд типичных ошибок, например:


  • неправильное использование Printf и аналогичных функций
  • некорректные build теги
  • сравнение function и nil

golint


Golint разработан командой Go и проверяет код на основе документов Effective Go и CodeReviewComments. К сожалению, подробной документации нет, но по коду можно понять, что проверяется следующее:


f.lintPackageComment()
f.lintImports()
f.lintBlankImports()
f.lintExported()
f.lintNames()
f.lintVarDecls()
f.lintElses()
f.lintRanges()
f.lintErrorf()
f.lintErrors()
f.lintErrorStrings()
f.lintReceiverNames()
f.lintIncDec()
f.lintErrorReturn()
f.lintUnexportedReturn()
f.lintTimeNames()
f.lintContextKeyTypes()
f.lintContextArgs()

staticcheck


Сами разработчики представляют staticcheck как улучшенный go vet. Проверок много, они разбиты по группам:


  • неправильные использования стандартных библиотек
  • проблемы с многопоточностью
  • проблемы с тестами
  • бесполезный код
  • проблемы производительности
  • сомнительные конструкции

gosimple


Специализируется на поиске конструкций, которые стоит упростить, например:


До (исходный код golint)


func (f *file) isMain() bool {
    if f.f.Name.Name == "main" {
        return true
    }
    return false
}

После


func (f *file) isMain() bool {
    return f.f.Name.Name == "main"
}

Документация аналогична staticcheck и включает подробные примеры.


errcheck


Возвращаемые функциями ошибки нельзя игнорировать. О причинах подробно рассказано в обязательном к прочтению документе Effective Go. Errcheck не пропустит следующий код:


json.Unmarshal(text, &val)
f, _ := os.OpenFile(/* ... */)

gas


Находит уязвимости в коде: захардкоженные доступы, sql инъекции и использование небезопасных хэш-функций.


Примеры ошибок:


// доступ со всех IP адресов
l, err := net.Listen("tcp", ":2000")

// потенциальная sql инъекция
q := fmt.Sprintf("SELECT * FROM foo where name = '%s'", name)
q := "SELECT * FROM foo where name = " + name

// используйте другой хэш алгоритм
import "crypto/md5"

maligned


В Go порядок полей в структурах влияет на потребление памяти. Maligned находит неоптимальную сортировку. При таком порядке полей:


struct {
    a bool
    b string
    c bool
}

Структура займет в памяти 32 бита из-за добавления пустых битов после полей a и c.


image


Если мы поменяем сортировку и поставим два bool поля вместе, то структура займет всего 24 бита:


image


Оригинал картинки на stackoverflow


goconst


Магические переменные в коде не отражают смысл и усложняют чтение. Goconst находит литералы и числа, которые встречаются в коде 2 раза и более. Обратите внимание, часто даже единственное использование может быть ошибкой.


gocyclo


Мы считаем цикломатическую сложность кода важной метрикой. Gocycle показывает сложность для каждой функции. Можно вывести только функции, которые превышают указанное значение.


gocyclo -over 7 package/name

Мы выбрали для себя пороговое значение 7, поскольку не нашли кода с более высокой сложностью, который не требовал рефакторинга.


Мертвый код


Есть несколько утилит для поиска неиспользуемого кода, их функциональность может частично пересекаться.


  • ineffassign: проверяет бесполезные присвоения

func foo() error {
    var res interface{}
    log.Println(res)
    res, err := loadData() //  переменная res дальше не используется
    return err
}

  • deadcode: находит неиспользуемые функции
  • unused: находит неиспользуемые функции, но делает это лучше, чем deadcode

func unusedFunc() {
    formallyUsedFunc()
}

func formallyUsedFunc() {
}

В результате unused укажет сразу на обе функции, а deadcode только на unusedFunc. Благодаря этому лишний код удаляется за один проход. Также unused находит неиспользуемые переменные и поля структур.


  • varcheck: находит неиспользуемые переменные
  • unconvert: находит бесполезные приведения типов

var res int
return int(res) // unconvert error

Если нет задачи экономить на времени запуска проверок, лучше запускать их все вместе. Если нужна оптимизация, рекомендую использовать unused и unconvert.


Как это все удобно настроить


Запускать перечисленные выше инструменты последовательно неудобно: ошибки выдаются в разном формате, выполнение занимает много времени. Проверка одного из наших сервисов размером ~8000 строк кода занимала больше двух минут. Устанавливать утилиты тоже придется по-отдельности.


Для решения этой проблемы есть утилиты-аггрегаторы, например goreporter и gometalinter. Goreporter рендерит отчет в html, а gometalinter пишет в консоль.


Gometalinter до сих пор используется в некоторых крупных проектах (например, docker). Он умеет устанавливать все утилиты одной командой, запускать их параллельно и форматировать ошибки по шаблону. Время выполнения в упомянутом выше сервисе сократилось до полутора минут.


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


В мае 2018 года на гитхабе появился проект golangci-lint, который сильно превосходит gometalinter в удобстве:


  • время выполнения на том же проекте сократилось до 16 секунд (в 8 раз)
  • практически не встречаются дубликаты ошибок
  • понятный yaml конфиг
  • приятный вывод ошибок со строкой кода и указателем на проблему
  • не требуется устанавливать дополнительные утилиты

Сейчас прирост в скорости обеспечивается переиспользованием SSA и loader.Program, в будущем планируется также переиспользовать дерево AST, о котором я писал в начале раздела Инструменты.


На момент написания статьи на hub.docker.com не было образа с документацией, поэтому мы сделали собственный, настроенный по нашим представлениям об удобстве. В будущем конфиг будет изменяться, так что для продакшена рекомендуем заменить его на собственный. Для этого достаточно добавить в корневую директорию проекта файл .golangci.yaml (пример есть в репозитории golangci-lint).


PACKAGE=package/name docker run --rm -t     -v $(GOPATH)/src/$(PACKAGE):/go/src/$(PACKAGE)     -w /go/src/$(PACKAGE)     roistat/golangci-lint

Этой командой можно проверить весь проект. Например, если он находится в ~/go/src/project, то поменяйте значение переменной на PACKAGE=project. Проверка работает рекурсивно по всем внутренним пакетам.


Обратите внимание, что эта команда работает корректно только при использовании vendor.


Внедрение


Во всех наших сервисах для разработки используется docker. Любой проект запускается без установленного окружения go. Для запуска команд используем Makefile и добавили в него команду lint:


lint:
    @docker run --rm -t -v $(GOPATH)/src/$(PACKAGE):/go/src/$(PACKAGE) -w /go/src/$(PACKAGE) roistat/golangci-lint

Теперь проверка запускается этой командой:


make lint

Есть простой способ заблокировать код с ошибками от попадания в мастер — создать pre-receive-hook. Он подойдет, если:


  1. У вас небольшой проект и мало зависимостей (или они находятся в репозитории)
  2. Для вас не проблема ждать выполнения команды git push несколько минут

Инструкция по настройке хуков: Gitlab, Bitbucket Server, Github Enterprise.


В остальных случаях лучше использовать CI и запретить мерж кода, в котором есть хоть одна ошибка. Мы поступаем именно так, добавляя запуск линтеров перед тестами.


Заключение


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

Комментарии (9)


  1. powerman
    08.06.2018 02:57
    +1

    Спасибо за наводку на golangci-lint, реально раз в 5 быстрее gometalinter. Ну и настраивать его через конфиг удобнее.


  1. Artem_zin
    08.06.2018 07:57

    форматтинг можно засунуть в pre-commit hook (есть нюансы, но в целом норм), одной бесякой будет меньше)


    1. aml
      08.06.2018 13:44

      Форматтинг (вместе с goimports) удобно засовывать в on-save hook в редакторе.


      1. Artem_zin
        08.06.2018 23:06

        это несомненно вариант, но в команде побольше он проблематичный :(

        разработчики используют разные ide/редакторы, автоматический провиженинг ide/редакторов может далеко не всем понравиться


        1. aml
          09.06.2018 09:03

          О, кстати, а вы не знаете способ, как автопровижнить pre-commit hook в git?


          1. Artem_zin
            09.06.2018 09:18
            +1

            Совсем автоматически никак, только если в гите нет уязвимости через которую это можно сделать)

            Если у вас есть Makefile или другой похожий энтрипоинт, можно через него, я делаю так, раскатывает на примерно 50 человек, всё ок


  1. jirfag
    08.06.2018 12:07
    +2

    Спасибо за статью и использование golangci-lint, я, как его автор хотел бы дополнить двумя пунктами:

    1. Рекомендую устанавливать фиксированную версию, а не через go get (увидел это в докерфайле): так билды в CI будут стабильнее при улучшении существующих линтеров
    2. Есть классные опции --new/--new-from-rev: они здорово помогают интегрироваться в крупные проекты — ошибки ищутся только в новом коде. Например, так смогли применить golint в GoogleContainerTools/skaffold.


    И было бы здорово ссылку на проект добавить, по аналогии с goreporter и gometalinter.


    1. m1nor Автор
      08.06.2018 13:53
      +1

      Спасибо за комментарии и прекрасный инструмент :) Добавил ссылку, зафиксировал версию в контейнере.


    1. powerman
      08.06.2018 22:36
      +1

      Стабильные билды в CI важны только тогда, когда линтеры не важны. Типа, формально мы линтеры используем, но на практике в коде сплошной //nolint и версия линтера устарела на пару лет.


      Я предпочитаю другой подход, при котором ошибки линтеров ничем не хуже ошибок тестов. Тесты могут поломаться из-за обновления зависимостей, равно как и из-за обновления используемых инструментов — в первую очередь самого Go, но так же и линтеров. Поломался билд в CI — ну так почини. Из-за обновления линтера? И что? Ты не виноват??? Не виноват, да. Тебя никто и не виноватит. Просто почини, раз сломалось именно на твоём PR, и двигайся дальше. В крайнем случае, если сейчас полноценно чинить нет времени — добавь исключение в линтер и таску в бэклог чтобы исключение удалить.


      В общем, не понимаю я этой паники насчёт стабильных версий в CI. Всё, что попадает в master (я сейчас про master-ветку линтера) — должно быть достаточно стабильным чтобы им пользоваться, это навязано стилем разработки на Go да и вообще здравая идея. А штатные изменения в поведении линтеров, которые не являются багами самих линтеров — вполне приемлемы и в CI.