В этой статье я расскажу о новой библиотеке (и утилите) статического анализа go-ruleguard, которая адаптирует gogrep для использования внутри линтеров.


Отличительная особенность: правила статического анализа вы описываете на особом Go-подобном DSL, который на старте ruleguard превращается в набор диагностик. Возможно, это один из самых легко конфигурируемых инструментов для реализации кастомных инспекций для Go.


В качестве бонуса, мы поговорим об go/analysis и его предшественниках.


Расширяемость статического анализа


Для Go существует множество линтеров, некоторые из которых можно расширять. Обычно для расширения линтера вам требуется написать Go код, использующий специальное API линтера.


Есть два основных пути: Go plugins и монолит. Монолит подразумевает, что все проверки (в том числе ваши личные) доступны на этапе компиляции.


revive требует включения новых проверок в своё ядро для расширения. go-critic вдобавок к этому умеет в плагины, что позволяет собирать расширения независимо от основного кода. Оба эти подхода подразумевают, что вы реализуете манипуляции над go/ast и go/types на Go, используя API линтера. Даже простые проверки требуют много кода.


go/analysis призван упростить картину тем, что "фреймворк" у линтеров становится практически идентичным, но он не решает проблему сложности технической реализации самих диагностик.


Отступление о `loader` и `go/packages`


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


Первым шагом в упрощении этого пайплайна стал пакет go/loader, который позволят "загрузить" всё нужное через пару вызовов. Всё было почти хорошо, а потом он стал deprecated в пользу go/packages. go/packages имеет немного улучшенное API и, в теории, хорошо работает с модулями.


Теперь для написания анализаторов лучше всего не использовать ничего из выше перечисленного напрямую, потому что go/analysis дал go/packages то, чего не было ни у одного предыдущего решения — структуру для вашей программы. Теперь мы можем использовать диктуемую go/analysis парадигму и переиспользовать работу анализаторов более эффективно. У этой парадигмы есть спорные моменты, например, go/analysis хорошо подходит для анализа на уровне одного пакета и его зависимостей, но сделать на нём глобальный анализ без хитрых инженерных ухищрений будет не просто.


go/analysis также упрощает тестирование анализаторов.




Что же такое ruleguard?



go-ruleguard — это утилита статического анализа, которая по умолчанию не включает в себя ни единой проверки.


Правила ruleguard подгружаются на старте, из специального gorules файла, декларативно описывающего паттерны кода, на которые стоит выдавать предупреждения. Этот файл может свободно редактироваться пользователями ruleguard.


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


Управляющая программа ruleguard выглядит так:


package main

import (
    "github.com/quasilyte/go-ruleguard/analyzer"
    "golang.org/x/tools/go/analysis/singlechecker"
)

func main() {
    singlechecker.Main(analyzer.Analyzer)
}

При этом analyzer реализован через пакет ruleguard, который и нужно использовать в случае, если вы хотите использовать его как библиотеку.


ruleguard VS revive


Возьмём простой, но реальный пример: предположим, мы хотите избегать вызовов runtime.GC() в наших программах. В revive для этого уже есть отдельная диагностика, она называется "call-to-gc".


Реализация call-to-gc (70 строк на Эльфийском)


package rule

import (
    "go/ast"

    "github.com/mgechev/revive/lint"
)

// CallToGCRule lints calls to the garbage collector.
type CallToGCRule struct{}

// Apply applies the rule to given file.
func (r *CallToGCRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
    var failures []lint.Failure
    onFailure := func(failure lint.Failure) {
        failures = append(failures, failure)
    }

    var gcTriggeringFunctions = map[string]map[string]bool{
        "runtime": map[string]bool{"GC": true},
    }

    w := lintCallToGC{onFailure, gcTriggeringFunctions}
    ast.Walk(w, file.AST)

    return failures
}

// Name returns the rule name.
func (r *CallToGCRule) Name() string {
    return "call-to-gc"
}

type lintCallToGC struct {
    onFailure             func(lint.Failure)
    gcTriggeringFunctions map[string]map[string]bool
}

