IntelliJ IDEA содержит тысячи инспекций для Java-кода. Большинство из них работает как продвинутые регулярные выражения: по определённому шаблону они ищут фрагменты программы, которые выглядят как опечатки, избыточны, некрасивы или могут работать медленно. Но есть инспекция совсем другого рода. У неё несколько странное название: «Constant conditions & exceptions». В действительности она выполняет анализ потоков данных в Java-методах с помощью так называемого «символьного выполнения». В результате такого анализа могут обнаружиться некоторые подозрительные факты. Вот некоторые примеры таких фактов:

  • Разыменование ссылки может привести к NullPointerException
  • Условие всегда истинно или ложно
  • Индекс массива всегда за пределами допустимых границ
  • Приведение типа может привести к ClassCastException

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

if (obj == null) {
  …
} 
else if (obj != null) { // Предупреждение: 'obj != null' всегда истинно
  …
}

Однако часто предупреждение от этой инспекции указывает на опечатку или говорит о том, что автор кода не совсем понял, что он написал. А главное, что эта инспекция способна обнаружить очень странные ошибки, которые не покрыты никакими шаблонами. Иногда даже мы, авторы IDE, удивляемся, глядя на код, в котором сработала инспекция «Constant conditions & exceptions».

С годами инспекция постоянно улучшается. Раньше она в основном отвечала за возможные проблемы с null и несоответствием типов. Позже в ней появился анализ наличия значения в Optional, анализ целочисленных диапазонов, анализ границ массивов. В предыдущем мажорном выпуске 2017.3 мы улучшили анализ целочисленных диапазонов и добавили базовую поддержку цепочек Stream API и Optional.

Каждое улучшение инспекции первым делом мы испытываем на коде IDEA Ultimate. Это крупный проект, содержащий десятки тысяч классов, которые пишутся второй десяток лет. К коду приложило руку не меньше сотни разработчиков с разным уровнем знаний и стилем. Поэтому всегда интересно прогнать «Constant conditions & exceptions» и обнаружить новый баг в древнем классе. Давайте посмотрим, что нового нашла инспекция после улучшений, сделанных для предстоящего релиза.

Одно из самых интересных улучшений анализа в 2018.1 — это отслеживание отношений между переменными. Ранее инспекция отслеживала только равенство и неравенство переменных. Например, условие if(a == b && a != b) помечалось как всегда ложное. Когда мы стали отслеживать целочисленные диапазоны, появились новые предупреждения в условиях вроде if(a > 0 && a <= 0), потому что левая часть истинна, если значение a лежит в диапазоне [1..MAX_VALUE], а правая часть — если в диапазоне [MIN_VALUE..0], и эти диапазоны не пересекаются. Однако до сих пор не было предупреждения на условии if(a > b && a <= b), потому что мы здесь мы ничего не знаем о диапазонах. Теперь мы следим, какое значение больше, даже если они оба неизвестны, поэтому в 2018.1 это условие тоже будет подсвечено. Кажется, что такую тривиальную ошибку наверняка заметит сам программист, но не забывайте, что анализ потоков данных не просто ищет шаблоны.

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

idx &lt; myAncestorRefs.length || idx &gt;= myAncestorRefs.length

Да, метод начинается с условия, которое всегда истинно, и поэтому всегда возвращает пустой список, потому что значение переменной idx либо меньше длины массива myAncestorRefs, либо больше, либо равно ей. Последующие десять строчек кода никогда не выполняются. Очевидно тут опечатка: имелось в виду idx < 0 вместо idx < myAncestorRefs.length.

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

if (initializers.length &lt; i) return initializers[i];

Теперь при чтении элемента массива инспекция уже знает, что i в данном месте всегда больше, чем количество элементов в initializers. Поэтому исключение ArrayIndexOutOfBoundsException неизбежно, если условие когда-либо выполнится. Думаю, на самом деле оно никогда не выполнялось, иначе бы исключение заметили. Конечно же условие перевернули по ошибке.

