[Перевод] Pylint: детальная проверка работы анализатора кода30.04.2020 13:03

Когда Люк работал с Flake8 и одновременно присматривался к Pylint, у него сложилось впечатление, что 95% ошибок, выдаваемых Pylint, были ложными. У других разработчиков был иной опыт взаимодействия с этими анализаторами, поэтому Люк решил детально разобраться в ситуации и изучить его работу на 11 тыс. строк своего кода. Кроме того, он оценил пользу от Pylint, рассматривая его как дополнение к Flake8.

Люк (Luke Plant) — один из британских разработчиков, на чью статью с разбором популярных анализаторов кода мы недавно наткнулись. Линтеры изучают код, помогают найти ошибки и сделать его стилистически согласованным со стандартами и кодом, который пишут разработчики в вашей команде. Самые распространенные из них — Pylint и Flake8. Мы в Leader-ID их тоже используем, потому с радостью сделали перевод его статьи. Надеемся, что она обогатит и ваш опыт работы с этими инструментами.

Начальные установки и тестовая база

Для данного эксперимента я взял часть кода из одного своего проекта и запустил Pylint с базовыми настройками. Затем попытался проанализировать результат: какие предупреждения оказались полезными, а какие ложными.

Небольшая справка о проекте, из которого был взят код:

Обычное приложение, написанное на Django (т.е. внутри все тот же Python). У Django есть свои особенности, и, как фреймворк, он имеет свои ограничения, но позволяет писать нормальный код на Python. Некоторые его недостатки как фреймворка также есть и у других библиотек, использующих шаблоны (callback-функций или шаблоны проектирования Template Method).
Состоит из 22 000 строк кода. Через Pylint прошло примерно 11 000 строк (9 000, если отбросить пропуски). Эта часть проекта состояла преимущественно из кода views и тестового кода.
Для анализа кода этого проекта я уже использовал Flake8, обработав все полученные ошибки. Смысл этого эксперимента состоял в том, чтобы оценить пользу от Pylint, как прибавку к Flake8.
У проекта хорошее тестовое покрытие кода, но так как я его единственный автор, у меня не было возможности воспользоваться коллегиальным рецензированием.

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

Итак, Pylint выдал 1650 претензий к моему коду.

Ниже я сгруппировал все замечания анализатора по типам и дал к ним свои комментарии. Если вам нужно подробное описание этих сообщений, загляните в соответствующий раздел списка функций Pylint.

Баги

Pylint нашел ровно один баг в моем коде. Багом я считаю ошибку, которая возникает или может потенциально возникнуть во время работы программы. В этом кейсе я использовал исключения — broad-except.То есть except Exception, а не просто except, который Flake8 отлавливает. Это повлекло бы за собой неправильное поведение во время выполнения при наличии некоторых исключений. Если бы эта ошибка когда-либо выскочила во время выполнения (не факт, что выскочит), то неверное поведение кода не вызвало бы серьезных последствий, хотя…

Итого: 1

Полезное

В придачу к той ошибке Pylint нашел еще несколько, которые я отнес к категории «полезное». Код от них не падал, но могли возникнуть проблемы при рефакторинге, да и в принципе к ошибкам в будущем при расширении списка фичей и поддержке кода.

Семь из них были too-many-locals / too-many-branches / too-many-local-variables. Они относились к трем частям моего кода, которые были плохо структурированы. Над структурой хорошо бы было еще подумать, и я уверен, что мог бы сделать лучше.

Остальные ошибки:

unused-argument × 3 — один из них действительно был косяком, и код выполнялся правильно случайно. Другие два ненужных и неиспользуемых аргумента привели бы к проблемам в будущем, если бы я их использовал.
redefined-builtin × 2
dangerous-default-value × 2 — не баги, ибо я никогда не использовал дефолтные значения, но хорошо бы это исправить на всякий случай.
stop-iteration-return × 1 — вот тут я узнал для себя что-то новое, никогда бы сам не нашел.
no-self-argument × 1

Итого: 16

Косметические правки

На эти вещи я бы обращал меньше внимания. Они либо незначительные, либо маловероятные. С другой стороны, их исправление лишним не будет. Часть из них — спорные стилистические. О некоторых похожих косяках я рассказывал в других разделах, но те, что будут тут перечислены, подходят и под этот контекст. Используя регулярно Pylint, я бы поправил эти «недочеты», но в большинстве случаев не стал бы беспокоиться о них.

invalid-name × 192

Это были в основном имена переменных из одной буквы. Использовал в тех контекстах, где это было не страшно, например:

или

Многие были в коде тестов:

