Мы используем куки, чтобы пользоваться сайтом
было удобно.
В разработке статического анализатора, как и в случае с любым ПО, приходится идти на компромиссы. Иногда мы вынуждены отсекать хорошие срабатывания, чтобы инструмент в целом стал лучше. В этой заметке на реальном примере посмотрим на такой компромисс.
Сейчас мы занимаемся активной разработкой статического анализатора для JavaScript и TypeScript. Он находится в стадии EAP, на который ещё можно записаться. Мне довелось поработать над правилом V7001, которое ищет опечатки в виде одинаковых операндов бинарных выражений:
let baz = foo – foo // foo - bar
if (foo == foo) { // foo == bar
// ....
}В фокусе и более сложные случаи, например вложенные выражения:
(a && b) || (c && d) || (a && b)И не идентичные, но эквивалентные выражения:
(a * b) > (b * a)Несколько находок в проектах с открытым исходным кодом были описаны в нашей статье про разработку этого анализатора.
Когда я только закончил писать это диагностическое правило, то запустил анализ на нашей базе проектов и нашёл такой фрагмент в pdf.js:
flagValues: Object.freeze({
applyOverPrint: 1,
applySoftProofSettings: 1 << 1,
applyWorkingColorSpaces: 1 << 2,
emitHalftones: 1 << 3,
emitPostScriptXObjects: 1 << 4,
emitFormsAsPSForms: 1 << 5,
maxJP2KRes: 1 << 6,
setPageSize: 1 << 7,
suppressBG: 1 << 8,
suppressCenter: 1 << 9,
suppressCJKFontSubst: 1 << 10,
suppressCropClip: 1 << 1,
suppressRotate: 1 << 12,
suppressTransfer: 1 << 13,
suppressUCR: 1 << 14,
useTrapAnnots: 1 << 15,
usePrintersMarks: 1 << 16,
}),Здесь определяют набор флагов управления конвертацией для печати. Каждый флаг — одна опция, и через побитовые операции их можно свободно комбинировать. Флаги делятся на группы: цветокоррекция, подавление трансформаций, шрифты и прочее. Однако с JavaScript кодом здесь что-то не так. Заметили?
Предупреждение PVS-Studio: V7001 The operands of the '<<' operator in the '1 << 1' expression are equivalent. print_params.js 90
Срабатывание ссылается на строчку между 10-м и 12-м флагом:
suppressCJKFontSubst: 1 << 10,
suppressCropClip: 1 << 1,
suppressRotate: 1 << 12,Вполне вероятно, что этот код писался с помощью Ctrl+C и Ctrl+V, после чего в каждой строке имя поля и номер флага заменялись вручную. Однако в одном месте 1 << 1 забыли заменить на 1 << 11. Вероятно, недостающая единица просто ускользнула от взора, ведь её и правда легко не заметить в ряду единиц.
Этот код добавлен недавно и пока не нигде не вызывается, так что к неправильному поведению не приводит. И всё же, когда его время настанет, applySoftProofSettings и suppressCropClip будут вести себя как один флаг. В общем, для анализатора это хорошее срабатывание, так как здесь он нашёл настоящую ошибку.
Тем не менее, запустив PVS-Studio для JavaScript или TypeScript, этого срабатывания вы не увидите. Почему? Во время разработки диагностика нашла очень много подобных срабатываний, но подавляющая часть из них выглядит вот так:
export enum InputFlags {
None = 0,
SignalBased = 1 << 0,
HasDecoratorInputTransform = 1 << 1,
}Предупреждение PVS-Studio: V7001 The operands of the '<<' operator in the '1 << 1' expression are equivalent. core.ts 49
const enum PRINTER_FLAGS {
EMPTY = 0,
PRESERVE_FORMAT = 1 << 0,
COMPACT = 1 << 1,
CONCISE = 1 << 2,
RETAIN_LINES = 1 << 3,
RETAIN_FUNCTION_PARENS = 1 << 4,
AUX_COMMENTS = 1 << 5,
}Предупреждение PVS-Studio: V7001 The operands of the '<<' operator in the '1 << 1' expression are equivalent. index.ts 10
Снова pdf.js
const ON_CURVE_POINT = 1 << 0;
const X_SHORT_VECTOR = 1 << 1;
const Y_SHORT_VECTOR = 1 << 2;
const REPEAT_FLAG = 1 << 3;
const X_IS_SAME_OR_POSITIVE_X_SHORT_VECTOR = 1 << 4;
const Y_IS_SAME_OR_POSITIVE_Y_SHORT_VECTOR = 1 << 5;
const OVERLAP_SIMPLE = 1 << 6;Предупреждение PVS-Studio: V7001 The operands of the '<<' operator in the '1 << 1' expression are equivalent. glyf.js 17
Это всего лишь три из 42 таких же срабатываний при общем числе в 126 V7001 в тестовой базе. Как можно заметить, везде анализатор смущает 1 << 1. И формально он прав: обычно большого смысла сдвигать число на само себя нет, за исключением очень нишевых случаев. Результатом 1 << 1 операции всегда будет 2. Всё, что такая запись делает, это заставляет интерпретатор свёртывать константу при компиляции в машинный код.
Но здесь мы работаем с битовыми флагами. И тяга к прекрасному сильнее умозрительной производительности, из-за чего программисты обычно объединяют битовые флаги в красивый список с порядковыми номерами флагов на каждой строке, благо нагрузка от лишней операции и правда исчезающе мала.
Кстати, на изначально приведённый фрагмент кода:
applyOverPrint: 1,
applySoftProofSettings: 1 << 1,
applyWorkingColorSpaces: 1 << 2,Анализатор тоже выдаст ложное срабатывание на 1 << 1 — ещё одна ложка дёгтя в эту бочку.
Если добавить в исключения случай с 1 << 1, то из нашей тестовой базы пропадает ровно треть срабатываний, все из которых гарантированно плохие, за исключением показанного в начале. Другими словами, исключение на 1 << 1 улучшает это правило на треть.
Подход, при котором мы сознательно ограничиваем выдачу срабатывания, если не до конца в нём уверены, называется unsound-стратегией. Процитирую нашу статью про taint-анализ:
Существуют sound и unsound стратегии статического анализа. Обычно анализатор работает в unsound-стратегии и генерирует предупреждения только если может доказать наличие ошибки. И при такой стратегии предупреждение о выходе за границу массива в последнем примере выдано не будет.
Sound-стратегия следует обратному принципу: анализатор формирует предупреждение, если не может доказать отсутствие ошибки. Главная проблема такой стратегии в большом количестве ложных срабатываний.
Так как мы стараемся придерживаться unsound-подхода и не шуметь сотнями и тысячами ложных срабатываний, нам приходиться вникать в существующие подходы и делать эвристические исключения для них. Так в диагностике V7001 и появилось исключение на 1 << 1, хотя пришлось потерять хорошее срабатывание.
Можно было бы попытаться придумать что-то более хитрое. Например, проверять, нет ли по соседству других битовых флагов. Но что такое "по соседству" для статического анализатора кода? Анализатор работает с синтаксическим деревом, и мы могли бы попробовать искать соседей по перечислению:
Но, во-первых, такую эвристику легко "обмануть":
В каждом из этих случаев анализатор либо не увидит реальную ошибку, либо снова начнёт выдавать ложные срабатывания — вернётся к той же проблеме, от которой мы уходили.
Во-вторых, если вы обратите внимание, то приведённые выше срабатывания были в трёх разных контекстах:
enum;И это не считая экзотических случаев вроде битовых флагов в полях класса. Таких случаев много, особенно в JavaScript/TypeScript, и их очень легко пропустить.
И, в-третьих, даже если их всех учесть, раздув код диагностики, нет гарантии, что с новой версией языка эвристика банально не устареет. Ведь могут появится синтаксические конструкции, которые её разрушат.
Так что да, иногда приходится делать сложное решение, выбрасывая хорошее срабатывание ради большей надёжности анализатора. К тому же эта ситуация натолкнула нас на мысль о том, что можно попробовать сделать отдельное диагностическое правило для поиска ошибок в определении битовых флагов :)
Надеюсь, вам понравилась эта история из будней разработки статических анализаторов, а также стало понятнее, как она ведётся и как мы стараемся сделать инструмент лучше. Если вдруг сталкивались с похожими дилеммами, то пишите в комментариях, мне будет интересно почитать :)
А чтобы следить за выходом новых статей про качество кода, можете подписаться на наш ежемесячный дайджест или на мой личный блог.
0
0
0
0