Комментарии 3
Не являюсь экспертом по статическим анализаторам, но очень люблю pylint, хотя моё первое впечатление было очень похоже на мнение автора оригинальной статьи. Мне кажется, прелесть pylint'а в том, что он хорошо подходит, чтобы задавать правильные вопросы, а не требовать немедленно что-то исправить.
Во-первых, многие сообщения содержат альтернативы. Например,
no-self-use: Method could be a function Used when a method doesn’t use its bound instance, and so could be written as a function.
Можно задуматься: "Ok. Если я вынесу это в функцию, какие будут последствия". Лично мне это помогает: иногда приходит понимание, что класс вообще не нужен, или метод должен быть явно объявлен статическим. Естественно, бывают и ситуации, когда приходится оставлять, как есть (например, для callback'ов в kivy).
Во-вторых, pylint имеет несколько уровней сообщений: error, warning, refactor, convention (возможно, какие-то упустил). Ни для кого не будет секретом, что все сообщения имеют код, начинающийся с буквы соответствующего уровня (например, R0801 для duplicate-code), но не все знают, что pylint формирует код возврата в зависимости от наличия сообщений той или иной группы: 1 группа — 1 бит. Мы это использовали, чтобы автоматически блокировать merge, если pylint находит ошибки (errors) или предупреждения (warnings), исходя из (status_code | 3). В итоге, у нас прикручен автоматический статический анализ, и именно команда решила, какие проблемы кода считать блокирующими.
Мне понравилось, что автор отметил несколько тонких моментов, таких как no-member, но смутил вывод по этому поводу. На всякий случай напишу, чтобы люди не отказывались от pylint'а из-за этого. Иногда модули создаются динамически, и pylint не может делать выводы на их основе (не знаю, как к этому относятся другие анализаторы кода). Эту проблему можно встретить, например, для sqlalchemy или pyvmomi. Но будет неправильным делать вывод, что настоящие ошибки no-member "утонут" среди ложных срабатываний из-за обращения к sqlalchemy. В конфиге pylint'а есть отдельная секция TYPECHECK, где можно указать динамически сгенерированные атрибуты или исключить классы и модули из проверки. Поэтому, мне кажется, что автор зря выгоняет pylint
Если бы Pylint был моим коллегой, помогающим делать ревью кода, я бы умолял его уйти.
Вы ведь не выгоняете коллегу, чьим комментариям не собираетесь следовать, а объясняете, почему не считаете изменения необходимыми?
Второй тонкий момент — это анализ тестов. Как автоматизатор тестирования, тоже часто обращаю на это внимание. Глобальных проблем в общем-то две. Первая — это duplicate-code. Выбирая между стремлением к "1 тест — 1 проверка" и DRY, я в 100% случаев выберу первое. Pylint достаточно (возможно, слишком) чувствителен к похожему коду и, к сожалению, не предоставляет возможности этим управлять так же, как другими сообщениями. Вторая проблема связана с фреймворками тестирования. Большинство проектов используют либо unittest, либо pytest. Первый был вдохновлён фреймворком из Java (jUnit?), поэтому там всё завязано на классах, что действительно приводит к их избыточности (too-many-public-methods и, подозреваю, too-many-ancestors). Переход на pytest поможет избавиться от этих сообщений, но привнесёт redefined-outer-name из-за фикстур, которые передаются, как аргументы другим фикстурам и тестовым функциям. В любом случае, это объективная реальность, о которой pylint Вам будет сообщать.
В некоторых пунктах мне показалось, что автор лукавит. В статье не так много кода, поэтому предметно не поговорить. Но, в частности, в примере с
foo, bar = get_some_stuff()мне не понятно, почему не убрать неиспользуемую bar так
foo, _ = get_some_stuff()или так
foo = get_some_stuff()[0]С docstring'ами, мне кажется, тоже есть доля лукавства. Docstring подаётся, как аналог комментария к неочевидному куску кода. Но это ведь в большинстве случаев не так: даже для очевидных функций docstring может дать дополнительные детали, такие как варианты использования и doctest'ы, не говоря уже об аргументах и возвращаемых значениях: современные IDE прямо из коробки умеют их парсить и использовать, как альтернативу type hint'ам.
Странный тест выходит. Многие ошибки можно было бы избежать установкой плагина для pylint, который помогает понимать Django
В свое время выбрал flake8 за возможность точной настройки отключения проверок. Была огромная кодовая база и расставлять комментарии на файлы и строчки не хотелось. Я просто прошёлся по отчету ошибок и отключил конкретные проверки в конкретных файлах. Это позволило быстро сконфигурировать CI и не давать этой заразе расползаться.
А вот пример из проекта на Джанго: никаких ложных срабатываний на отсутствие докстрингов нет.
per-file-ignores =
*/tests/*:D102,D103,S101
*/manage.py:D103,C812
*/settings/*:F401,F403,F40
Pylint: детальная проверка работы анализатора кода