Мы выяснили, что в 5% из 666 исследованных нами репозиториев Python с открытым исходным кодом на GitHub есть три бага, вызванных ошибочным использованием запятых.
Случайно пропущенная запятая в строке списка/кортежа/множества, приводящая к ненужной конкатенации строк.
Неявная конкатенация строк, возникающая вследствие опечатки, может изменить поведение приложения. Например:
is_positive('agreed') равно False из-за опечатки — отсутствия запятой в конце 'affirmative', из-за чего 'affirmative' и 'agreed' неявно конкатенируются в 'affirmativeagreed'.
Случайная потеря запятой в кортеже из 1 элемента, из-за чего он превращается в
Из-за отсутствующей запятой ожидайте получение
Случайное добавление запятой в конец значения, превращающее его в кортеж.
Для непустых кортежей парсер Python считает скобки опциональными. Согласно документации, кортеж создаёт запятая, а не скобки. Скобки опциональны, за исключением случая пустого кортежа.
Мы не занимались построчным изучением репозиториев, а пропустили их через наш пакет статического анализа, работающий в AWS Lambda. В сумме на выполнение потребовалось 90 секунд.
Основная сложность заключалась в снижении количества ложноположительных срабатываний. С точки зрения синтаксиса, не существует разницы между отсутствием запятой в списке и намеренной неявной конкатенации строк. И в самом деле, когда мы написали первую версию проверки «возможно отсутствие запятой», то выяснили, что 95% проблем были ложноположительными: вполне возможно, что разработчик выполняет неявную конкатенацию строк, растянувшуюся на несколько строк кода:
Проверив 666 кодовых баз, мы поняли основные источники ложноположительных срабатываний и добавили допуски неявной конкатенации строк для таких типов данных. После добавления этих допусков количество ложноположительных срабатываний снизилось до незначительных величин. Так мы получили список из 24 репозиториев с реальными багами, упущенными на этапе code review.
Суммарно мы обнаружили эти баги в 24 из 666 репозиториев, и многие из них были крупными проектами в open source. Мы отправили issues и помогли устранить их за один очень загруженный день работы с тикетами и PR. Вот самые интересные ошибки:
5% репозиториев имело один из этих багов, и большинство из них были «крупными», хорошо известными и активно используемыми проектами со множеством контрибьюторов и очень надёжными процессами code review. Это подчёркивает тот факт, что для ручных процессов наподобие code review для распознавания 100% багов в 100% случаев 100% людей должны работать на 100% во время code review в 100% случаев. Видите, в чём здесь когнитивный диссонанс? Мы выполняем code review, потому что знаем, что при написании кода человек может ошибаться, но ожидаем, что при проверке кода человек не ошибётся. Есть вещи, которые хорошо умеют делать люди, а есть вещи, с которыми лучше справляется бот. Поиск запятых в ненужных местах — одна из таких вещей. Например:
Видите, в чём проблема? В строке 572 есть неявная конкатенация строк! В ней не хватает запятых, то есть она должна выглядеть так:
Неудивительно, что человек это упустил!
Некоторые из отсутствующих запятых практически никак не влияют на код, но некоторые очень значимы. Вот пример из Sentry:
Обратите внимание на отсутствие запятой в списке:
Отсутствует запятая между «releases» и «discover», что приводит к неявной конкатенации этих двух значений с созданием «releasesdiscover».
Из-за этого тест запрашивает несуществующий "/releasesdiscover", поэтому вместо "/releases" и "/discover" тестируется страница 404. Это означает, что переключатель организаций для /releases и /discover может быть сломан. Здесь есть плохой devout, из-за которого переключатель организаций полагается на этот тест и использует его как проверку качества, дающую разработчикам уверенность, что они не сломали продукт, но на самом деле тест не работает. Подобные вещи могут привести к серьёзной головной боли!
Слишком мало запятых
Случайно пропущенная запятая в строке списка/кортежа/множества, приводящая к ненужной конкатенации строк.
Неявная конкатенация строк, возникающая вследствие опечатки, может изменить поведение приложения. Например:
def is_positive(word):
words = (
'yes',
'correct',
'affirmative'
'agreed',
)
return word in words
is_positive('agreed') is True # evaluates to Falseis_positive('agreed') равно False из-за опечатки — отсутствия запятой в конце 'affirmative', из-за чего 'affirmative' и 'agreed' неявно конкатенируются в 'affirmativeagreed'.
Случайная потеря запятой в кортеже из 1 элемента, из-за чего он превращается в
str.
Из-за отсутствующей запятой ожидайте получение
TypeError, когда код позже попытается выполнить операцию кортежа с переменной, которая имеет значение str.Слишком много запятых
Случайное добавление запятой в конец значения, превращающее его в кортеж.
Для непустых кортежей парсер Python считает скобки опциональными. Согласно документации, кортеж создаёт запятая, а не скобки. Скобки опциональны, за исключением случая пустого кортежа.
Распознавание багов
Мы не занимались построчным изучением репозиториев, а пропустили их через наш пакет статического анализа, работающий в AWS Lambda. В сумме на выполнение потребовалось 90 секунд.
Основная сложность заключалась в снижении количества ложноположительных срабатываний. С точки зрения синтаксиса, не существует разницы между отсутствием запятой в списке и намеренной неявной конкатенации строк. И в самом деле, когда мы написали первую версию проверки «возможно отсутствие запятой», то выяснили, что 95% проблем были ложноположительными: вполне возможно, что разработчик выполняет неявную конкатенацию строк, растянувшуюся на несколько строк кода:
- Код наподобие SQL или HTML
- Строка User agent
- Значения-пути, например пути к файлам и URL
- Кодированный текст, например, содержимое файла JSON или CSV
- Очень длинное сообщение
- Хэш SHA
Проверив 666 кодовых баз, мы поняли основные источники ложноположительных срабатываний и добавили допуски неявной конкатенации строк для таких типов данных. После добавления этих допусков количество ложноположительных срабатываний снизилось до незначительных величин. Так мы получили список из 24 репозиториев с реальными багами, упущенными на этапе code review.
Насколько распространена эта проблема?
Суммарно мы обнаружили эти баги в 24 из 666 репозиториев, и многие из них были крупными проектами в open source. Мы отправили issues и помогли устранить их за один очень загруженный день работы с тикетами и PR. Вот самые интересные ошибки:
- Tensorflow
- Sentry
- Tensorflow
- Pydata xarray
- Google V8
- PyTorch
- Снова PyTorch
- Третий Pytorch
- rapidpro
- django-colorfield
- django-helpdesk
5% репозиториев имело один из этих багов, и большинство из них были «крупными», хорошо известными и активно используемыми проектами со множеством контрибьюторов и очень надёжными процессами code review. Это подчёркивает тот факт, что для ручных процессов наподобие code review для распознавания 100% багов в 100% случаев 100% людей должны работать на 100% во время code review в 100% случаев. Видите, в чём здесь когнитивный диссонанс? Мы выполняем code review, потому что знаем, что при написании кода человек может ошибаться, но ожидаем, что при проверке кода человек не ошибётся. Есть вещи, которые хорошо умеют делать люди, а есть вещи, с которыми лучше справляется бот. Поиск запятых в ненужных местах — одна из таких вещей. Например:
Видите, в чём проблема? В строке 572 есть неявная конкатенация строк! В ней не хватает запятых, то есть она должна выглядеть так:
'”()<>[]:,;@\\”!#$%&\’-/=?^_{}| ~.a”@example.org', '“ “@example.org',Неудивительно, что человек это упустил!
Последствия
Некоторые из отсутствующих запятых практически никак не влияют на код, но некоторые очень значимы. Вот пример из Sentry:
Обратите внимание на отсутствие запятой в списке:
Отсутствует запятая между «releases» и «discover», что приводит к неявной конкатенации этих двух значений с созданием «releasesdiscover».
Из-за этого тест запрашивает несуществующий "/releasesdiscover", поэтому вместо "/releases" и "/discover" тестируется страница 404. Это означает, что переключатель организаций для /releases и /discover может быть сломан. Здесь есть плохой devout, из-за которого переключатель организаций полагается на этот тест и использует его как проверку качества, дающую разработчикам уверенность, что они не сломали продукт, но на самом деле тест не работает. Подобные вещи могут привести к серьёзной головной боли!
