Моя любовь к линтерам

Jul 15, 2021 09:02 · 1484 words · 7 minute read python

Я думаю, все понимают пользу стайлгайдов и оду линтерам не писал только ленивый, так что я собираюсь покинуть их число. За последние 10 лет правила форматирования кода появились у всех языков (а для некоторых даже несколько!). Причина понятна - единообразно написанный код проще читать. Глаз не цепляется за пропущенный пробел или скобку не на той строке. В Go это даже возведено в абсолют - вы не можете форматировать свою программу по-другому. Ну а для других языков есть линтеры и форматтеры. Зачастую их встраивают в git-хуки и CI/CD для нанесения тотальной пользы, но есть нюансы…

Код пишут разные люди

Даже если код отформатирован одинаково, то стиль мышления у людей различается. Кто-то любит ранние выходы из метода:

def some(self, arg1, arg2):
    if arg1 < 0:
        return None
    if arg2 > 0:
        return None
    
    return arg1 + arg2

кто-то любит единственный return:

def some(self, arg1, arg2):
    result = None
    if arg1 > 0 and arg2 < 0:
        result = arg1 + arg2
    
    return result

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

Длина строки 89 символов

“Обожаю” магические константы. А почему не 90? Или 120? А зачем нам вообще ограничивать длину строки? Наверно, чтобы избежать вот этого:

волк с пальцем у виска

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

Рассмотрим следующий кейс. Допустим, у меня есть набор методов, которые делают примерно одно и то же. Например, как-то обрабатывают событие:

class StateProcessor:
    pass


class NewStateProcessor(StateProcessor):
    template = "User %s created new document."


class RestoredFromArchiveStateProcessor(StateProcessor):
    template = "User %s sent document to archive but user %s restored it."

И я всеми силами хочу сохранить семантику - что эти классы выполняют одно и то же, но различаются лишь в одном атрибуте. В данном примере различие легко увидеть, да и вообще заметить. Но что останется от моего намерения после прохода форматтера? (Утрирую, чтобы показать суть)

class NewStateProcessor(StateProcessor):
    template = "User %s created new document."


class RestoredFromArchiveStateProcessor(
    StateProcessor
    ):
    template = "User %s sent document to"
               "archive but user %s restored 
               "it."

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

Отдельное спасибо black за такое

До форматирования:

    @parameterized.expand([
        (
            "credit_card",
            {"card_type": "Visa", "first_six": "411111", "last_four": "1111", "exp_month": 1, "exp_year": 2022},
            {"card_type": "Visa", "last_four": "1111", "month": 1, "year": 2022}
        ), (
            "bank_account_info",
            {"last_four": "1111"},
            {"last_four": "1111"}
        )
    ])

После форматирования:

    @parameterized.expand(
        [
            (
                "credit_card",
                {
                    "card_type": "Visa",
                    "first_six": "411111",
                    "last_four": "1111",
                    "exp_month": 1,
                    "exp_year": 2022,
                },
                {"card_type": "Visa", "last_four": "1111", "month": 1, "year": 2022},
            ),
            ("bank_account_info", {"last_four": "1111"}, {"last_four": "1111"}),
        ]
    )

Согласитесь, даже количество циклов unittest подсчитать уже не просто. Я уже и не говорю про редактирование.

Но можете сказать “Да кому нужна эта семантика? Код же работает!". Минимум мне она нужна и важна - я хочу понять как размышлял автор, что для него казалось важным, а что просто скопипастил. Форматирвоание кода автором помогает расставить акценты, а это в свою очередь приводит к уменьшению когнитивной нагрузки.

Но ладно, семантика какая-то… Сложно!

Другой пример - какой код проще понять? Я специально обрежу первый блок так, как будто бы он уходил за экран.

