# Ошибка в коде, на которую приходится не обращать внимание

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

![1381_warning_loss_ru/image1.png](https://import.viva64.com/docx/blog/1381_warning_loss_ru/image1.png)

## Предыстория

Сейчас мы занимаемся активной разработкой статического анализатора для JavaScript и TypeScript\. Он находится в стадии [EAP](https://pvs-studio.ru/ru/pvs-studio-eap/), на который ещё можно записаться\. Мне довелось поработать над правилом [V7001](https://pvs-studio.ru/ru/docs/warnings/v7001/), которое ищет опечатки в виде одинаковых операндов бинарных выражений: 

```cpp
let baz = foo – foo // foo - bar
if (foo == foo) { // foo == bar
  // ....
}
```

В фокусе и более сложные случаи, например вложенные выражения:

```cpp
(a && b) || (c && d) || (a && b)
```

И не идентичные, но эквивалентные выражения:

```cpp
(a * b) > (b * a)
```

Несколько находок в проектах с открытым исходным кодом были описаны в нашей [статье](https://pvs-studio.ru/ru/blog/posts/js/1363/#ID68AC3AA657) про разработку этого анализатора\.

## От очарования до разочарования

Когда я только закончил писать это диагностическое правило, то запустил анализ на нашей [базе проектов](https://pvs-studio.ru/ru/blog/posts/js/1363/#ID79DE99F61B) и нашёл такой фрагмент в [pdf\.js](https://github.com/mozilla/pdf.js/):

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v7001/) The operands of the '<<' operator in the '1 << 1' expression are equivalent\. [print\_params\.js 90](https://github.com/mozilla/pdf.js/blob/25c7d9eaace0438316714aff7033dd5f4c1a542e/src/scripting_api/print_params.js#L90)

Срабатывание ссылается на строчку между 10\-м и 12\-м флагом:

```cpp
suppressCJKFontSubst: 1 << 10,
suppressCropClip: 1 << 1,
suppressRotate: 1 << 12,
```

Вполне вероятно, что этот код писался с помощью Ctrl\+C и Ctrl\+V, после чего в каждой строке имя поля и номер флага заменялись вручную\. Однако в одном месте `1 << 1` забыли заменить на `1 << 11`\. Вероятно, недостающая единица просто ускользнула от взора, ведь её и правда легко не заметить в ряду единиц\.

Этот код добавлен недавно и пока не нигде не вызывается, так что к неправильному поведению не приводит\. И всё же, когда его время настанет, `applySoftProofSettings` и `suppressCropClip` будут вести себя как один флаг\. В общем, для анализатора это хорошее срабатывание, так как здесь он нашёл настоящую ошибку\.

### Бочка дёгтя

Тем не менее, запустив PVS\-Studio для JavaScript или TypeScript, этого срабатывания вы не увидите\. Почему? Во время разработки диагностика нашла **очень много** подобных срабатываний, но подавляющая часть из них выглядит вот так:

[Angular](https://www.google.com/url?sa=t&source=web&rct=j&opi=89978449&url=https://github.com/angular/angular&ved=2ahUKEwi-6JquuMqUAxW-gf0HHcevJbIQFnoECA4QAQ&usg=AOvVaw1tvULFkP3fqqpks94a3f2q)

```cpp
export enum InputFlags {
  None = 0,
  SignalBased = 1 << 0,
  HasDecoratorInputTransform = 1 << 1,
}
```

Предупреждение PVS\-Studio: [V7001](https://pvs-studio.ru/ru/docs/warnings/v7001/) The operands of the '<<' operator in the '1 << 1' expression are equivalent\. core\.ts 49

[Babel](https://www.google.com/url?sa=t&source=web&rct=j&opi=89978449&url=https://github.com/babel/babel&ved=2ahUKEwi6mei1uMqUAxVs8LsIHVAGMBQQFnoECA4QAQ&usg=AOvVaw17APTqb3HJkkAI0IarYX1Z)

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v7001/) The operands of the '<<' operator in the '1 << 1' expression are equivalent\. index\.ts 10

Снова [pdf\.js](https://github.com/mozilla/pdf.js/)

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v7001/) The operands of the '<<' operator in the '1 << 1' expression are equivalent\. glyf\.js 17

Это всего лишь три из 42 таких же срабатываний при общем числе в 126 [V7001](https://pvs-studio.ru/ru/docs/warnings/v7001/) в тестовой базе\. Как можно заметить, везде анализатор смущает `1 << 1`\. И формально он прав: _обычно_ большого смысла сдвигать число на само себя нет, за исключением очень нишевых случаев\. Результатом `1 << 1` операции всегда будет `2`\. Всё, что такая запись делает, это заставляет интерпретатор свёртывать константу при компиляции в машинный код\.

Но здесь мы работаем с битовыми флагами\. И тяга к прекрасному сильнее умозрительной производительности, из\-за чего программисты обычно объединяют битовые флаги в красивый список с порядковыми номерами флагов на каждой строке, благо нагрузка от лишней операции и правда исчезающе мала\.

Кстати, на изначально приведённый фрагмент кода:

```cpp
applyOverPrint: 1,
applySoftProofSettings: 1 << 1,
applyWorkingColorSpaces: 1 << 2,
```

Анализатор тоже выдаст ложное срабатывание на `1 << 1` — ещё одна ложка дёгтя в эту бочку\.

Если добавить в исключения случай с `1 << 1`, то из нашей тестовой базы пропадает ровно **треть** срабатываний, все из которых гарантированно плохие, за исключением показанного в начале\. Другими словами, исключение на `1 << 1` улучшает это правило на треть\.

### Нешумный анализ

Подход, при котором мы сознательно ограничиваем выдачу срабатывания, если не до конца в нём уверены, называется unsound\-стратегией\. Процитирую нашу [статью](https://pvs-studio.ru/ru/blog/posts/cpp/1302/) про taint\-анализ:

> Существуют sound и unsound стратегии статического анализа\. Обычно анализатор работает в unsound\-стратегии и генерирует предупреждения только если **может доказать наличие ошибки**\. И при такой стратегии предупреждение о выходе за границу массива в последнем примере выдано не будет\.
>
> Sound\-стратегия следует обратному принципу: анализатор формирует предупреждение, если **не может доказать отсутствие ошибки**\. Главная проблема такой стратегии в большом количестве ложных срабатываний\.

Так как мы стараемся придерживаться unsound\-подхода и не шуметь сотнями и тысячами [ложных срабатываний](https://pvs-studio.ru/ru/blog/terms/6461/), нам приходиться вникать в существующие подходы и делать эвристические исключения для них\. Так в диагностике [V7001](https://pvs-studio.ru/ru/docs/warnings/v7001/) и появилось исключение на `1 << 1`, хотя пришлось потерять хорошее срабатывание\.

### Почему не сделать более точное исключение?

Можно было бы попытаться придумать что\-то более хитрое\. Например, проверять, нет ли по соседству других битовых флагов\. Но что такое "по соседству" для статического анализатора кода? Анализатор работает с синтаксическим деревом, и мы могли бы попробовать искать соседей по перечислению:

![1381_warning_loss_ru/image2.png](https://import.viva64.com/docx/blog/1381_warning_loss_ru/image2.png)

Но, во\-первых, такую эвристику легко "обмануть":

* добавить лишний элемент в дереве \(вызов функции, арифметическая операция и пр\.\);
* поменять порядок определения флагов;
* записать флаги в промежуточные переменные\.

В каждом из этих случаев анализатор либо не увидит реальную ошибку, либо снова начнёт выдавать ложные срабатывания — вернётся к той же проблеме, от которой мы уходили\.

Во\-вторых, если вы обратите внимание, то приведённые выше срабатывания были в трёх разных контекстах:

* в `enum`;
* в инициализации поля внутри object expression;
* в константных переменных\.

И это не считая экзотических случаев вроде битовых флагов в полях класса\. Таких случаев много, особенно в JavaScript/TypeScript, и их очень легко пропустить\. 

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

Так что да, иногда приходится делать сложное решение, выбрасывая хорошее срабатывание ради большей надёжности анализатора\. К тому же эта ситуация натолкнула нас на мысль о том, что можно попробовать сделать отдельное диагностическое правило для поиска ошибок в определении битовых флагов :\)

## Послесловие

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

А чтобы следить за выходом новых статей про качество кода, можете подписаться на наш ежемесячный [дайджест](https://pvs-studio.ru/ru/subscribe/) или на мой личный [блог](https://x.com/kvolokhovskii)\.