len-as-condition × 20
useless-object-inheritance × 16 (наследие Python 2)
no-else-return × 11
no-else-raise × 1
bad-continuation × 6
redefined-builtin × 4
inconsistent-return-statements × 1
consider-using-set-comprehension × 1
chained-comparison × 1

ИТОГО: 252

Бесполезное

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

too-many-ancestors × 76

Были в тестовом коде, где я использовал много классов примесей, чтобы поставить утилиты или заглушки.

unused-variable × 43

Встречалось почти все время в тестовом коде, где я разбивал запись:

… и не использовал ни один из элементов. Есть несколько способов сделать так, чтобы Pylint тут не сообщал об ошибках (например, дать названия unused). Но если оставить в том виде, в котором я написал, он будет читабельным, и люди (в том числе я) смогут его понимать и поддерживать.

invalid-name × 26

Это были случаи, когда я присваивал подходящие названия в контексте, но те, которые не соответствовали стандартам именования Pylint. Например db (это общепринятая аббревиатура для database) и некоторые другие нестандартные названия, которые, на мой взгляд, были более понятны. Но вы можете не согласиться со мной.

redefined-outer-name × 16

Иногда имя переменной указано правильно как для внутреннего, так и для внешнего контекста. И вам никогда не придется использовать внешнее имя из внутреннего контекста.

too-few-public-methods × 14

Примеры включают классы с данными, созданными при помощи attrs, где нет публичных методов, и класс, который реализует интерфейс словаря, но который нужен, чтобы обеспечить корректную работу метода __getitem__

no-self-use × 12

Возникали в коде тестов, где я намеренно добавлял методы к базовому классу без параметра self, потому что таким образом их было удобнее импортировать и сделать доступными для тестового кейса. Некоторые из них даже обернули в отдельные функции.

attribute-defined-outside-init × 10

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

too-many-locals × 6, too-many-return-statements × 6, too-many-branches × 2, too-many-statements × 2

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

arguments-differ × 6

Возникает в основном из-за использования *args и **kwargs в переопределенном методе, который позволяет защитить себя от изменений в сигнатуре методов из сторонних библиотек (но в некоторых случаях это сообщение может указывать на настоящие баги).

ungrouped-imports × 4

Я уже использую isort для импорта

fixme × 4

Да, есть несколько вещей, которые надо исправить, но прямо сейчас исправлять их не хочу.

duplicate-code × 3

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

broad-except × 2
abstract-method × 2
redefined-builtin × 2
too-many-lines × 1

Я пытался придумать каким естественным способом разбить этот модуль, но не смог. Это один из примеров, где видно, что линтер — неправильный инструмент. Если у меня есть модуль с 980 строками кода, и я добавлю еще 30, я пересекаю лимит в 1000 строк, и уведомления от линтера мне тут ничем не помогут. Если 980 строк — это нормально, то почему 1010 плохо? Я не хочу рефакторить этот модуль, но хочу, чтобы линтер не выдавал ошибки. Единственным решением в этот момент я вижу сделать как-то так, чтобы линтер замолчал, а это противоречит конечной цели.

pointless-statement × 1
expression-not-assigned × 1
cyclic-import × 1

С циклом разобрались перемещением его части в одну из функций. Я не мог найти лучшего способа структурировать код с учетом ограничений.

unused-import × 1

Я уже добавлял # NOQA при использовании Flake8, чтобы эта ошибка не выскакивала.

too-many-public-methods × 1

Если в моем тестовом классе 35 тестов вместо 20 регламентируемых, неужели это действительно проблема?

too-many-arguments × 1

Итого: 243

Невозможно поправить

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

unused-argument × 21
invalid-name × 13
protected-access × 3

Включало доступ к «документированным внутренним объектам», таким как sys._getframe в stdlib и Django Model._meta.

too-few-public-methods × 3
too-many-arguments × 2
wrong-import-position × 2
attribute-defined-outside-init × 1
too-many-ancestors × 1

Итого: 46

Ложные сообщения

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

no-member × 395

Связано с несколькими базовыми классами: из Django и теми, которые я создал сам. Pylint не смог обнаружить переменную из-за динамизма / метапрограммирования.

Многие ошибки возникли из-за структуры кода тестов (я использовал шаблон от django-functest, который в некоторых случаях можно было поправить, добавив дополнительные базовые классы с помощью «абстрактных» методов, которые вызывают NotImplementedError) или, возможно, переименовав многие тестовые классы (я не стал этого делать, потому что в некоторых случаях это бы запутывало).

invalid-name × 52