func (w lintCallToGC) Visit(node ast.Node) ast.Visitor {
    ce, ok := node.(*ast.CallExpr)
    if !ok {
        return w // nothing to do, the node is not a call
    }

    fc, ok := ce.Fun.(*ast.SelectorExpr)
    if !ok {
        return nil // nothing to do, the call is not of the form pkg.func(...)
    }

    id, ok := fc.X.(*ast.Ident)

    if !ok {
        return nil // in case X is not an id (it should be!)
    }

    fn := fc.Sel.Name
    pkg := id.Name
    if !w.gcTriggeringFunctions[pkg][fn] {
        return nil // it isn't a call to a GC triggering function
    }

    w.onFailure(lint.Failure{
        Confidence: 1,
        Node:       node,
        Category:   "bad practice",
        Failure:    "explicit call to the garbage collector",
    })

    return w
}



А теперь сравните с тем, как это делается в go-ruleguard:


package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func callToGC(m fluent.Matcher) {
    m.Match(`runtime.GC()`).Report(`explicit call to the garbage collector`)
}

Ничего лишнего, только то, что действительно важно — runtime.GC и сообщение, которое нужно выдавать в случае срабатывания правила.


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


Quick start


В go-critic есть диагностика rangeExprCopy, которая находит в коде потенциально неожиданные копирования массивов.


Вот этот код итерируется по копии массива:


var xs [2048]byte
for _, x := range xs { // Copies 2048 bytes
    // Loop body.
}

Исправление этой проблемы заключается в добавлении одного символа:


  var xs [2048]byte
- for _, x := range xs {  // Copies 2048 bytes
+ for _, x := range &xs { // No copy
    // Loop body.
  }

Скорее всего, вам это копирование не нужно, а производительность исправленного варианта всегда лучше. Можно ждать, пока Go компилятор станет лучше, а можно детектировать такие места в коде и поправить их уже сегодня с помощью того же go-critic.


Эту диагностику можно реализовать на языке gorules (файл rules.go):


package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func _(m fluent.Matcher) {
    m.Match(`for $_, $_ := range $x { $*_ }`,
            `for $_, $_ = range $x { $*_ }`).
            Where(m["x"].Addressable && m["x"].Type.Size >= 128).
            Report(`$x copy can be avoided with &$x`).
            At(m["x"]).
            Suggest(`&$x`)
}

Правило находит все циклы for-range, где используются обе итерируемые переменные (именно этот случай ведёт к копированию). Итерируемое выражение $x должно быть addressable и его размер должен быть выше выбранного порога в байтах.


Report() определяет сообщение, которое нужно выдавать пользователю, а Suggest() описывает quickfix шаблон, который может использоваться в вашем редакторе через gopls (LSP), а также интерактивно, если ruleguard вызван с аргументом -fix (мы ещё к этому вернёмся). At() привязывает предупреждение и quickfix к конкретной части шаблона. Нам это необходимо, чтобы заменить $x на &$x, а не переписать весь цикл.


И Report(), и Suggest(), принимают строку, в которую можно интерполировать захваченные шаблоном из Match() выражения. Предопределённая переменная $$ означает "весь захваченный фрагмент" (как $0 в регулярных выражениях).


Создадим файл rangecopy.go:


package example

// sizeof(builtins[...]) = 240 on x86-64
var builtins = [...]string{
    "append", "cap", "close", "complex", "copy",
    "delete", "imag", "len", "make", "new", "panic",
    "print", "println", "real", "recover",
}

func builtinID(name string) int {
    for i, s := range builtins {
        if s == name {
            return i
        }
    }
    return -1
}

Теперь мы можем запустить ruleguard:


$ ruleguard -rules rules.go -fix rangecopy.go
rangecopy.go:12:20: builtins copy can be avoided with &builtins

Если после этого мы посмотрим в rangecopy.go, то увидим исправленный вариант, потому что ruleguard был вызван с параметром -fix.


Простейшие правила можно отлаживать и без создания gorules файла:


$ ruleguard -c 1 -e 'm.Match(`return -1`)' rangecopy.go
rangecopy.go:17:2: return -1
16      }
17      return -1
18  }

