quasilyte Jun 21 2018 at 00:08

go-critic: самый упрямый статический анализатор для Go

8 min

10K

Go*Open source*Programming*Perfect code*Development Management*

+26

Comments 7

alan008 Jun 21 2018 at 09:08

А что такое "линтер"? Статический анализатор?

quasilyte Jun 21 2018 at 11:05

Есть общепризнанный и лаконичный аналог на Русском, или только полная форма «статический анализатор»? Возможно, «анализатор» был бы уместным компромиссом.

LazyWolf Jun 21 2018 at 19:09

Идея этакого тестового полигона для новых проверок интересная, но особой разницы между добавлением проверки в go-critic и любой другой проект с точки зрения пользователя может и не быть, к тому же не очень понятно как эту идею пользователям «продать».

Ну и отсутствие интеграции с редакторами/gometalinter делает ваш проект заметно менее привлекательным, ведь если уже кто-то заморочился с новой проверкой она должна быть полностью интегрирована в процесс разработки.

Про сам linter — опробовал на своем pet project, весьма полезной показалась rangeExprCopy, а paramTypeCombine скорее вредной. И если вы хотите иметь opinionated проверки, то надо чтоб их было легко отключить, а пока есть только опция -enable, тобишь явный whitelist.

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

quasilyte Jun 21 2018 at 19:22

paramTypeCombine

В ней присутствуют false positive срабатывания для безымянных параметров. Если бы их не было, всё равно был бы вредным?

И если вы хотите иметь opinionated проверки

В недавнем обновлении по умолчанию experimental и opinionated выключены из -enable=all. В идеале eventually придём к более-менее нормальному default набору, но в качестве интеграции куда-то всё равно лучше явный список использовать.

Ну и отсутствие интеграции с редакторами/gometalinter делает ваш проект заметно менее привлекательным, ведь если уже кто-то заморочился с новой проверкой она должна быть полностью интегрирована в процесс разработки.

Уже в golangci-lint добавиться планируем, там есть интеграции в том числе с редакторами. В gometalinter, наверное, тоже можно. Для проекта эти интеграции важны, согласен полностью.

UncleAndy Jul 13 2018 at 12:20

case ast.IsExported(name.ID.String()):

 c.warnCapitalized(name.ID)

Вот это не совсем понял. По логике вещей, предупреждение должно выдаваться если переменная НЕ экспортируемая (т.е. локальная) и Capitalized. А здесь как-то наоборот. Это ошибка или я что-то не понял?

quasilyte Jul 13 2018 at 13:42

Понимаю ваше недоумение.

Но фукнция IsExported(s) проверяет лишь то, что переданная ей строка начинается с заглавной буквы. Метод VisitLocalDef() вызывается только для локальных определений,
поэтому если мы уже внутри и IsExported(s) вернул true, то это локальная переменная, которая начинается с заглавное буквы, что не имеет особого смысла, потому что это не влияет на семантику.

UncleAndy Jul 13 2018 at 13:44

Ясно. Спасибо! Я подозревал нечто подобное. :)

Шикарный инструмент! В ближайшее время проверю весь свой код. :) Может и в разработке правил поучаствую. :)