Ещё одна проблема, связанная с границами массива, обнаружилась в коде одной Java-инспекции, которая обрабатывает блоки catch:

while (catchSections[index] != catchSection &amp;&amp; index &lt; catchSections.length)

Как видно, граница значений index проверяется после того как эта переменная используется для доступа к элементу массива. Условие не может быть ложным: если оно ложно, то мы бы уже улетели из этого кода с ArrayIndexOutOfBoundsException.

В новой версии мы иногда отслеживаем значения из инициализаторов массива. Это позволило нам обнаружить избыточный код вроде такого:

boolean[] result = new boolean[] {false}; &lt;...&gt; if (result[0])

Давным давно одноэлементный массив result использовался внутри лямбды command, чтобы зафиксировать необходимость активации редактора. Со временем активация редактора стала не нужна, но ни одна существующая инспекция не замечала, что условие избыточно. Теперь мы это видим.

Теперь мы считаем, что булевы методы без аргументов, имя которых начинается с “is”, возвращают один и тот же результат, если были дважды вызваны с одним и тем же квалификатором. Да, это эвристика, она не обязательно верна. Но если она стала причиной ложного срабатывания в вашем коде, задумайтесь. Возможно, это знак, что код труден для понимания не только для IDE, но и для ваших коллег. Зато такая эвристика позволяет найти настоящие баги. Например, такой код обнаружился при обработке CSS селекторов:

localSelectorElement = (CssSimpleSelector)localElement;remoteSelectorElement = (CssSimpleSelector)localSelectorElement;if(localSelectorElement.isUniversalSelector() != remoteSelectorElement.isUniversalSelector())

Хотя квалификаторы здесь различны, выше им присвоили одно и то же значение. Поэтому анализатор предполагает, что isUniversalSelector вернёт одинаковый результат. И это действительно так! Опечатка здесь не в условии, а строчкой выше: должно быть, конечно, remoteSelectorElement = (CssSimpleSelector)remoteSelectorElements[j].

Также в 2018.1 инспекция выдаёт новое предупреждение. Если переменной присваивается значение, и то же самое значение она уже имела в этом месте всегда, мы это подсвечиваем. Опять же не всегда это именно ошибка, но присваивание заведомо избыточно. Настоящие ошибки с помощью этого предупреждения тоже нашлись. Например, это фрагмент юнит-теста, проверяющего форматирование HTML:

int indentSize = htmlIndentOptions.INDENT_SIZE = 2;

Проблема тремя строчками выше предупреждения: на конце не должно быть “= 2”. В результате размер отступа не восстанавливается после теста, как ожидалось.

Также это предупреждение вскрыло дубликаты в цепочках инициализации полей или элементов массива:

defaults[CONSTANT_Int] = ints;...defaults[CONSTANT_Int] = ints;

Возможно, эта строчка просто избыточна, но может быть и имелось в виду что-то другое. Так или иначе это очевидно ошибка.

Небольшое улучшение обработки this вскрыло ошибку в коде, обрабатывающем контрол с вкладками:

while (component != this || component != null)

Если левая часть истинна, то правая не проверяется. Если же левая ложна, то component равен this, а this точно не null, значит и component не null. Поэтому всё условие всегда истинно. Очень частая ошибка, когда используют || вместо && или наоборот.

Наконец, мы улучшили обработку Stream API. В частности, она теперь работает и для незавершённых цепочек (то есть не заканчивающихся терминальной операцией). Ошибок в нашем коде это улучшение не выявило, но нашлись некоторые избыточные проверки. Например, такой метод имеется в обработке результатов code coverage:

mapToObj(idx -&gt; Pair.create(...)).filter((x) -&gt; x != null &amp;&amp; ...); интересно, а слепые читают эту статью? Я тут альтернативный текст пишу ко всем картинкам, от этого польза есть или нету?

