IntelliJ IDEA содержит тысячи инспекций для Java-кода. Большинство из них работает как продвинутые регулярные выражения: по определённому шаблону они ищут фрагменты программы, которые выглядят как опечатки, избыточны, некрасивы или могут работать медленно. Но есть инспекция совсем другого рода. У неё несколько странное название: «Constant conditions & exceptions». В действительности она выполняет анализ потоков данных в Java-методах с помощью так называемого «символьного выполнения». В результате такого анализа могут обнаружиться некоторые подозрительные факты. Вот некоторые примеры таких фактов:
Эти факты не всегда свидетельствуют об ошибке. Например, иногда хочется добавить заведомо истинное условие для большей ясности кода:
Однако часто предупреждение от этой инспекции указывает на опечатку или говорит о том, что автор кода не совсем понял, что он написал. А главное, что эта инспекция способна обнаружить очень странные ошибки, которые не покрыты никакими шаблонами. Иногда даже мы, авторы IDE, удивляемся, глядя на код, в котором сработала инспекция «Constant conditions & exceptions».
С годами инспекция постоянно улучшается. Раньше она в основном отвечала за возможные проблемы с null и несоответствием типов. Позже в ней появился анализ наличия значения в Optional, анализ целочисленных диапазонов, анализ границ массивов. В предыдущем мажорном выпуске 2017.3 мы улучшили анализ целочисленных диапазонов и добавили базовую поддержку цепочек Stream API и Optional.
Каждое улучшение инспекции первым делом мы испытываем на коде IDEA Ultimate. Это крупный проект, содержащий десятки тысяч классов, которые пишутся второй десяток лет. К коду приложило руку не меньше сотни разработчиков с разным уровнем знаний и стилем. Поэтому всегда интересно прогнать «Constant conditions & exceptions» и обнаружить новый баг в древнем классе. Давайте посмотрим, что нового нашла инспекция после улучшений, сделанных для предстоящего релиза.
Одно из самых интересных улучшений анализа в 2018.1 — это отслеживание отношений между переменными. Ранее инспекция отслеживала только равенство и неравенство переменных. Например, условие
Во-первых, он также легко находит противоположный случай. И такая ошибка действительно обнаружилась в нашем коде обработки SQL-таблиц:
Да, метод начинается с условия, которое всегда истинно, и поэтому всегда возвращает пустой список, потому что значение переменной
Кстати, о длине массива. Приятным сюрпризом оказалось, что эта фича сама подружилась с уже имевшимися проверками длин массивов. Мы совершенно не ожидали обнаружить ошибок, подобных вот этой, найденной в Groovy-плагине:
Теперь при чтении элемента массива инспекция уже знает, что
Ещё одна проблема, связанная с границами массива, обнаружилась в коде одной Java-инспекции, которая обрабатывает блоки catch:
Как видно, граница значений
В новой версии мы иногда отслеживаем значения из инициализаторов массива. Это позволило нам обнаружить избыточный код вроде такого:
Давным давно одноэлементный массив
Теперь мы считаем, что булевы методы без аргументов, имя которых начинается с “is”, возвращают один и тот же результат, если были дважды вызваны с одним и тем же квалификатором. Да, это эвристика, она не обязательно верна. Но если она стала причиной ложного срабатывания в вашем коде, задумайтесь. Возможно, это знак, что код труден для понимания не только для IDE, но и для ваших коллег. Зато такая эвристика позволяет найти настоящие баги. Например, такой код обнаружился при обработке CSS селекторов:
Хотя квалификаторы здесь различны, выше им присвоили одно и то же значение. Поэтому анализатор предполагает, что
Также в 2018.1 инспекция выдаёт новое предупреждение. Если переменной присваивается значение, и то же самое значение она уже имела в этом месте всегда, мы это подсвечиваем. Опять же не всегда это именно ошибка, но присваивание заведомо избыточно. Настоящие ошибки с помощью этого предупреждения тоже нашлись. Например, это фрагмент юнит-теста, проверяющего форматирование HTML:
Проблема тремя строчками выше предупреждения: на конце не должно быть “
Также это предупреждение вскрыло дубликаты в цепочках инициализации полей или элементов массива:
Возможно, эта строчка просто избыточна, но может быть и имелось в виду что-то другое. Так или иначе это очевидно ошибка.
Небольшое улучшение обработки
Если левая часть истинна, то правая не проверяется. Если же левая ложна, то
Наконец, мы улучшили обработку Stream API. В частности, она теперь работает и для незавершённых цепочек (то есть не заканчивающихся терминальной операцией). Ошибок в нашем коде это улучшение не выявило, но нашлись некоторые избыточные проверки. Например, такой метод имеется в обработке результатов code coverage:
Для метода
Инспекция “Constant conditions & exceptions” на удивление часто находит ошибки, которые прячутся в больших проектах годами. Ещё больше она помогает, когда вы пишете новый, непроверенный код. Поначалу некоторые предупреждения этой инспекции могут раздражать, но со временем понимаешь, как с ними жить дружно.
Мы уже начали EAP-программу 2018.1, поэтому вы можете испытать новые возможности прямо сейчас. Эта инспекция постоянно улучшается, исчезают ложные сработки и добавляются полезные. У нас ещё много идей, что можно в ней улучшить, так что следите за новостями.
- Разыменование ссылки может привести к
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
либо меньше длины массива myAncestorRefs
, либо больше, либо равно ей. Последующие десять строчек кода никогда не выполняются. Очевидно тут опечатка: имелось в виду idx < 0
вместо idx < myAncestorRefs.length
.Кстати, о длине массива. Приятным сюрпризом оказалось, что эта фича сама подружилась с уже имевшимися проверками длин массивов. Мы совершенно не ожидали обнаружить ошибок, подобных вот этой, найденной в Groovy-плагине:
Теперь при чтении элемента массива инспекция уже знает, что
i
в данном месте всегда больше, чем количество элементов в initializers
. Поэтому исключение ArrayIndexOutOfBoundsException
неизбежно, если условие когда-либо выполнится. Думаю, на самом деле оно никогда не выполнялось, иначе бы исключение заметили. Конечно же условие перевернули по ошибке.Ещё одна проблема, связанная с границами массива, обнаружилась в коде одной Java-инспекции, которая обрабатывает блоки catch:
Как видно, граница значений
index
проверяется после того как эта переменная используется для доступа к элементу массива. Условие не может быть ложным: если оно ложно, то мы бы уже улетели из этого кода с ArrayIndexOutOfBoundsException
.В новой версии мы иногда отслеживаем значения из инициализаторов массива. Это позволило нам обнаружить избыточный код вроде такого:
Давным давно одноэлементный массив
result
использовался внутри лямбды command
, чтобы зафиксировать необходимость активации редактора. Со временем активация редактора стала не нужна, но ни одна существующая инспекция не замечала, что условие избыточно. Теперь мы это видим.Теперь мы считаем, что булевы методы без аргументов, имя которых начинается с “is”, возвращают один и тот же результат, если были дважды вызваны с одним и тем же квалификатором. Да, это эвристика, она не обязательно верна. Но если она стала причиной ложного срабатывания в вашем коде, задумайтесь. Возможно, это знак, что код труден для понимания не только для IDE, но и для ваших коллег. Зато такая эвристика позволяет найти настоящие баги. Например, такой код обнаружился при обработке CSS селекторов:
Хотя квалификаторы здесь различны, выше им присвоили одно и то же значение. Поэтому анализатор предполагает, что
isUniversalSelector
вернёт одинаковый результат. И это действительно так! Опечатка здесь не в условии, а строчкой выше: должно быть, конечно, remoteSelectorElement = (CssSimpleSelector)remoteSelectorElements[j]
.Также в 2018.1 инспекция выдаёт новое предупреждение. Если переменной присваивается значение, и то же самое значение она уже имела в этом месте всегда, мы это подсвечиваем. Опять же не всегда это именно ошибка, но присваивание заведомо избыточно. Настоящие ошибки с помощью этого предупреждения тоже нашлись. Например, это фрагмент юнит-теста, проверяющего форматирование HTML:
Проблема тремя строчками выше предупреждения: на конце не должно быть “
= 2
”. В результате размер отступа не восстанавливается после теста, как ожидалось.Также это предупреждение вскрыло дубликаты в цепочках инициализации полей или элементов массива:
Возможно, эта строчка просто избыточна, но может быть и имелось в виду что-то другое. Так или иначе это очевидно ошибка.
Небольшое улучшение обработки
this
вскрыло ошибку в коде, обрабатывающем контрол с вкладками:Если левая часть истинна, то правая не проверяется. Если же левая ложна, то
component
равен this
, а this
точно не null, значит и component
не null. Поэтому всё условие всегда истинно. Очень частая ошибка, когда используют ||
вместо &&
или наоборот.Наконец, мы улучшили обработку Stream API. В частности, она теперь работает и для незавершённых цепочек (то есть не заканчивающихся терминальной операцией). Ошибок в нашем коде это улучшение не выявило, но нашлись некоторые избыточные проверки. Например, такой метод имеется в обработке результатов code coverage:
Для метода
Pair.create
автоматически выводится аннотация @NotNull
, поэтому условие x != null
избыточно. Инспекция в курсе, что x
имеет то же значение, которое вернула предыдущая лямбда.Инспекция “Constant conditions & exceptions” на удивление часто находит ошибки, которые прячутся в больших проектах годами. Ещё больше она помогает, когда вы пишете новый, непроверенный код. Поначалу некоторые предупреждения этой инспекции могут раздражать, но со временем понимаешь, как с ними жить дружно.
Мы уже начали EAP-программу 2018.1, поэтому вы можете испытать новые возможности прямо сейчас. Эта инспекция постоянно улучшается, исчезают ложные сработки и добавляются полезные. У нас ещё много идей, что можно в ней улучшить, так что следите за новостями.
artspb
А вы сообщили разработчикам? А патч отправили? Какую версию этого проекта вы проверяли? Master? Так надо 173! А вы проверяли проект IDEA Community? А что так МАЛО? А есть версия IntelliJ IDEA под Linux? Вы проверяли IDEA с помощью IDEA? Где статья о результатах проверки?
lany Автор
Не распознают люди сарказм, вот и минусуют, прости их :-)
artspb
Я знаю, Тагир, я знаю :) И в профиль пользователя тоже не любят заглядывать.
dougrinch
Распознают, и в профиль заглядывают, и даже видят там уже поставленный плюс в карму. Но конкретно этот коммент, что называется, «слишком толстый». И парсится уже не как шутка, а как глупое отвлечение от темы.
p.s. Сам пост отличный, спасибо!
lany Автор
Честно говоря, я больше опасался комментариев вроде «У… да ваша IDEA — глюкавище, как можно было такие баги допустить».
TheKnight
У… да ваша IDEA — глюкавище, как можно было такие баги допустить
Все, страшное случилось, дальше боятся смысла нет :)
a__v__k
Есть ли возможность пометить, что именно проверяют свои функции-проверки, как то
lany Автор
Есть контракты, но они довольно ограничены. Здесь можно написать
@Contract("null -> false")
, и это будет учитываться. А вот информация о длине массива во внешний метод не попадёт.a__v__k
Спасибо. Попробую.
lany Автор
Если метод статический, скорее всего контракт сам выведется. Посмотрите по ctrl+q.
Hixon10
Спасибо за интересный рассказ!
Один комментарий — очень плохо видно скриншоты кода. Понятно, что тут хочется показать реакцию IDEA, но факт — есть факт.
lany Автор
Да, со скриншотами не очень получилось. Шрифт явно не подходящий. В следующий раз учту :-)
ggo
IDE — хорошо, а как запустить инспекции в безлюдном режиме (в рамках CI) и получить отчет о найденных ошибках/предупреждениях? a la standalone static code analyzer
TheKnight
+1 к этому комментарию :) Желательно еще плагин, позволяющий автоматически постить результаты проверки в ПР на Github/Bitbucket/etc :)
lany Автор
https://www.jetbrains.com/help/idea/running-inspections-offline.html
А если у вас правильный CI, то всё будет работать из коробки :-)