Благодаря использованию go/analysis/singlechecker у нас есть опция -c, которая позволяет выводить указанное строк контекста вместе с самим предупреждением. Управление этим параметром немного контринтуитивно: значение по умолчанию равно -c=-1, что означает "без контекста", а -c=0 будет выводить одну строку контекста (ту, на которую указывает диагностика).


Вот ещё несколько интересных возможностей gorules:


  • Шаблоны типов, которые позволяют задавать ожидаемые типы. Например, выражение map[$t]$t описывает все мапы, у которых тип значения совпадает с типом ключа, а *[$len]$elem захватываем все указатели на массивы.
  • Внутри одной функции может быть несколько правил,
    а сами функции стоит называть группами правил.
  • Правила в группе применяются одно за другим, в порядке их определения. Первое сработавшее правило отменяет сопоставление с оставшимися правилами. Это важно не столько для оптимизации, сколько для специализации правил для конкретных случаев. Примером, где это полезно, является правило переписывания $x=$x+$y в $x+=$y, для случая с $y=1 вы хотите предлагать $x++, а не $x+=1.

Больше информации об используемом DSL можно найти в docs/gorules.md.


Ещё больше примеров


package gorules

import "github.com/quasilyte/go-ruleguard/dsl/fluent"

func exampleGroup(m fluent.Matcher) {
        // Находим потенциально некорректные использования json.Decoder.
        // См. http://golang.org/issue/36225
        m.Match(`json.NewDecoder($_).Decode($_)`).
                Report(`this json.Decoder usage is erroneous`)

        // Делаем умный unconvert, предлагая убирать лишние преобразования.
        m.Match(`time.Duration($x) * time.Second`).
                Where(m["x"].Const).
                Suggest(`$x * time.Second`)

        // Предлагаем заменить fmt.Sprint() на вызов метода String(),
        // если у $x таковой имеется.
        m.Match(`fmt.Sprint($x)`).
                Where(m["x"].Type.Implements(`fmt.Stringer`)).
                Suggest(`$x.String()`)

        // Упрощаем логические выражения.
        m.Match(`!($x != $y)`).Suggest(`$x == $y`)
        m.Match(`!($x == $y)`).Suggest(`$x != $y`)
}

Если для правила нет вызова Report(), то будет использоваться сообщение, выводимое из Suggest(). Это позволяет в некоторых случаях избежать дублирования.


Фильтры типов и подвыражений могут проверять различные свойства. Например, полезными являются свойства Pure и Const:


  • Var.Pure означает, что выражение не имеет побочных эффектов.
  • Var.Const означает, что выражение может быть использовано в константном контексте (например, размерность массива).

Для package-qualified имён в Where() условиях нужно использовать метод Import(). Для удобства, все стандартные пакеты импортированы за вас, поэтому в примере выше нам не нужно делать дополнительных импортов.


go/analysis quickfix actions


Поддержку quickfix за нас реализует go/analysis.


В модели go/analysis, анализатор генерирует диагностики и факты. Диагностики отправляются пользователям, а факты предназначены для использования другими анализаторами.


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


Официальное описание доступно в документе go/analysis/doc/suggested_fixes.md.


Заключение



Попробуйте ruleguard на своих проектах, а в случае, если вы нашли баг или хотите попросить новую фичу, откройте issue.


Если вам всё ещё сложно придумать применение ruleguard, вот примеры:


  • Реализация своих собственных диагностик для Go.
  • Автоматическая модернизация или рефакторинг кода с помощью -fix.
  • Сбор статистики по коду с обработкой -json результата анализатора.

Планы по развитию ruleguard на ближайшее будущее:



