# PVS\-Studio для Go: краткий обзор новых диагностических правил Вышел PVS\-Studio 8\.0\! Мы добавили поддержку анализа Go проектов и многое другое\. В этой статье мы разберем интересные диагностические правила для Go, которые мы подготовили с выходом новой версии нашего анализатора\. ![1413_New_For_Go_ru/image1.png](https://import.viva64.com/docx/blog/1413_New_For_Go_ru/image1.png) ## Введение В PVS\-Studio 8\.0 официально появилась поддержка анализа Go проектов, а также возможность работы из IDE [GoLand](https://pvs-studio.ru/ru/docs/manual/7190/) и [Visual Studio Code](https://pvs-studio.ru/ru/docs/manual/6646/) \(если вы используете Go\)\. > А ещё плагин для Visual Studio Code был значительно переработан и получил поддержку не только анализа Go проектов, но и всех новых анализаторов PVS\\\-Studio 8\\\.0\\\. Про множество других нововведений вы можете узнать в \[пресс\\\-релизе\]\(https://pvs\-studio\.ru/ru/blog/posts/1406/\)\\\. С выпуском новой версии в анализаторе появилось 40 новых диагностических правил для Go\. В статье разберём несколько наиболее интересных из них, разные кейсы и сравнения с классическими инструментами\. Приятного чтения\! ## Классические ошибки Начнем, пожалуй, не с особенных для языка Go правил, а с чего\-то более универсального и привычного — с ошибок, которые может совершить каждый, независимо от языка, на котором он пишет или общается\. Сразу хочу добавить, что с этим направлением "типичных ошибок разработчиков" у нас особый подход\. Помимо изучения Go проектов, форумов разработчиков и прочих ресурсов, во время разработки диагностических правил для выявления типовых проблем мы использовали наше "секретное оружие"\.\.\. 18\-летний опыт поиска проблем в коде :\) Благодаря тому, что мы имеем за плечами анализаторы для таких языков, как C, C\+\+, C\# и Java, у нас появилась неплохая экспертиза в поиске типовых для всех разработчиков ошибок\. Это позволяет нам поддерживать больше кейсов и сценариев ошибок в коде уже на этапе проектирования нового анализатора\. Связано это с тем, что ошибки по типу опечаток, копипаст и других паттернов, которые свойственны человеку, кажутся простыми\.\.\. Давайте разберем несколько примеров\. ### Обращение по константному индексу внутри цикла Пример: ```cpp for i := 0; i < n; i++ { sum += arr[0] // <= скорее всего должно быть arr[i] } ``` Предупреждение PVS\-Studio: [V8008](https://pvs-studio.ru/ru/docs/warnings/v8008/)\. Suspicious access to a collection element by a constant index inside a loop\. Это диагностическое правило интересно тем, что проблема скрывается не в синтаксисе, а в логической ошибке — они свойственны разработчикам любого уровня\. Индекс синтаксически валиден, и компилятор молчит, но проблема остаётся и подобные ситуации происходят сильно чаще чем может показаться\. Ситуация "ой, забыл поменять `i` на `j`" или "забыл про существование `i` в принципе" уже становится паттерном, а не местечковой проблемой\. Пишем мы об этом, потому что не понаслышке знаем, что так ошибаются\! У нас есть аналоги такого правила в каждом из "классических" анализаторов \(C\# — [V3102](https://pvs-studio.ru/ru/docs/warnings/v3102/), C\+\+ — [V767](https://pvs-studio.ru/ru/docs/warnings/v767/), Java — [V6016](https://pvs-studio.ru/ru/docs/warnings/v6016/)\), и в каждом можно найти примеры ошибок в известных open source проектах, так что недооценивать такие проблемы не стоит\. Примеры ошибок можно найти тут: * [C\# примеры](https://pvs-studio.ru/ru/blog/examples/v3102/) \(\.NET 8, Orleans, PascalABC\.NET и др\.\); * [C\+\+ примеры](https://pvs-studio.ru/ru/blog/examples/v767/) \(Godot Engine, RT\-Thread и др\.\); * [Java примеры](https://pvs-studio.ru/ru/blog/examples/v6016/) \(Apache Solr, Bouncy Castle, Apache Dubbo и др\.\)\. ### Повторная проверка условия \(Recurring check\) Пример: ```cpp if A == B { if A == B { .... } } ``` Предупреждение PVS\-Studio: [V8020](https://pvs-studio.ru/ru/docs/warnings/v8020/)\. Recurring check\. This condition was already verified on a previous line\. Думаю, уже можно понять, что я не врал, когда говорил, что на вид они и правда простые\.\.\. даже слишком\. И это самое страшное\! Создаётся стойкое ощущение, что "вот я никогда\-никогда так не ошибусь, я же не глупый", а потом муха пролетела, баг пробежал, коллега подошёл, и вот у нас уже парочка проверок одного и того же\. Живой пример из проекта [Incus](https://github.com/lxc/incus), который мы разобрали в одной из [статей](https://pvs-studio.ru/ru/blog/posts/go/1371/): ```cpp err = p.Save(pidPath) if err != nil { err2 := p.Stop() if err != nil { // <= похоже, должно быть err2 return fmt.Errorf("...: %s: %s", err, err2) } ... } ``` Предупреждение PVS\-Studio: [V8020](https://pvs-studio.ru/ru/docs/warnings/v8020/) Recurring check\. Thе 'err \!\= nil' condition was already verified on line 407 [proxy\.go 407](https://github.com/lxc/incus/blob/321c40f963e9c1e7b68736fb4941431cccac3d60/internal/server/device/proxy.go#L407) Похоже, что во втором условии стоит проверять переменную `err2` вместо `err`, учитывая, что один раз её уже проверили, и после она не менялась\. Но проблемы тут не заканчиваются, потому что `err2` фигурирует в обработке ошибок: ```cpp return fmt.Errorf("....: %s: %s", err, err2) ``` Казалось бы, что такого специфичного в таком контексте? Но ответ лежит на поверхности: если проблема будет при обработке ошибок, то выявить и исправить их будет труднее\. И ещё немного на эту тему: философия обработки ошибок в Go придерживается простого принципа "ошибка — это значение"\. Обычное значение, которое функция возвращает явно и которое вызывающий код обязан явно обработать\. Такого кода много, и он является [бойлерплейтом](https://ru.wikipedia.org/wiki/%D0%A8%D0%B0%D0%B1%D0%BB%D0%BE%D0%BD%D0%BD%D1%8B%D0%B9_%D0%BA%D0%BE%D0%B4) — это значит, что в нём могут часто ошибаться\! Одно из наших новых диагностических правил было сделано с учётом такой специфики Go: [V8023](https://pvs-studio.ru/ru/docs/warnings/v8023/)\. It is possible that a wrong variable of the 'error' type is checked for 'nil'\. Взглянем на пример: ```cpp func processOrder(orderID string) error { order, err := fetchOrder(orderID) if err != nil { return fmt.Errorf("fetch order: %w", err) } payment, errPay := chargePayment(order) if err != nil { return fmt.Errorf("charge payment: %w", err) } return savePayment(payment) } ``` Возвращаясь к типичным ошибкам разработчиков: копипаста\. Думаю, особо долго смотреть не надо, чтобы заметить использование `err` вместо `errPay` во втором кейсе\. Результат `chargePayment` помещается в другую переменную, но поменять их забыли\. Важно понимать, что в контексте статьи вы без проблем обнаружите большинство ошибок, но если это будет во время код ревью или написания кода, когда мы смотрим не на отдельную функцию, а на сотни и тысячи строк без перерыва, такие вещи могут быть пропущены, и это одна из главных бед с подобными ошибками\. > Эту и другие проблемы во время обработки ошибок в Go мы разобрали в статье "\[Как можно ошибиться при обработке ошибок в Go\]\(https://pvs\-studio\.ru/ru/blog/posts/go/1371/\)"\\\. ### Параметр перезаписывается до использования Пример: ```cpp func Translate(value *T, numericValue int) { numericValue = reader.ReadInt32() // <= исходное значение параметра // нигде не используется ... } ``` Предупреждение PVS\-Studio: [V8038](https://pvs-studio.ru/ru/docs/warnings/v8038/)\. A parameter is always rewritten in the function body before being used\. Достаточно грустная ошибка: значение, переданное в параметр функции, никогда не читается — оно сразу же перезаписывается внутри тела функции\. Формально не ошибка компиляции, так что всё будет работать\. Но почти всегда это признак того, что либо забыли использовать входное значение, либо перепутали переменные при рефакторинге\. ## Базовые инструменты не всегда справляются Всегда круто, когда есть неплохой набор инструментов из коробки и, конечно, различные open source решения, но их может не хватать для более серьёзных задач\. Возьмём в качестве примера самое первое диагностическое правило нашего Go анализатора: [V8001](https://pvs-studio.ru/ru/docs/warnings/v8001/)\. Identical sub\-expressions to the left and to the right of the 'foo' operator\. Взглянем на пример: ```cpp func rgb1(r float32, g float32, b float32) { if r > 1 || g > 1 || r > 1 { .... } } ``` Ошибка заключается в одинаковых подвыражениях в бинарном выражении\. Эта проблема может возникнуть, к примеру, из\-за невнимательного копирования кода\. Такие ошибки хорошо известных и не являются чем\-то особенным, соответственно, классический go vet её находит\. Но не всё так гладко\.\.\. Давайте взглянем на такой код: ```cpp func rgb(r float32, g float32, b float32) { if r > 1 || g > 1 || 1 < r { .... } } ``` Всё, что изменилось, —`1` и `r` поменялись местами\. Но вот незадача, go vet уже не ругается, но проблема осталась, так как подвыражение `r > 1`, по сути, эквивалентно подвыражению `1 < r`\. ### Невозможное приведение типа Разберём ещё один кейс с диагностическим правилом [V8035](https://pvs-studio.ru/ru/docs/warnings/v8035/) — невозможное приведение типа \(impossible type assertion\)\. Взглянем на пример: ```cpp var v interface { Read() Read2() Read3() Read4() Read5() } _ = v.(io.Reader) ``` Суть проста: `v` объявлена как анонимный интерфейс с методом `Read()`, у которого сигнатура не совпадает с `Read([]byte) (int, error)` из `io.Reader/io.ReadCloser` \(в примере `Read()` без аргументов\)\. Значит, ни один конкретный тип не может одновременно реализовывать и анонимный интерфейс `v`, и `io.Reader/io.ReadCloser`\. И, как итог такой реализации, `assertion` гарантированно запаникует в рантайме\. Компилятор такое не ловит, потому что формально это валидный Go\-код и все прекрасно запускается\. И, барабанная дробь\.\.\. Go vet это находит\! Проблема известная и хорошо задокументирована, так что удивляться этому не стоит\. Кстати, PVS\-Studio выдаёт такое сообщение на подобный код: Предупреждение PVS\-Studio: [V8035](https://pvs-studio.ru/ru/docs/warnings/v8035/)\. Type assertion is always false\. Check the 'interface\{Read\(\); Read2\(\); Read3\(\); Read4\(\); Read5\(\)\}' and 'io\.Reader' interfaces, as they contain methods with incompatible signatures\. А вот другой случай: ```cpp var v interface { Read() } switch v.(type) { case io.ReadCloser: fmt.Println(1) default: fmt.Println(2) } ``` Предупреждение PVS\-Studio: [V8035](https://pvs-studio.ru/ru/docs/warnings/v8035/)\. Type assertion is always false\. Check the 'interface\{Read\(\)\}' and 'io\.ReadCloser' interfaces, as they contain methods with incompatible signatures\. Фактически это абсолютно тот же самый невозможный `assertion`\. Всё, что изменилось, это запись через `case` в `switch` и через `v.(type)`, а не через `v.(io.ReadCloser)`\. И тут я должен подвести к тому, что go vet это не найдёт\.\.\. но нет\! Он это находит, но история не о нём\. Один из популярных Open Source анализаторов тут не выявит проблемы, но найдёт её в первом случае\. Ситуация с отсутствием срабатывания называется [false negative](https://pvs-studio.ru/ru/blog/terms/6460/)\. И возникает дилемма: искать инструмент, который покроет все кейсы, и, возможно, потратить время на перебор; или же присмотреться к PVS\-Studio, изначально заточенному на более глубокий анализ и с гарантированной поддержкой на многие годы\. > Если вы сомневаетесь в этих словах, то всегда можете \[попробовать наш инструмент\]\(https://pvs\-studio\.ru/ru/pvs\-studio/try\-free/\) и сравнить самостоятельно\\\. Но хватит синтетического кода\. Будет ли что\-то реальное? ### Что\-то реальное Помните "самую первую диагностику Go анализатора", которую я разобрал в начале раздела? Вот пример срабатывания из реального проекта [Nuclei:](https://github.com/projectdiscovery/nuclei) ```cpp func NewEntityParser(dir string) (*EntityParser, error) { cfg := &packages.Config{ Mode: packages.NeedName | packages.NeedFiles | packages.NeedImports | packages.NeedTypes | packages.NeedSyntax | packages.NeedTypes | packages.NeedModule | packages.NeedTypesInfo, Tests: false, Dir: dir, ParseFile: func(....) (*ast.File, error) { return parser.ParseFile(fset, filename, src, parser.ParseComments) }, } } ``` Предупреждение PVS\-Studio: [V8001](https://pvs-studio.ru/ru/docs/warnings/v8001/) There are identical sub\-expressions 'packages\.NeedTypes' to the left and to the right of the '\|' operator\. [parser\.go 33](https://github.com/projectdiscovery/nuclei/blob/ee8287a7b756d07e09e22b8945cddeb58e727a22/pkg/js/devtools/tsgen/parser.go#L33) Если хорошенько присмотреться, можно заметить, что здесь лишний раз повторяется флаг `packages.NeedTypes`\. Возможно, ошибка произошла из\-за автоподстановки кода или опечатки\. Скорее всего, тут должен быть флаг `packages.NeedTypesSizes`: ```cpp const ( .... // NeedTypes adds Types, Fset, and IllTyped. NeedTypes // NeedSyntax adds Syntax and Fset. NeedSyntax // NeedTypesInfo adds TypesInfo and Fset. NeedTypesInfo // NeedTypesSizes adds TypesSizes. NeedTypesSizes ... ) ``` Казалось бы, ошибка примитивная, но с помощью стандартного go vet найти её не получится\. Но open source инструмент, о котором я говорил в предыдущем кейсе, её находит, и я вновь напоминаю о проблеме "где\-то go vet находит, а другой инструменты нет" и наоборот\. Эту и другие ошибки, которые не может найти стандартный go vet, мы разобрали в статье "[Go vet не поможет\. Статический анализ Golang проектов с помощью PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/go/1342/)"\. ## Go\-специфичные Конечно, в Go можно ошибиться не только "по универсальному"\. Язык богат своими особенностями и подводными камнями, о которых могут не знать даже те, кто давно с ним работает \(что уж говорить о тех, кто в него переходит с других языков\)\. На этот случай у нас всегда есть подробная документация, в которой можно найти объяснения, из\-за чего произошла ошибка\. А ещё примеры, как можно ошибиться и что стоит сделать, чтобы это исправить :\) Давайте рассмотрим пару примеров диагностических правил\. ### recover\(\) внутри необёрнутой в defer анонимной функции В Go `recover` восстанавливает выполнение только если вызван внутри отложенной \(`defer`\) функции\. Если же анонимная функция с `recover` вызывается сразу `(func() { ... }())`, то к моменту потенциальной паники этот вызов уже давно завершился — `recover` физически не может её перехватить\. Таким образом, при возникновении паники невозможно восстановить исполнение горутины\. Вот пример: ```cpp func foo() { func() { if r := recover(); r != nil { fmt.Println("Recovered", r) } }() // <= recover сработал и сразу завершился q, r := bits.Div64(hi, lo, y) // паника здесь recover уже не поймает } ``` После определения анонимной функции и её вызова происходит вызов функции `bits.Div64`\. Эта функция может вызвать панику, если `y` окажется равной `0`\. Если паника дойдёт до верха стека, то программа аварийно завершится\. И, конечно, документация покажет, как это исправить: нужно сделать вызов анонимной функции отложенным с помощью ключевого слова `defer`: ```cpp func foo() { defer func() { if r := recover(); r != nil { fmt.Println("Recovered", r) } }() q, r := bits.Div64(hi, lo, y) } ``` Подробнее можно узнать в [документации](https://pvs-studio.ru/ru/docs/warnings/v8007/)\. ### Ну не могли они так ошибиться А вот эта ошибка настолько интересная, что мы написали статью "[Ошибка, которую никто никогда не совершал\. Ведь так, да?](https://pvs-studio.ru/ru/blog/posts/go/1393/)"\. Так как в статье она достаточно подробно описывается и приводятся примеры из реальных проектов, я лишь приведу короткий пример: ```cpp func SimpleExample(arr []int) { _ = arr[len(arr)] } ``` Индексы коллекций всегда начинаются с 0\. Соответственно, для слайса длины `n` допустимые индексы находятся в диапазоне от 0 до `n – 1`\. И `len(arr)` вернёт `n`\. Поэтому обращение к `arr[len(arr)]` всегда приведёт к панике программы\. Интересно тут то, что при её разработке возникали мысли, что она "надуманная" и "да никто так не ошибался", потому что это поймает любой тест, но отнюдь — результаты были очень интересные\! Посмотреть на них можно в [статье](https://pvs-studio.ru/ru/blog/posts/go/1393/)\. ### Путаница XOR \(^\) и возведения в степень Ещё одна интересная ошибка, которая достаточно специфична для Go\. Суть в том, что в Go `^` не имеет отношения к степени \(в отличие от Lua/VB\.NET/R\), и это ловушка для разработчиков, переключающихся между языками\. Этим она особенно интересна \(вспоминаем проблему универсальных ошибок из начала статьи и получаем ошибочное комбо\)\. Бинарные выражения, в которых задумывалось возведение в степень, становятся ошибочными, если выбор падает на `^`\. Этот оператор на самом деле осуществляет побитовое исключающее "ИЛИ"\. Пример до боли простой: ```cpp x := 2 ^ 16 ``` На самом деле здесь будет значение 18 вместо ожидаемого 2 в степени 16 \(65536\)\. Для того, чтобы получить степень числа 2, можно использовать побитовый сдвиг влево, как нам подсказывает [документация](https://pvs-studio.ru/ru/docs/warnings/v8015/): ```cpp x := 1 << 16 ``` ## Что дальше? Если вы с нами знакомы, то, скорее всего, слышали, что PVS\-Studio не только выявляет проблемы с качеством кода, но является ещё и [SAST \(Static Application Security Testing\) решением](https://pvs-studio.ru/ru/pvs-studio/security/)\. Это говорит о том, что наш анализатор способен находить потенциальные уязвимости\. Наш новый анализатор для Go пока включает в себя только базовый набор диагностических правил, покрывающих частые проблемы с качеством кода\. Но это только начало\! Можете считать это анонсом, потому что мы уже начали работу по охвату SAST\-направления для Go анализатора, ждите новости в [нашем блоге](https://pvs-studio.ru/ru/blog/posts/) :\) Ну а чтобы не быть голословным, представляю вам одно из первых диагностических правил раздела проблем безопасности кода\. ### Trojan Source Взгляните на фрагмент: ```cpp isAuthorized := false /* verify before transfer */ if isAuthorized { TransferFunds(account, amount) /* end verify */ } ``` Как, по\-вашему, он будет выглядеть для компилятора \(без всех условностей и низкоуровневого представления\)? Если вы посчитали, что так: ```cpp isAuthorized := false if isAuthorized { TransferFunds(account, amount) ``` То\.\.\. ну вы правы\.\.\. почти\.\.\. Выглядит так, будто перевод средств действительно защищён проверкой `isAuthorized`\. Но проблемы скрывается там, где мы не видим, потому что компилятор увидит вот такое: ```cpp isAuthorized := false TransferFunds(account, amount) ``` Уже не так славно, не правда ли? Проверка `isAuthorized` фактически не выполняется никогда: `if` вместе с `{...}` оказались внутри "закомментированного" — по мнению ревьюера — текста, а вызов `TransferFunds` выполняется безусловно\. Дело тут в том, что в коде есть специальные символы, и они могут не отображаться\. Они способны изменять его видимое представление в среде разработки\. Комбинации таких символов могут привести к тому, что человек и компилятор будут интерпретировать код по\-разному\. И наш злополучный код на самом деле такой: ```cpp isAuthorized := false /*[RLO] } [LRI] if isAuthorized[PDI] [LRI] verify before transfer */ TransferFunds(account, amount) /* end verify [RLO]{ [LRI]*/ ``` Кратко разберём, что тут делают эти символы: * `[LRI] if isAuthorized[PDI]` — фрагмент между `LRI` и `PDI` изолируется и отображается слева направо как единый блок: `if isAuthorized`\. * `[LRI] verify before transfer */` — до конца строки, тоже изолированный блок: `verify before transfer */`\. * `[RLO]` разворачивает всё, что после него, справа налево, причём каждый из уже полученных изолированных блоков воспринимается как один неделимый "символ"\. В конечном итоге получаем такой порядок: ```cpp 'verify before transfer */', 'if isAuthorized', {пробел}, '{', {пробел} ``` И, о чудо\! Парой лёгких движений закрывающая скобочка визуально превращается в открывающую\. Конечно же, это проблема, и PVS\-Studio скажет нам следующее: Предупреждение PVS\-Studio: [V5901](https://pvs-studio.ru/ru/docs/warnings/v5901/)\. OWASP\. Code contains invisible characters that may alter its logic\. Consider enabling the display of invisible characters in the code editor\. Это диагностика из [стандарта OWASP](https://pvs-studio.ru/ru/pvs-studio/sast/owasptopten/), и аналоги такой проблемы можно встретить в наших других анализаторах: * V5801 — JS\\TS; * V5340 — Java; * V5629 — C\#; * V1076 — C\+\+\. А подробнее это диагностическое правило мы разобрали в [документации](https://pvs-studio.ru/ru/docs/warnings/v5901/)\. ## Конец? Нет, начало\! Новый анализатор для Go проектов вышел, но это только начало его истории\! 40 диагностических правил, появившихся с выходом PVS\-Studio 8\.0, являются стартом истории, и ваше участие в ней будет как никогда полезно\. Если вы Go разработчик, предлагаем вам [попробовать наш инструмент бесплатно](https://pvs-studio.ru/ru/pvs-studio/try-free/)\. Если у вас появятся какие\-то идеи для новых диагностик или пожелания по улучшению уже существующих, то можете [написать нам](https://pvs-studio.ru/ru/about-feedback/) через форму обратной связи\! Спасибо, что прочитали\. Берегите себя и свой код\!