Статический анализ в Go: как мы экономим время при проверке кода


    Привет, Хабр. Меня зовут Сергей Рудаченко, я техлид в компании 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 и запретить мерж кода, в котором есть хоть одна ошибка. Мы поступаем именно так, добавляя запуск линтеров перед тестами.


    Заключение


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

    Roistat 36,76
    Система сквозной бизнес-аналитики
    Поделиться публикацией
    Комментарии 12
    • +2

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

      • 0
        форматтинг можно засунуть в pre-commit hook (есть нюансы, но в целом норм), одной бесякой будет меньше)
        • +1
          Форматтинг (вместе с goimports) удобно засовывать в on-save hook в редакторе.
          • 0
            это несомненно вариант, но в команде побольше он проблематичный :(

            разработчики используют разные ide/редакторы, автоматический провиженинг ide/редакторов может далеко не всем понравиться
            • 0
              О, кстати, а вы не знаете способ, как автопровижнить pre-commit hook в git?
              • +2
                Совсем автоматически никак, только если в гите нет уязвимости через которую это можно сделать)

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


          И было бы здорово ссылку на проект добавить, по аналогии с goreporter и gometalinter.
          • +1
            Спасибо за комментарии и прекрасный инструмент :) Добавил ссылку, зафиксировал версию в контейнере.
            • +1

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


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


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

              • 0
                А за счет чего golangci-lint быстрее gometalinter? Не потому, случайно, что megacheck выключен по умолчанию? На gometalinter именно он у меня обычно в таймауты не влезает.
                • +2
                  оу, в ридми ошибка, megacheck по умолчанию включен, спасибо. И, действительно, megacheck это самый медленный линтер.
                  golangci-lint быстрее за счет того, что:
                  1. Он загружает программу и проверяет ее типы только 1 раз: это 80% работы большинства линтеров.
                  2. Он парсит AST дерево не для каждого линтера: для части линтеров переиспользуется уже распаршенное дерево. В плане делать это для всех.
                  3. Нет создания процессов и использования больше потоков, чем доступно процессоров или указал пользователь. С запущенным gometalinter из-за этого, в частности, за ноутом невозможно работать: он есть все ядра, в не столько, сколько ему указал.
                  4. Умный планировщик линтеров, учитывающий их время выполнения
                  • +1
                    Круто! Теперь понятно, спасибо.

              Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

              Самое читаемое