22.10.2018 10:57
Andrey2008 79 16100 Источник
?Как PVS-Studio оказался внимательнее, чем три с половиной программиста

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь оказался внимательнее нескольких человек.

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


Святослав прочитал письмо и засомневался, что анализатор мог так грубо ошибиться. Поэтому он пришёл ко мне на консультацию. У Святослава была надежда, что мой глаз намётан и я замечу что-то такое, что подскажет, почему анализатор выдал все эти странные сообщения. К сожалению, я только подтвердил, что сообщения действительно очень странные и их не должно быть. Однако что же послужило причиной их возникновения, я заметить не смог. Было решено открыть задачу в багтрекере и начать разбираться, что же не так.

И только когда Святослав начал делать синтетические примеры, чтобы подробно расписать проблему в багтрекере, на него снизошло озарение. Давайте теперь посмотрим, сможете ли вы быстро найти причину, почему анализатор выдаёт 4 сообщения.

Вот текст письма, публикуемый с разрешения автора. И поясняющая картинка, которая была прикреплена к письму.

V560 warnings here are all false. Running with most recent version of PVS-Studio for personal use. Basically, «IF» statement is correct. Outer one is done for speed — inner ones are still needed and non are always true or false.

Письмо


Теперь, уважаемый читатель, ваше время проверить себя! Видите ошибку?

Ваше время проявить внимательность. А единорог пока немного подождёт.

Единорог-ждун


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

Теперь пояснение для тех, кто поленился искать баг. Ещё раз рассмотрим условие:

if (!((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
     ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
     ((ch >= 0x0FF41) && (ch <= 0x0FF5A)))

Автор кода планировал проверить, что символ не входит ни в один из трёх диапазонов.

Ошибка в том, что оператор логический NOT (!) применяется только к первому подвыражению.

Если выполнилось условие:

!((ch >= 0x0FF10) && (ch <= 0x0FF19))

то вычисление выражения прерывается согласно short-circuit evaluation. Если условие не выполняется, то значение переменной ch лежит в диапазоне [0xFF10..0xFF19]. Соответственно, четыре дальнейших сравнения не имеют смысла. Все они будут ложными или истинными.

Ещё раз. Смотрите, если ch лежит в диапазоне [0xFF10..0xFF19] и продолжается вычисление выражения, то:

  • ch >= 0x0FF21 — всегда false
  • ch <= 0x0FF3A — всегда true
  • ch >= 0x0FF41 — всегда false
  • ch <= 0x0FF5A — всегда true

Об этом и предупреждает анализатор PVS-Studio.

Вот так, статический анализатор оказался внимательнее пользователя и двух с половиной программистов из нашей команды.

Чтобы исправить ситуацию, надо добавить дополнительные скобки:

if (!(((ch >= 0x0FF10) && (ch <= 0x0FF19)) ||
      ((ch >= 0x0FF21) && (ch <= 0x0FF3A)) ||
      ((ch >= 0x0FF41) && (ch <= 0x0FF5A))))

Или переписать условие:

if (((ch < 0x0FF10) || (ch > 0x0FF19)) &&
    ((ch < 0x0FF21) || (ch > 0x0FF3A)) &&
    ((ch < 0x0FF41) || (ch > 0x0FF5A)))

Впрочем, я не могу рекомендовать к использованию ни один из этих вариантов. Я бы для упрощения чтения кода написал так:

const bool isLetterOrDigit =    (ch >= 0x0FF10 && ch <= 0x0FF19)  // 0..9
                             || (ch >= 0x0FF21 && ch <= 0x0FF3A)  // A..Z
                             || (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z
if (!isLetterOrDigit)

Обратите внимание, что я убрал часть скобок. Как только что мы видели, большое количество скобок вовсе не помогло избежать ошибки. Скобки должны облегчать чтение кода, а не усложнять. Программисты хорошо помнят, что приоритет сравнений =<, => выше, чем у оператора &&. Поэтому здесь скобки не нужны. А вот если спросить, что приоритетней — && или ||, многие растеряются. Поэтому для задания последовательности вычислений &&, || скобки лучше поставить.

Почему лучше писать || в начале я описывал в статье "Главный вопрос программирования, рефакторинга и всего такого" (см. главу: Выравнивайте однотипный код «таблицей»).

Всем спасибо за внимание. Скачивайте и начинайте использовать PVS-Studio. Он поможет выявлять многие ошибки и потенциальные уязвимости на самых ранних этапах.



Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers.

Комментарии (79)


  1. Zolg
    22.10.2018 14:08
    #19265643
    +1

    Примерно поэтому придерживаюсь правила 'лишних скобок не бывает'


    1. Archius11
      23.10.2018 10:14
      #19269293
      +5

      Вы будто статью не дочитали.


      1. Zolg
        24.10.2018 12:44
        #19275041

        Лично мне так наглядней. Оператор — явно указанная область действия. Плюс подсветка в IDE и вот это вот все.
        Ошибку в примере нашел сразу.

        Your milage may vary.


    1. AgentRX
      23.10.2018 10:32
      #19269391
      +3

      Простота и читаемость кода лишней не бывает. А множество скобок лишь ухудшают восприятие логики.


      1. Yuuri
        23.10.2018 16:35
        #19271579

        Вы сейчас просто разом всех лисперов идеологически оскорбили!


    1. EviLOne
      24.10.2018 23:22
      #19277795

      Большое количество скобок увеличивает вероятность ошибки. Лучше разделять операции по строчкам, по идеи для компилятора это не имеет смысла.

      Пример 1
      bool Predicate;

Predicate  = (CONST1 < a) && (CONST2 > a);

Predicate |= (CONST3 < a) && (CONST4 > a);

Predicate |= (CONST5 < a) && (CONST6 > a);

if (false != Predicate)
{
...
};


      1. Antervis
        25.10.2018 00:56
        #19278053

        когда много строк относится к одной операции читать код тоже неприятно. Обычно лучше вынести сложную проверку в функцию, аля:

        inline bool withinSomeRange(int a) {
    return (a > CONST1 && a < CONST2)
        && (a > CONST3 && a < CONST4)
        && (a > CONST5 && a < CONST6);
}

if (withinSomeRange(a)) {
// ...

        Самое главное в таком подходе то, что условие именуется. С удачно выбранным именем функции во-первых её назначение понятно без чтения реализации, во-вторых проще найти ошибку в условии, нарушающем предполагаемую из имени логику


  1. questor
    22.10.2018 14:35
    #19265795
    +2

    Красивый пример. Коротко, наглядно, понятно. Переписано хорошо. Можно статью джунам показывать в качестве примера.


  1. MonkAlex
    22.10.2018 14:46
    #19265831
    +1

    Поэтому мне нравится подсветка скобочек в IDE.
    После этого самого "!" ставишь курсор и смотришь, подсветило в конце или нет.


    1. Vlad_IT
      22.10.2018 15:13
      #19265979
      +1

      Я вот таким чудом пользуюсь https://marketplace.visualstudio.com/items?itemName=TomasRestrepo.Viasfora там сразу без курсора видно, т.к. для каждой пары задается свой цвет.


      Заголовок спойлера

      image


      1. MonkAlex
        22.10.2018 15:34
        #19266089

        А можете вариант из статьи показать как будет выглядеть?
        ПС: не всегда пишу в студии, так что конкретно студийное решение не всегда хорошо.


        1. Vlad_IT
          22.10.2018 17:20
          #19266681

          А можете вариант из статьи показать как будет выглядеть?

          Студии под рукой нет, в VS Code такое же расширение у меня стоит.
          image


          1. MonkAlex
            22.10.2018 17:41
            #19266793
            +3

            Красных скобок в итоге много, т.е. первая и последняя в принципе красные, в конкретно этом кейсе не сильно помогает.


            1. AngReload
              23.10.2018 06:44
              #19268603

              Здесь нужна по настоящему мощная подсветка:


      1. berezuev
        22.10.2018 15:54
        #19266193

        И вариант для тех, кто использует продукты Intellij: plugins.jetbrains.com/plugin/10080-rainbow-brackets


        1. lany
          22.10.2018 19:26
          #19267263

          В Идейке (правда только для Джавы, видимо) и подсветка скобок не нужна, варнинги будут сразу в редакторе при вводе текста. У нас это онлайн-анализатор ловит:



          Любопытно, кстати, что PVS-Studio анализирует недостижимый код. Мы, например, не выдаём предупреждение, что ch <= 0x0FF3A всегда истинно, потому что оно вообще недостижимо. Видимо, когда PVS-Studio находит всегда ложное условие, оно выдаёт на него варнинг и считает, что этого условия нету и продолжает анализ дальше?


          1. lany
            22.10.2018 19:35
            #19267295

            А ещё нажав Ctrl+Shift+P два раза на выражении, можно увидеть статически выведенный диапазон значений:



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


            Все эти фичи есть в бесплатной версии IDEA Community :-)


          1. Andrey2008 Автор
            22.10.2018 19:44
            #19267333

            PVS-Studio иногда анализирует недостижимый код, хотя многие диагностики туда не лезут. Вот именно V560 это в рамках одного выражения этого не учитывает. Так уж диагностика была исторически реализована. Хорошо это или плохо, не знаю. Всё относительно :). Менять поведение особенного смысла не вижу.

            А вот, например, диагностика V522 ведёт себя иначе. Здесь будет предупреждение:

            void F(int *a)
{
  if (!a && *a == 2)
    foo();
}

            V522 CWE-476 Dereferencing of the null pointer 'a' might take place. consoleapplication2017.cpp 125

            А здесь (в мёртвом коде) уже нет:
            void F(int *a)
{
  if (a && !a && *a == 2)
    foo();
}

            Здесь нет V522, зато ожидаемо есть V560 CWE-570 A part of conditional expression is always false: !a. test.cpp 125


            1. lany
              22.10.2018 19:50
              #19267363

              Менять не надо, если откровенно путающих ситуаций не возникает, согласен.


              a && !a && *a == 2

              А если наоборот, !a && a && *a == 2


              1. Andrey2008 Автор
                22.10.2018 20:13
                #19267455

                V560 CWE-570 A part of conditional expression is always false: a. test.cpp 125


    1. vsb
      23.10.2018 09:58
      #19269193
      +2

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


  1. thatsme
    22.10.2018 15:27
    #19266049
    +1

    От этого "!", захотелось избавиться с первого взгляда на код. И только потом спросил себя, а не хотел-ли автор вынести это «НЕ» за общие скобки. Вобщем да, — лишних скобок не бывает.


  1. Sabubu
    22.10.2018 16:15
    #19266317

    А в PHP есть костыль: при использовании ! по-другому интерпретируются приоритеты операторов:

    if (!$x = 2)

    Интерпретируется как !($x = 2), а не как (!$x) = 2, как следует из приоритетов.


    1. AllexIn
      22.10.2018 16:29
      #19266411
      +1

      Ужас, блин! Тут надо ворнинг пихать, а не другую интрепретацию делать…


      1. GreedyIvan
        22.10.2018 18:36
        #19267039
        +1

        Не надо такое в ворнинг.
        Это базовый шаблон для PHP

        if (!$x = foo()) {
    // функция не вернула not empty результат
}
// используем $x


        1. lex_r
          23.10.2018 10:14
          #19269291
          +1

          В условиях лучше не использовать присваивания независимо от языка. Лучше писать так:

          $x = foo();
if (!$x) {
    // функция не вернула not empty результат
}
// используем $x


    1. a-tk
      22.10.2018 17:30
      #19266733
      +1

      А ещё в PHP есть операторы && и AND, || и OR, с разными приоритетами (буквенные — ниже)


    1. Goodkat
      22.10.2018 21:39
      #19267801
      +1

      !($x = 2) всегда будет false.
      (!$x) = 2 — тут ошибка.
      if (!$x = 2) — условие никогда не выполнится же, так как !2 даст false.


  1. aamonster
    22.10.2018 17:11
    #19266633
    +2

    Плохо стало уже при попытке прочитать код. Такое надо сразу переписывать, не дожидаясь диагностики анализатора (всё равно она тут "понятна" как ошибки при компиляции сложносочинённых c++ темплейтов), хотя бы в три строчки, как вы сделали при разборе.


    1. Mingun
      22.10.2018 17:52
      #19266845

      Хотя бы банально выкинуть все лишние скобки, которые только запутывают, но не несут никакого практического смысла.


      1. aamonster
        22.10.2018 19:04
        #19267167

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


        1. Mingun
          22.10.2018 19:14
          #19267213
          +1

          Речь именно что про данный случай. Ошибка то возникла как раз из-за того, что программист запутался в скобках. А было их всего две пары — для if'а и отрицания — проблемы бы вообще не было.
          Ну, и никто не заставляет помнить приоритеты всех операторов, но, извините, приоритеты сложения/умножения и ИЛИ/И/НЕ должны отскакивать на зубок, на мой взгляд, и в дополнительных скобках не нуждаться.


          1. aamonster
            22.10.2018 22:50
            #19268063

            У меня в анамнезе Паскаль, в котором приоритет операции and выше, чем операции сравнения. Я, конечно, запомнил разницу, но скобки зачастую ставлю. И не поручусь, что кто-то по запарке не прочитает неправильно.


  1. vanxant
    22.10.2018 18:11
    #19266939
    +3

    Приоритеты & и | запоминаются очень легко. & это умножение, | сложение.


    1. lany
      23.10.2018 06:36
      #19268599

      Главное не распространить это на битовый сдвиг, который у многих ассоциируется с умножением и делением, а приоритет его ниже сложения.


  1. Akon32
    22.10.2018 18:58
    #19267135

    А вот если спросить, что приоритетней — && или ||, многие растеряются.

    С кем вам приходится работать? Гнать надо этих "многих", извините.


    А не лучше ли написать вместо


    цитата
    const bool isLetterOrDigit =    (ch >= 0x0FF10 && ch <= 0x0FF19)  // 0..9
|| (ch >= 0x0FF21 && ch <= 0x0FF3A)  // A..Z
|| (ch >= 0x0FF41 && ch <= 0x0FF5A); // a..z
if (!isLetterOrDigit)


    1. multiprogramm
      22.10.2018 19:29
      #19267267

      Вот да. Правда, второй вариант не всегда может прокатить, не каждый символ можно так просто добавить в исходник — и в таких случаях я бы рекомендовал завести константы с говорящими именами, типа BIG_LETTER_A или FIRST_BIG_LETTER_EN, а в них уже можно любые коды засовывать, всё равно далее исходник будет читаемым.
      А вот третий вариант лично я бы рекомендовал почти всем и всегда. А то и в отдельный метод IsLetterOrDigit.

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


    1. beeruser
      22.10.2018 23:00
      #19268091
      +1

      У вас ASCII символы, а не unicode.
      Код '0' будет 0x30 а не как ожидалось 0xff10
      unicode-table.com/ru/blocks/halfwidth-and-fullwidth-forms


      1. Akon32
        23.10.2018 11:50
        #19269911
        +1

        И правда. Тогда нужно u'a', u'z' и т.п., или как там выглядят юникод литералы на С++. Писать 0xff10 — явно плохая практика.


        1. beeruser
          23.10.2018 20:38
          #19272655
          +1

          Тогда нужно u'a', u'z'

          И получится у вас ещё большая путаница. Эти символы лишь похожи на обычные, но на самом деле другие. Ссылку я не случайно привёл.
          a !=a

          Вот вы собрались кого-то там гнать с работы, но сами с двух попыток не смогли правильный символ задать.

          Может сейчас всё иначе, но раньше использовать исходники в unicode/uft8 — хороший способ познать боль, получив на экране квадратики любимым шрифтом или проблемы с контролем версий.


    1. Salabar
      23.10.2018 12:21
      #19270117
      -2

      Еще вариант:

      код
      const DWORD ranges[RANGE_COUNT][2] = {{0x0FF10, 0x0FF19}, {0x0FF21, 0x0FF3A },
{0x0FF41, 0x0FF5A }};
bool isLetterOrDigit = false;
for (size_t i = 0; i < RANGE_COUNT; ++i){
     isLetterOrDigit ||= ch >= ranges[i][0] && ch <= ranges[i][1];
}


  1. multiprogramm
    22.10.2018 19:53
    #19267379

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


    1. Andrey2008 Автор
      22.10.2018 20:15
      #19267467

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


      1. aamonster
        23.10.2018 12:23
        #19270127
        +1

        Ну, тут imho невозможно выдать понятную диагностику, пока не будет понятного кода. Т.е., получается, что надо навязывать codestyle?


        1. Mingun
          23.10.2018 17:02
          #19271737
          +1

          Я думаю, будет достаточно добавить в сообщение ", потому что ...". В данном примере,


          выражение (ch >= 0x0FF21) && (ch <= 0x0FF3A) всегда false, потому что ch < 0x0FF10 и ch > 0x0FF19, что следует из выражения !((ch >= 0x0FF10) && (ch <= 0x0FF19))

          Подчеркнутые места сделать ссылками на соответствующие места в коде. Так как абстрактный вычислитель значений уже есть, то кажется, что это не очень сложно должно быть сделать


  1. slonopotamus
    22.10.2018 20:58
    #19267647
    -5

    Эмм… Это вот вы серьёзно про "многих" которые не знают у чего выше приоритет — && или ||? Они точно программисты? А бухгалтеры многие не знают что приоритетнее — умножение или сложение?


    1. Andrey2008 Автор
      22.10.2018 21:13
      #19267711

      Добавил опрос.


      1. slonopotamus
        23.10.2018 08:43
        #19268809

        Спасибо. Результаты… удивляют.


    1. iig
      22.10.2018 22:06
      #19267897
      -1

      Джва с половиной программиста уже запуталась в казалось бы несложном выражении.


    1. Antervis
      22.10.2018 22:33
      #19268005
      +1

      вот только знание приоритетов этих операций нужно лишь для явных нарушений правил «хорошего тона»


      1. slonopotamus
        22.10.2018 22:46
        #19268053
        -1

        Утверждаете ли вы что запись 2 + 2 * 2 нарушает правила "хорошего тона"? И кто устанавливает эти правила? Чем эта запись лучше/хуже a || b && c?


        1. Antervis
          23.10.2018 01:32
          #19268367

          как минимум тем, что не выучив приоритет арифметических операций даже школу не закончишь. Во-вторых, логических операций несколько больше, и, даже если вы способны вспомнить приоритет конъюнкции и дизъюнкции, сомневаюсь, что выражение аля x & y && z | q || w окажется для вас таким же тривиальным.


        1. mSnus
          23.10.2018 03:09
          #19268453

          Конечно, нарушает. Найти вам примеры языков, где 2+3*2 будет равно 10?


          1. slonopotamus
            23.10.2018 08:37
            #19268779
            -1

            Текст правила или статьи или чего там именно оно «конечно» нарушает в студию. Желательно со ссылкой на место откуда ваше правило взялось.


          1. slonopotamus
            23.10.2018 09:36
            #19269061
            -2

            Что докажет ваш пример? Я могу выдумать язык программирования где результатом этого выражения будет «КРАСНЫЙ КРУГ,» и что? Есть языки в которых это вообще не является валидным выражением, и, опять же, что с того?


            1. mSnus
              23.10.2018 13:22
              #19270541

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


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


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


              В исходном случае правило, что && это умножение, а || — сложение, может быть далеко не очевидно тем, кто привык к другим языкам. И подобная привычка легко может сработать как выстрел в ногу.


              1. slonopotamus
                23.10.2018 14:02
                #19270835
                +1

                Я понимаю что «хороший тон», «правила приличия», «мораль» и всё такое штука субъективная и различается от социума к социуму и от индивидуума к индивидууму. Но если вы посмотрите тредик, мои прямые вопросы о том, что собеседники вкладывают в понятие «хорошего тона» применительно к &&/||, успешно игнорируются и мне видимо предлагается догадываться с помощью телепатии.

                В исходном случае правило, что && это умножение, а || — сложение, может быть далеко не очевидно тем, кто привык к другим языкам. И подобная привычка легко может сработать как выстрел в ногу.


                Разверните мысль, какая именно привычка может сработать как выстрел в ногу? Привыкание к языкам в которых || приоритетнее &&? Вполне допускаю что вы правы и лучше бы не привыкать к таким языкам.


        1. Am0ralist
          23.10.2018 11:25
          #19269757

          Могу в ответ баянчик приложить:
          image


        1. NDA81
          24.10.2018 08:42
          #19273973

          Расставленные скобки однозначно показывают какое выражение должно(хотел написать автор) быть записано:
          (2 + 2) * 2 или 2 + (2 * 2).


    1. ads83
      23.10.2018 08:49
      #19268829
      +9

      Когда я учился, я знал про приоритет этих операций. Также как и чем отличается сектор диска от кластера и на каком прерывании «висит» клавиатура. Но сегодня мне нужно посмотреть документацию.
      На это есть причины

      • Я отдаю отчет, что в разных языках правила могут быть разными. В статье С, который я не знаю. Вероятно, там такие же правила как в родной Java. Но хороший код работает строго определенным образом, а заказчик не поймет фразы «наверное, код правильный». Я лучше сверюсь с доками или напишу тест на это условие.
      • Я не пишу в таком стиле и переписываю, когда читаю такие конструкции. Вот уже много лет. Мне переписать проще, чем постоянно помнить порядок операндов.
        • Эта строка перегружена, ее стоит упростить. Как уже писали, выделить три boolean const — самое то. В перегруженном коде легче допустить ошибку, статья — наглядный пример.
        • Код будет изменяться, и лучше предотвратить возможность ошибиться. Через полгода кто-то захочет добавить к условию дефисы-запятые и в моих силах сделать так, чтобы он не мог написать неправильно. Даже зеленый джун, писавший на Паскале.


    1. Aiditz
      23.10.2018 11:08
      #19269629
      +1

      Несмотря на знание приоритетов, читать выражения типа (a || b && c || d) крайне трудно.


      1. ads83
        24.10.2018 10:36
        #19274417
        +1

        Хотя выражение (a || b && c || d) уже неприятно читать, через N коммитов оно может стать
        (a || b && c || d && !e),
        а позже мутировать через
        (a || b && c || d && !e || (f && g))
        в (a || b && c || d && !e || (f && g || !a)).
        Через два года тимлид тратит четыре дня чтобы отловить плавающий баг, доходит до этой строчки, в гневе пишет git history… и видит коммиты от троих людей, двое из которых давно уволились, а init commit писал этот тимлид, когда был молодым.


        1. ads83
          24.10.2018 10:47
          #19274493
          +1

          И уже не у кого спросить, зачем появились скобки в хвосте выражения и нельзя по коду понять что хотели достичь последние два коммита, а контакты в телеграмме давно протухли. По старым адресам жили другие люди (тимлид на пару с битой проверял лично).
          Осталось последнее средство, к которому прибегают в крайнем случае. Несмотря на всю его мощь, мало кто рисковал использовать его. Но выбора не было. Тимлид закрылся в своем кабинете, сдул пыль с бумажной стопки и открыл ТЗ…
          конец бесплатного фрагмента


  1. AdAbsurdum
    22.10.2018 21:14
    #19267713
    +1

    А почему бы не сделать выделение метода для условий и назвать его нормально?
    IsDigit, IsUpperLetter, IsLowerLetter?


  1. bodqhrohro
    22.10.2018 22:41
    #19268037
    +1

    А почему статья на английском не на Хабре?


  1. Error1024
    23.10.2018 02:38
    #19268423

    А Delphi/Object Pascal просто не даст скомпилироваться такому коду, пока все скобки у логических операций не проставишь.


    1. mayorovp
      23.10.2018 07:46
      #19268669

      Конкретно эту ошибку система типов Паскаля не ловит.


  1. UncleJey
    23.10.2018 08:19
    #19268733
    -4

    Доброго дня! А вы только по c? Есть ли у вас angularJS анализатор? Или может подскажет кто?


    1. ads83
      23.10.2018 08:53
      #19268851
      +2

      Я далек от мира фронтенда вообще и JavaScript фреймворков в частности. Поэтому вполне серьезно спрашиваю: AngularJS действительно так развился, что для него нужен особенный анализатор, отличный от plain old JavaScript анализатора?
      Просветите, пожалуйста


      1. lany
        23.10.2018 16:52
        #19271697

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


        В IntelliJ IDEA вижу одну инспекцию: Angular|Empty Event Handler. Что бы это ни значило.


    1. Andrey2008 Автор
      23.10.2018 10:14
      #19269285
      +1

      PVS-Studio: C, C++, C#, Java (на подходе).

      Другие анализаторы: en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis


      1. MadHacker
        23.10.2018 13:11
        #19270463

        Хороший список.
        А для Pascal/Delphi не планируете сделать анализатор? Там в списке про такой язык упоминают в комментариях два универсальных анализатора. :)


        1. Andrey2008 Автор
          23.10.2018 14:15
          #19270911

          Нет. Быть может дальше (но не скоро): JavaScript или PHP.


  1. UncleJey
    23.10.2018 10:40
    #19269443

    а не анализировали doom например?


    1. Andrey2008 Автор
      23.10.2018 10:45
      #19269477

      Анализировали (ещё в 2011 году).

      Примечание: Обновляемый список статей, в которых мы рассказываем об ошибках, найденных с помощью PVS-Studio в открытых проектах.


  1. olekl
    23.10.2018 11:06
    #19269615

    Я вот сначала подумал, что там word стал отрицательным (0x0FF00 в 16-битном представлении будет отрицательным) и поэтому «больше-меньше» местами поменялось…


  1. chabapok
    23.10.2018 13:41
    #19270665
    +1

    А зачем люди препендят 0 к hex числам? 0x0ff21 вместо 0xff21?


    1. DelphiCowboy
      23.10.2018 13:57
      #19270789
      +1

      Действительно странно! Тогда было бы логичнее писать 0x00ff21, чтобы не прочитать в спешке ff с лишним ноликом как 0f f…


  1. hhba
    25.10.2018 10:31
    #19279035

    Статья хоть и короткая, но как обычно интересная! Занятно то, что даже такой лось, как я, нашел ошибку в коде буквально с первых прочитанных символов. Так что автор кода меня слегка пугает… И, да, я бы предпочел лучшее раскрытие условий в тексте программы (например так, как автор статьи предлагает, хотя варианты в комментариях есть и получше), но при этом скобок я бы не стеснялся. Как выше написали — в гробу я видал правила приоритета в тех или иных языках. При правильном раскрытии условий скобок будет минимум, но ровно там, где нужно.