class Command(BaseCommand):
    def handle(self, *args, **kwargs):
        users_without_notification_settings = User.objects.filter(
        notifications = [NotificationSettings(user=user) for user in 
        NotificationSettings.objects.bulk_create(notifications)

class Command(BaseCommand):
    def handle(self, *args, **kwargs):
        users_without_notification_settings = User.objects.filter(
            notification_settings__isnull=True
        ).all()
        notifications = [NotificationSettings(user=user) for user in 
            users_without_notification_settings
        ]
        NotificationSettings.objects.bulk_create(notifications)

Лично мне для понимания проще первый: получили существующие данные -> подготовили новые -> обработали новые. Какие там запросы и параметры, я буду смотреть только когда буду искать там баг. Для общего понимания они не существенны, а лишь мешают.

Ладно, вот пример ещё проще:

    @dispatcher.on_message(DocumentEvent.NEW, NewDocumentData)
    @dispatcher.on_message(DocumentEvent.RESTORED_FROM_ARCHIVE, 
        RestoredFromArchiveData)
    @dispatcher.on_message(DocumentEvent.RESTORED_FROM_DELETE, 
        RestoredFromDeleteData)
    @dispatcher.on_message(DocumentEvent.REMOVED_MANUALLY, 
        RemovedManuallyData)
    @dispatcher.on_message(DocumentEvent.SENT, SentDocumentData)
    @dispatcher.on_message(DocumentEvent.VEIWED, 
        ViewedDocumentData)
    async def handle():
        ...

Сможете сходу сказать какие события здесь обрабатываются? У меня аж глаз задёргался слева-направо, хотя код мог бы читаться линейно вертикально. Вот зачем здесь перенос на новую строку?! Мелочь? А выбивает также, как и не там поставленная скобка. Стоп! Так мы ж как раз эту проблему и решали…

Сортировка импортов

Уже во второй компании я вижу подключённый isort, но так и не могу понять зачем. Этот блок по умолчанию свёрнут, вручную его правят очень редко. По сути, я его изучаю в 2х случаях: на код ревью оцениваю с какими доменами идёт работа и когда разруливаю циклические зависимости. Последнее, кстати, было лет 5 назад. В остальном меня совсем не интересует что там написано и в каком порядке. Это чисто утилитарная секция, которая нужна больше компилятору, нежели программисту. Зачем мне о ней думать? Мне приходит на ум лишь одна причина - успокоить свой ОКР.

Линтеры

you shall not pass linters Но раз мы заговорили про isort, то где-то там рядом и pylint/flake8. Я поддался хайпу и подключил у себя в GeoPuzzle. Исправление всех “ошибок” заняло целый вечер. Сколько из них было полезных предупреждений? Примерно 2 из начальных 160. Что-то мне кажется, их полезность несколько преувеличена. Может, я один такой? Но нет - вот автор приходит к таким же выводам.

Что самое обидное - все линтеры глупые. Ладно, они не понимают семантику (например, почему количество методов у этого класса должно быть больше разрешёного), но они забагованные и откровенно глупые.

possible-unused-variable

Например, у вас большая таска, которую вы хотите подробить на кусочки. В первом коммите описать интерфейс, пустые методы и согласовать на ревью. А не получится даже закоммитить (у нас же настроены хуки, верно?), т.к. pylint не любит неиспользуемые переменные/методы, так что придётся ставить # pylint: disable=possibly-unused-variable. Работа, достойная senior developer.

too-many-ancestors

class MultiPolygonWidget(BaseMultiPolygonWidget, BaseGMapWidget):  # pylint: disable=too-many-ancestors
    google_maps_api_key = settings.GOOGLE_KEY

    @property
    def media(self):
        return super().media + Media(js=('gis/MapBoxExtend.js', ))

Вот серьёзно, как я должен это переписать? И должен ли я вообще что-то переписывать?

unused-argument

@receiver(post_save, sender=Region, dispatch_uid="clear_region_cache")
def clear_region_cache(sender, instance: Region, **kwargs):  # pylint: disable=unused-argument
    for key in instance.caches():
        cache.delete(settings.POLYGON_CACHE_KEY.format(func=key, id=instance.pk))

Моё любимое! sender здесь - обязательный аргумент, _ не работает. Вот что мне с этим делать, если я его не использую?!

line-too-long

Есть у меня один файлик с тестами размером 70 строк. Как вы думаете сколько там тестов? Один из 8 строк; всё остальное - строковая константа, разбитая по строкам. Фактически это закодированный в base64 токен, подписанный одним ключом и зашифрованный другим. Правильнее было бы, конечно, написать свою реализацию подписи и шифровки, но ради одного теста можно и константу поставить.

глупый, глупый линтер

Я уже не говорю про более простые кейсы too-many-instance-attributes или line-too-long. Если по предметной области мне нужно держать большое количество аргументов вместе, то я не собираюсь угождать линтеру, а просто отключу его. Или про более сложные, когда линтер не смог в интроспекцию, или когда мне нужно вызвать super не у родителя, а у его родителя, или когда нужно сделать локальный импорт (повесить обработчик на сигнал в методе ready у django-приложения)… И во всех этих случаях я вынужден писать # pylint disable=..., очень интересно. А зачем мне тогда вообще подключённые линтеры? Но интереснее не отключать, а переписывать свой код так, чтобы пройти валидацию. “Уговори линтер” как отдельный вид искусства!

Другие языки

В некоторых других языках использование проверок и единственно возможного форматирования обязательно и обсуждению не подлежит. Например, в Go утилита gofmt преобразует ваш файл в соответствии с общим стандартом; или Rust имеет очень сильную проверку при компиляции. Но это несколько другое - скорее фишки языка, которые лежат в основе его существования, нежели какие-то сторонние вещи, подключаемые (или нет) в каждый проект и имеющие кучу настроек.

Мыши плакали, кололись, но продолжали жрать ёжиков

К сожалению, в современной командной разработке линтеры необходимы. Если вы работаете один или над своим pet-проектом, то вполне можно их и не подключать, но в команде и для бизнеса совсем другое дело. Разброс компетенций может быть очень большой, и надо как-то унифицировать код. Если сениор может написать метакласс потому что он действительно уместен, то более младшие разработчики могут использовать его не по делу. Явное отключение проверок при ревью помогает выявить сомнительные места, которые можно было бы написать проще. Да и в целом взгляд со стороны пусть даже глупого робота не даст скатиться к написанию откровенного треша.

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

Реализация хуков, определение правил, подключение линтеров в CI/CD занимает не то чтобы много времени, но может существенно упростить коммуникацию, если все в команде согласились использовать единый стиль. Да, перед каждым коммитом вам надо будет ждать секунд 15, а при пуше уже пару минут, но рассматривайте это как повод оторваться от монитора и пару раз подтянуться.