Для метода Pair.create автоматически выводится аннотация @NotNull, поэтому условие x != null избыточно. Инспекция в курсе, что x имеет то же значение, которое вернула предыдущая лямбда.

Инспекция “Constant conditions & exceptions” на удивление часто находит ошибки, которые прячутся в больших проектах годами. Ещё больше она помогает, когда вы пишете новый, непроверенный код. Поначалу некоторые предупреждения этой инспекции могут раздражать, но со временем понимаешь, как с ними жить дружно.

Мы уже начали EAP-программу 2018.1, поэтому вы можете испытать новые возможности прямо сейчас. Эта инспекция постоянно улучшается, исчезают ложные сработки и добавляются полезные. У нас ещё много идей, что можно в ней улучшить, так что следите за новостями.

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


  1. artspb
    24.01.2018 16:47

    А вы сообщили разработчикам? А патч отправили? Какую версию этого проекта вы проверяли? Master? Так надо 173! А вы проверяли проект IDEA Community? А что так МАЛО? А есть версия IntelliJ IDEA под Linux? Вы проверяли IDEA с помощью IDEA? Где статья о результатах проверки?


    1. lany Автор
      24.01.2018 17:31
      +1

      Не распознают люди сарказм, вот и минусуют, прости их :-)


      1. artspb
        24.01.2018 17:38

        Я знаю, Тагир, я знаю :) И в профиль пользователя тоже не любят заглядывать.


      1. dougrinch
        24.01.2018 17:57

        Распознают, и в профиль заглядывают, и даже видят там уже поставленный плюс в карму. Но конкретно этот коммент, что называется, «слишком толстый». И парсится уже не как шутка, а как глупое отвлечение от темы.

        p.s. Сам пост отличный, спасибо!


        1. lany Автор
          25.01.2018 10:25

          Честно говоря, я больше опасался комментариев вроде «У… да ваша IDEA — глюкавище, как можно было такие баги допустить».


          1. TheKnight
            25.01.2018 16:55
            +1

            У… да ваша IDEA — глюкавище, как можно было такие баги допустить
            Все, страшное случилось, дальше боятся смысла нет :)

            Для фанатов Дюны
            image


  1. a__v__k
    24.01.2018 17:31

    Есть ли возможность пометить, что именно проверяют свои функции-проверки, как то

    boolean isNotEmpty(Object[] arr) { return arr != null && arr.length > 0; }
    


    1. lany Автор
      24.01.2018 17:34

      Есть контракты, но они довольно ограничены. Здесь можно написать @Contract("null -> false"), и это будет учитываться. А вот информация о длине массива во внешний метод не попадёт.


      1. a__v__k
        24.01.2018 17:49

        Спасибо. Попробую.


        1. lany Автор
          24.01.2018 18:11

          Если метод статический, скорее всего контракт сам выведется. Посмотрите по ctrl+q.


  1. Hixon10
    24.01.2018 22:29
    +1

    Спасибо за интересный рассказ!

    Один комментарий — очень плохо видно скриншоты кода. Понятно, что тут хочется показать реакцию IDEA, но факт — есть факт.


    1. lany Автор
      25.01.2018 06:16

      Да, со скриншотами не очень получилось. Шрифт явно не подходящий. В следующий раз учту :-)


  1. ggo
    25.01.2018 09:45

    IDE — хорошо, а как запустить инспекции в безлюдном режиме (в рамках CI) и получить отчет о найденных ошибках/предупреждениях? a la standalone static code analyzer


    1. TheKnight
      25.01.2018 09:51

      +1 к этому комментарию :) Желательно еще плагин, позволяющий автоматически постить результаты проверки в ПР на Github/Bitbucket/etc :)


    1. lany Автор
      25.01.2018 10:23

      https://www.jetbrains.com/help/idea/running-inspections-offline.html


      А если у вас правильный CI, то всё будет работать из коробки :-)