Полезные ссылки и ресурсы


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


  1. astec
    27.12.2019 04:50

    Очень круто. Спасибо.


  1. host13
    27.12.2019 09:44

    Плагин для VS Code планируется?


    1. slisli
      27.12.2019 14:17

      Я полагаю после внедрения в go-critic будет достаточно использовать golangci-lint с VS Code.


    1. quasilyte Автор
      27.12.2019 14:23

      К gopls попробую прикрутить, в нём очень хорошая поддержка go/analysis. А сам gopls в VS Code есть.


  1. powerman
    27.12.2019 13:51

    - for _, x := range xs {  // Copies 2048 bytes
    + for _, x := range &xs { // No copy

    Не совсем ясно, что имелось в виду, потому что данный код не компилируется:


    ./prog.go:9:14: cannot range over &xs (type *[]byte)


    1. quasilyte Автор
      27.12.2019 14:23

      Там копирование не элемента, а массива. Это для многих неожиданное поведение, поэтому и привел пример такой инспекции. Иногда в циклах поиска по короткому словарю (иногда это массив, а не слайс), делается лишнее копирование, в итоге вместо оптимизации получаем деградацию.


      Явно в спеке нигде это не указывается, по-моему, но это можно вывести из того, что аргументы цикла "присваиваются" как обычные переменные. А массивы копируются при присваивании. Вот пример, где всё было бы очевидно:


      var arr [10]int
      
      _iter := arr // Вот тут весь массив копируется
      for i := 0; i < len(_iter); i++ {
        el := _iter[i]
      }

      Итого у вас будет 2N копий. Сначала весь массив во временную переменную, а потом ещё N для каждой итерации. Проверьте лично или гляньте -S вывод компилятора, если нужны доказательства.


      Если же там указатель, вы присвоите в _iter указатель, скопируя 8 байтов, что не страшно. Go умеет делать range по указателю на массив, поэтому всё будет работать без последствий.


      1. powerman
        27.12.2019 15:03

        А зачем вообще range нужно копировать исходный массив?


        Ну т.е. речь о том, что компилятор копирует аргумент range в отдельную переменную, хотя в этом нет никакой необходимости. И чинить это в компиляторе никто не планирует (и чтобы не усложнять его, и потому что range по большим массивам это не такая уж частая ситуация), поэтому предлагается "оптимизировать" это вручную разработчикам.


        1. quasilyte Автор
          27.12.2019 16:22

          И чинить это в компиляторе никто не планирует (и чтобы не усложнять его, и потому что range по большим массивам это не такая уж частая ситуация), поэтому предлагается "оптимизировать" это вручную разработчикам.

          В общем, вы правы, но тут "чинить" не очень корректное слово. Это не баг, а поведение по спеке. Плюс, как вы уже тоже отметили, это редкий кейс, но если он появляется в программе, есть шанс, что это была ошибка программиста, а фикс очень простой...


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


          Если копию не делать, конкуррентные программы могут измениться. Можно, конечно, делать анализ, не доступен ли массив для такого доступа, но чаще всего массивы для оптимизаций — глобальны, а Go слабовато пока такое оптимизирует и анализирует. И не факт, что когда-либо gc (компилятор от Google) будет такое делать, потому что есть требование держать время компиляции под контролем, а флаги -O они не хотят.


          1. powerman
            27.12.2019 21:50

            Это не баг, а поведение по спеке. … Если "починить", сломается код, который ожидает, что массив копируется. Эти программы корректные по спецификации языка.

            Я не вижу в спеке информации о том, что for/range копирует свой аргумент. Можете процитировать, где это сказано?


            В целом, если программа запускает горутину до for, то она и так уже некорректная, т.к. никто не гарантирует что выполнится раньше — код в горутине или копирование массива в range. А если программа запускает горутину внутри for… ну, да, она вполне может сломаться. Но, будем откровенны, не факт, что в мире сейчас в принципе есть программы, которые делают range по массиву и запускают горутину внутри for которая рассчитывает, что for не увидит вносимые горутиной изменения в этот массив. А вот программы, производительность которых немного улучшится от этого изменения — скорее всего есть, и их немало. В общем, если спека явно это поведение не декларирует, то я не вижу смысла его сохранять. Оно, мягко говоря, неочевидное, и скорее вредное, нежели полезное.


            1. quasilyte Автор
              27.12.2019 22:49

              Я не вижу в спеке информации о том, что for/range копирует свой аргумент. Можете процитировать, где это сказано?

              В спеке явно не написано, но вот из этого выводится:


              The range expression x is evaluated once before beginning the loop, with one exception: if at most one iteration variable is present and len(x) is constant, the range expression is not evaluated.

              Если x — это массив, то его "evaluation" в контексте выражений — это копирование.


              А вот почему нет копирования, когда используется только 1 range value, это отдельно описано:


              If at most one iteration variable is present, the range loop produces iteration values from 0 up to len(a)-1 and does not index into the array or slice itself.

              Прочитайте внимательно For statements with range clause, взял оттуда.


              И чинить это в компиляторе никто не планирует (и чтобы не усложнять его, и потому что range по большим массивам это не такая уж частая ситуация), поэтому предлагается "оптимизировать" это вручную разработчикам.
              А вот программы, производительность которых немного улучшится от этого изменения — скорее всего есть, и их немало.

              Вы сами предположили, что это редкий кейс. Я могу это подтвердить, он реально редкий и проверял я на Go корпусе при тестировании go-critic, где эта диагностика есть. Имхо, это такой редкий зверь, что даже нашего диалога не стоит.


              Я не буду говорить, хорошо это или плохо, что в gc компиляторе неохотно делают оптимизации, которые:


              a) срабатывают очень редко и легко правятся в коде (явно, в 1 символ)
              b) когда цикл работает 1 раз в каком-нибудь init, то разницы нет, есть эта оптимизация или нет


              Такая вот политика. Она может нравиться или не нравиться, нам об этом спорить толку мало.


              Насколько мне известно, там ещё 1 нюанс есть: если массив маленький, по копии пройти может быть быстрее, потому что тогда у нас будет a[i] для доступа к элементу, который на каждой итерации генерируется для range value. А если это указатель, то будет (*a)[i] (массив — это указатель на данные под капотом, а указатель на массив будет указателем на указатель). Мне больше нравится подход, где мы говорим программисту: "здесь может быть что-то подозрительное, разберись, пожалуйста" (как это делает go-critic).


              1. powerman
                27.12.2019 22:55

                Если x — это массив, то его "evaluation" в контексте выражений — это копирование.

                Эмм… Это довольно сомнительное трактование. Например, выражение a[1] при "evaluation" обходится без копирования массива a. Равно как и len(a). (Наверняка есть и другие примеры, эти два просто сходу в голову пришли.) В общем, я допускаю, что Вы можете быть правы, а вышеупомянутые примеры оговорены в спеке как частные случаи (аналогично упомянутому Вами примеру с range), но это как минимум не очевидно.


                1. quasilyte Автор
                  27.12.2019 23:05

                  (Про то, как трактовать тот параграф в спеке можно порассуждать, но ценность не очень высокая. Я на 100% не могу утверждать, но это та интерпретация, которую я видел в нескольких обсуждениях.)


                  Если отбросить спеку в сторону, так как не понятно, как её правильно читать, то a[i] имеет семантику, как в C. У нас массив — это просто указатель и длина только на этапе компиляции хранится. Чтобы взять элемент, копирования не нужно.


                  А в случае цикла иногда копия лучше, посмотрите мой update, пожалуйста:


                  Насколько мне известно, там ещё 1 нюанс есть: если массив маленький, по копии пройти может быть быстрее, потому что тогда у нас будет a[i] для доступа к элементу, который на каждой итерации генерируется для range value. А если это указатель, то будет (*a)[i] (массив — это указатель на данные под капотом, а указатель на массив будет указателем на указатель).

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


                  При этом x должен быть addressable, чтобы от него можно было взять адрес. Если там вызов функции, то уже не прокатит.