Проблема возникала в основном потому что Pylint применил правило PEP8 о константах, считая, что каждое имя верхнего уровня, определенное с помощью =, является «константой». Определить точно, что мы подразумеваем под константой, сложнее, чем кажется, но это не относится к некоторым вещам, которые по своей природе являются константами, например, к функциям. Также правило не должно применяться к менее привычным способам создания функций, например:

Некоторые примеры — дискуссионные из-за отсутствия определения того, что такое константа. Например, следует ли считать константой экземпляр класса, определенный на уровне модуля, который может иметь или не иметь изменяемое состояние? Например, в этом выражении:

no-self-use × 23

Pylint неправильно заявил Method could be a function для множества случаев, где я использую наследование для выполнения различных реализаций, соответственно, я не могу преобразовать их в функции.

protected-access × 5

Pylint неверно оценил, кто был «владельцем» (текущий фрагмент кода создает protected атрибут объекта и использует его локально, но Pylint этого не видит).

no-name-in-module × 1
import-error × 1
pointless-statement × 1

Это утверждение на самом деле давало результат:

Я использовал это, чтобы намеренно вызвать необычную ошибку, которая вряд ли будет найдена тестами. Я не виню Pylint в том, что он это не распознал…

Итого: 477

Промежуточный итог

Мы еще не на финише, но самое время сгруппировать наши результаты:

«Хорошо» — блоки «Баги» и «Полезности» — тут Pylint определенно помог: 17.
«Нейтрально» — «Косметические правки» — незначительная польза от Pylint, ошибки не причинят урон: 252.
«Плохо» — «Бесполезное», «Невозможно поправить», «Неточности» — там, где Pylint хочет изменений в коде, где этого не требуется. В том числе там, где правки нельзя внести из-за внешних зависимостей или где Pylint просто неверно проанализировал код: 766.

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

Чтобы убрать ложные уведомления, можно заглушить вылетающий класс ошибок (что тогда повысит бесполезность использования Pylint) или добавить специальные комментарии в код. Последнее мне не хочется делать, потому что:

Это занимает время!
Мне не нравятся нагромождения из комментариев, которые существуют для того, чтобы заставить замолчать линтер.

Я с радостью добавлю эти псевдокомментарии, когда от линтера будут несомненный плюс. Кроме того, я трепетно отношусь к комментариям, поэтому моя подсветка синтаксиса отображает их ярко: так, как рекомендовано в этой статье. Тем не менее, в некоторых местах я уже добавил комментарии #NOQA для заглушения Flake8, но с ними для одной секции можно добавить лишь пять кодов ошибок.

Docstrings

Остальные проблемы, которые обнаружил Pylint — пропущенные строки документации. Я вынес их в отдельную категорию, потому что:

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

Всего Pylint обнаружил 620 недостающих докстрингов (в модулях, функциях, методах классов). Но во многих случаях я оказался прав. Например:

Когда из названия уже все понятно. Например:
Когда docstring уже определена — например, если я реализую интерфейс Django«s database router. Добавление своей строки в данном случае может быть опасным.

В остальных случаях моему коду не помешали бы эти строки описаний. Примерно в 15–30% случаях, найденных Pylint, я бы подумал «Да, надо добавить здесь docstring, спасибо Pylint за напоминание».

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

их читать — трата времени: в них нет дополнительной информации или они содержат некорректную информацию,
они помогают на подсознательном уровне выделять докстринги в тексте, поскольку в них содержится полезная информация (если вы пишите их по делу).

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

Заключение

Я считаю, что мои изначальные посылы о бесполезности Pylint оказались верны (в контексте той кодовой базы, для которой я использую Flake8). Чтобы я стал использовать его, показатель, отражающий количество ложные срабатываний, должен быть ниже.

Помимо создания визуального шума требуется дополнительное время, чтобы разобраться или отфильтровать ложные уведомления, и я бы не хотел добавлять в проект что-то из этого. Младшие разработчики будут покорнее вносить правки, чтобы снять замечания Pylint. Но это приведет к тому, что они сломают рабочий код, не понимая, что Pylint ошибается, или к тому, что в результате у вас будет много рефакторинга, чтобы Pylint смог понять ваш код правильно.

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

Другой подход заключается в использовании Pylint для ограниченного числа видов ошибок. Однако таких было лишь несколько, срабатывания на которых оказывались правильными или крайне редко ложными (в относительном и абсолютном выражении). Среди них: dangerous-default-value, stop-iteration-return, broad-exception, useless-object-inheritance.

В любом случае я надеюсь, что эта статья помогла вам при рассмотрении вопроса об использовании Pylint или в споре с коллегами.