Главная » Хабрахабр » PVS-Studio для Java

PVS-Studio для Java

PVS-Studio для Java

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

PVS-Studio

Для Java разработчиков, которые ранее не слышали об инструменте PVS-Studio, приведу совсем краткое его описание.

Работает в среде Windows, Linux и macOS. PVS-Studio — это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C++, C# и Java.

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

Начало

Я бы мог придумать умную историю, как мы два года размышляли о том, какой следующий язык поддержать в PVS-Studio. О том, что Java — это разумный выбор, основанный на высокой популярности этого языка и так далее.

Да, мы размышляли, в какую сторону дальше развивать анализатор PVS-Studio. Однако как это бывает в жизни, всё решил не глубокий анализ, а эксперимент :). Причём мы склонялись именно к языку Java, но окончательный выбор так ещё и не был сделан. Рассматривались такие языки программирования, как: Java, PHP, Python, JavaScript, IBM RPG. Тех, у кого взгляд застрял на незнакомом IBM RPG, отсылаю к вот этой заметке, из которой всё станет ясно.

И наткнулся на несколько проектов для разбора Java кода. В конце 2017 года коллега Егор Бредихин посмотрел, какие есть готовые библиотеки разбора кода (проще говоря — парсеры) под интересные нам новые направления. Более того, стало понятно, что мы сможем использовать в Java анализаторе некоторые механизмы C++ анализатора с помощью SWIG. На основе Spoon, ему довольно быстро удалось сделать прототип анализатора с парой диагностик. Мы посмотрели на то, что получилось, и поняли, что наш следующий анализатор будет для Java.

Как именно шла разработка он описал в статье "Разработка нового статического анализатора: PVS-Studio Java". Спасибо Егору за его начинание и активную работу, проделанную им над Java анализатором.

Конкуренты?

В мире существует множество бесплатных и коммерческих статических анализаторов кода для Java. Перечислять их все в статье не имеет смысла, и я просто оставлю ссылку на "List of tools for static code analysis" (смотрите раздел Java и Multi-language).

Однако я знаю, что в первую очередь нас спросят про IntelliJ IDEA, FindBugs и SonarQube (SonarJava).

IntelliJ IDEA

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

Мы же свободны в том, что можем делать с нашим анализатором. Статический анализ в IntelliJ IDEA – это, в первую очередь, одна из фишек среды разработки, что накладывает на него определённые ограничения. Быстрая и глубокая поддержка — наше конкурентное преимущество. Например, мы можем быстро адаптировать анализатор под конкретные нужды заказчика. Наши клиенты напрямую общаются непосредственно с программистами, разрабатывающими ту или иную часть PVS-Studio.

Это интеграция с SonarQube. В PVS-Studio много возможностей по интеграции его в цикл разработки больших старых проектов. PVS-Studio встраивается в процесс непрерывной интеграции. Это массовое подавление сообщений анализатора, что позволяет сразу начать использовать анализатор в большом проекте для отслеживания ошибок только в новом или изменённом коде. Думаю, именно эти и другие возможности помогут нашему анализатору найти место под солнцем в Java мире.

FindBugs

Но его следует вспомнить по той причине, что это, пожалуй, наиболее известный бесплатный статический анализатор Java кода. Проект FindBugs заброшен.

Однако он менее популярен, и что с ним будет, тоже пока не совсем понятно. Преемником FindBugs можно назвать проект SpotBugs.

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

S. P. Кстати, теперь PVS-Studio также можно использовать бесплатно при работе с открытыми проектами.

SonarQube (SonarJava)

PVS-Studio интегрируется с SonarQube, что позволяет разработчикам находить большее количество ошибок и потенциальных уязвимостей в своих проектах. Мы считаем, что не конкурируем с SonarQube, а дополняем его. Как интегрировать в SonarQube инструмент PVS-Studio и другие анализаторы, мы регулярно рассказываем на мастер-классах, которые проводим в рамках различных конференций (пример).

Как запустить PVS-Studio для Java

Мы сделали доступными для пользователей самые популярные способы интеграции анализатора в сборочную систему:

  • Плагин для Maven;
  • Плагин для Gradle;
  • Плагин для IntelliJ IDEA

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

Подробную информацию о всех способах запуска анализатора вы можете найти на странице документации "Как запустить PVS-Studio Java".

Мы не могли обойти стороной платформу контроля качества кода SonarQube, так популярную среди Java разработчиков, поэтому добавили поддержку языка Java в наш плагин для SonarQube.

Дальнейшие планы

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

  • Создание новых диагностик и доработка существующих;
  • Развитие Dataflow-анализа;
  • Повышение надёжности и удобства использования.

Возможно, мы найдём время адаптировать плагин IntelliJ IDEA для CLion. Привет C++ разработчикам, читающим про анализатор Java 🙂

Примеры ошибок, найденных в открытых проектах

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

Поэтому у меня просто нет выхода, и я начну именно с этих проектов. Однако я сразу предвижу вопросы, а сможем ли мы найти что-то в таких проектах как IntelliJ IDEA, FindBugs и так далее. Итак, я решил бегло проверить и выписать несколько интересных примеров ошибок из следующих проектов:

  • IntelliJ IDEA Community Edition. Думаю, не нужно объяснять, почему был выбран этот проект :).
  • SpotBugs. Как я уже писал ранее, проект FindBugs не развивается. Поэтому заглянем в проект SpotBugs, который является преемником FindBugs. SpotBugs — это классический статический анализатор Java-кода.
  • Что-то из проектов компании SonarSource, которая разрабатывает программное обеспечение для непрерывного контроля качества кода. Заглянем в проект SonarQube и SonarJava.

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

Второго шанса написать что-то про них у меня не будет. Несмотря на всё это, мне придётся начать именно с этих проектов. Например, я знаю, что Тагир Валеев (lany), один из разработчиков JetBrains, занимающийся статическим анализатором кода IntelliJ IDEA, в тот момент, пока я пишу статью, уже во всю играется с Beta-версией PVS-Studio. Я уверен, что после выхода релиза PVS-Studio для Java, разработчики перечисленных проектов возьмут PVS-Studio на вооружение и начнут его использовать для регулярных или, по крайней мере, для периодических проверок своего кода. Спасибо, Тагир! Он написал нам уже около 15 писем с баг-репортами и рекомендациями.

Сейчас моя задача показать, что анализатор PVS-Studio для Java появился не зря и сможет пополнить линейку других инструментов, предназначенных для улучшения качества кода. К счастью, мне не требуется найти как можно больше ошибок в одном конкретном проекте. По возможности я старался выписывать ошибки разного типа. Я просто бегло просмотрел отчёты анализатора и выписал несколько ошибок, которые показались мне интересными. Давайте посмотрим, что получилось.

IntelliJ IDEA, целочисленное деление

private static boolean checkSentenceCapitalization(@NotNull String value) { List<String> words = StringUtil.split(value, " "); .... int capitalized = 1; .... return capitalized / words.size() < 0.2; // allow reasonable amount of // capitalized words
}

Предупреждение PVS-Studio: V6011 [CWE-682] The '0.2' literal of the 'double' type is compared to a value of the 'int' type. TitleCapitalizationInspection.java 169

На самом деле, проверка не работает, так как происходит целочисленное деление. По задумке, функция должна возвращать истину, если менее 20% слов начинаются с заглавной буквы. В результате деления можно получить только два значения: 0 или 1.

Во всех остальных случаях при делении будет получаться 0, и функция будет возвращать истинное значение. Функция вернёт ложное значение, только если все слова будут начинаться с заглавной буквы.

IntelliJ IDEA, подозрительный цикл

public int findPreviousIndex(int current) } ....
}

Предупреждение PVS-Studio: V6007 [CWE-571] Expression 'index >= 0' is always true. Updater.java 184

Оно выполняется только в том случае, если значение переменной count больше 0. Вначале посмотрите на условие (0 <= current && current < count).

Теперь посмотрим на цикл:

for (int index = count - 1; index >= 0; index++)

Переменная index инициализируется выражением count — 1. Так как переменная count больше 0, то начальное значение переменной index всегда больше или равно 0. Получается, что цикл будет выполняться до тех пор, пока не произойдёт переполнение переменной index.

Скорее всего, это просто опечатка и должен выполняться не инкремент, а декремент переменной:

for (int index = count - 1; index >= 0; index--)

IntelliJ IDEA, Copy-Paste

@NonNls public static final String BEFORE_STR_OLD = "before:";
@NonNls public static final String AFTER_STR_OLD = "after:"; private static boolean isBeforeOrAfterKeyword(String str, boolean trimKeyword) { return (trimKeyword ? LoadingOrder.BEFORE_STR.trim() : LoadingOrder.BEFORE_STR).equalsIgnoreCase(str) || (trimKeyword ? LoadingOrder.AFTER_STR.trim() : LoadingOrder.AFTER_STR).equalsIgnoreCase(str) || LoadingOrder.BEFORE_STR_OLD.equalsIgnoreCase(str) || // <= LoadingOrder.BEFORE_STR_OLD.equalsIgnoreCase(str); // <=
}

Предупреждение PVS-Studio: V6001 [CWE-570] There are identical sub-expressions 'LoadingOrder.BEFORE_STR_OLD.equalsIgnoreCase(str)' to the left and to the right of the '||' operator. Check lines: 127, 128. ExtensionOrderConverter.java 127

Программист поторопился и, размножив строчку кода, забыл её исправить. Старый добрый эффект последней строки. Скорее всего, одно из сравнений должно быть с AFTER_STR_OLD. В результате, дважды строка str сравнивается с BEFORE_STR_OLD.

IntelliJ IDEA, опечатка

public synchronized boolean isIdentifier(@NotNull String name, final Project project) { if (!StringUtil.startsWithChar(name,'\'') && !StringUtil.startsWithChar(name,'\"')) { name = "\"" + name; } if (!StringUtil.endsWithChar(name,'"') && !StringUtil.endsWithChar(name,'\"')) { name += "\""; } ....
}

Предупреждение PVS-Studio: V6001 [CWE-571] There are identical sub-expressions '!StringUtil.endsWithChar(name,'"')' to the left and to the right of the '&&' operator. JsonNamesValidator.java 27

Если это не так, то двойные кавычки добавляются автоматически. Данный фрагмент кода проверяет, что имя взято в одинарные или двойные кавычки.

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

Имя

'Abcd'

из-за добавления лишних двойных кавычек превратится в:

'Abcd'"

IntelliJ IDEA, неправильная защита от выхода за границу массива

static Context parse(....) { .... for (int i = offset; i < endOffset; i++) { char c = text.charAt(i); if (c == '<' && i < endOffset && text.charAt(i + 1) == '/' && startTag != null && CharArrayUtil.regionMatches(text, i + 2, endOffset, startTag)) { endTagStartOffset = i; break; } } ....
}

Предупреждение PVS-Studio: V6007 [CWE-571] Expression 'i

Переменная i и так всегда меньше endOffset, что следует из условия выполнения цикла. Подвыражение i < endOffset в условии оператора if не имеет смысла.

Скорее всего, программист хотел защититься от выхода за границу строки при вызове функций:

  • text.charAt(i + 1)
  • CharArrayUtil.regionMatches(text, i + 2, endOffset, startTag)

В этом случае подвыражение для проверки индекса должно быть таким: i < endOffset — 2.

IntelliJ IDEA, повторяющаяся проверка

public static String generateWarningMessage(....)
{ .... if (buffer.length() > 0) { if (buffer.length() > 0) { buffer.append(" ").append( IdeBundle.message("prompt.delete.and")).append(" "); } } ....
}

Предупреждение PVS-Studio: V6007 [CWE-571] Expression 'buffer.length() > 0' is always true. DeleteUtil.java 62

Это может быть как безобидным избыточным кодом, так и серьезной ошибкой.

Вторую проверку можно просто удалить. Если проверка-дубликат появилась случайно, например, в ходе рефакторинга, то ничего плохого в этом нет.

Вторая проверка должна быть совсем другой и код ведёт себя не так, как задумано. Но возможен и другой сценарий. Тогда это настоящая ошибка.

Причём часто видно, что это не ошибка. Примечание. Кстати, разных избыточных проверок находится очень много. Для пояснения приведу вот такой пример, также взятый из IntelliJ IDEA: Однако и назвать сообщения анализатора ложными срабатываниями тоже нельзя.

private static boolean isMultiline(PsiElement element) { String text = element.getText(); return text.contains("\n") || text.contains("\r") || text.contains("\r\n");
}

Анализатор говорит, что функция text.contains("\r\n") всегда возвращает ложь. И действительно, если не найден символ "\n" и "\r", то и нет смысла искать "\r\n". Это не ошибка, и код плох только тем, что работает чуть медленнее, выполняя бессмысленный поиск подстроки.

Я при написании статей, как правило, просто не обращаю внимания на подобный код. Как быть с подобным кодом, в каждом конкретном случае уже решать программистам.

IntelliJ IDEA, что-то не так

public boolean satisfiedBy(@NotNull PsiElement element) { .... @NonNls final String text = expression.getText().replaceAll("_", ""); if (text == null || text.length() < 2) { return false; } if ("0".equals(text) || "0L".equals(text) || "0l".equals(text)) { return false; } return text.charAt(0) == '0';
}

Предупреждение PVS-Studio: V6007 [CWE-570] Expression '«0».equals(text)' is always false. ConvertIntegerToDecimalPredicate.java 46

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

Если это не так, то функция возвращает false. В начале проверятся, что строка должна содержать не менее двух символов.

Она бессмысленна, так как строка не может содержать только один символ. Далее следует проверка «0».equals(text).

В общем, что-то здесь не так и код следует поправить.

SpotBugs (преемник FindBugs), ошибка ограничения по количеству итераций

public static String getXMLType(@WillNotClose InputStream in) throws IOException
{ .... String s; int count = 0; while (count < 4) { s = r.readLine(); if (s == null) { break; } Matcher m = tag.matcher(s); if (m.find()) { return m.group(1); } } throw new IOException("Didn't find xml tag"); ....
}

Предупреждение PVS-Studio: V6007 [CWE-571] Expression 'count

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

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

SpotBugs (преемник FindBugs), затирание значения

private void reportBug() { int priority = LOW_PRIORITY; String pattern = "NS_NON_SHORT_CIRCUIT"; if (sawDangerOld) { if (sawNullTestVeryOld) { priority = HIGH_PRIORITY; // <= } if (sawMethodCallOld || sawNumericTestVeryOld && sawArrayDangerOld) { priority = HIGH_PRIORITY; // <= pattern = "NS_DANGEROUS_NON_SHORT_CIRCUIT"; } else { priority = NORMAL_PRIORITY; // <= } } bugAccumulator.accumulateBug( new BugInstance(this, pattern, priority).addClassAndMethod(this), this);
}

Предупреждение PVS-Studio: V6021 [CWE-563] The value is assigned to the 'priority' variable but is not used. FindNonShortCircuit.java 197

Однако это не играет никакой роли. Значение переменной priority выставляется в зависимости от значения переменной sawNullTestVeryOld. Явная ошибка в логике работы функции. Далее переменной priority в любом случае будет присвоено другое значение.

SonarQube, Copy-Paste

public class RuleDto { .... private final RuleDefinitionDto definition; private final RuleMetadataDto metadata; .... private void setUpdatedAtFromDefinition(@Nullable Long updatedAt) { if (updatedAt != null && updatedAt > definition.getUpdatedAt()) { setUpdatedAt(updatedAt); } } private void setUpdatedAtFromMetadata(@Nullable Long updatedAt) { if (updatedAt != null && updatedAt > definition.getUpdatedAt()) { setUpdatedAt(updatedAt); } } ....
}

PVS-Studio: V6032 It is odd that the body of method 'setUpdatedAtFromDefinition' is fully equivalent to the body of another method 'setUpdatedAtFromMetadata'. Check lines: 396, 405. RuleDto.java 396

Скорее всего, должно использоваться поле metadata. В методе setUpdatedAtFromMetadata используется поле definition. Это очень похоже на последствия неудачного Copy-Paste.

SonarJava, дубликаты при инициализации Map

private final Map<JavaPunctuator, Tree.Kind> assignmentOperators = Maps.newEnumMap(JavaPunctuator.class); public KindMaps() { .... assignmentOperators.put(JavaPunctuator.PLUSEQU, Tree.Kind.PLUS_ASSIGNMENT); .... assignmentOperators.put(JavaPunctuator.PLUSEQU, Tree.Kind.PLUS_ASSIGNMENT); ....
}

Предупреждение PVS-Studio: V6033 [CWE-462] An item with the same key 'JavaPunctuator.PLUSEQU' has already been added. Check lines: 104, 100. KindMaps.java 104

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

Заключение

Да какое тут может быть заключение?! Приглашаю всех, не откладывая, скачать PVS-Studio и попробовать проверить свои рабочие проекты на языке Java! Скачать PVS-Studio.

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

PVS-Studio for Java. Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov.


Оставить комментарий

Ваш email нигде не будет показан
Обязательные для заполнения поля помечены *

*

x

Ещё Hi-Tech Интересное!

Дата-центры на выбор: Лондон, Москва, Цюрих, Санкт-Петербург

Отчасти санкции, отчасти рост технологического бизнеса, отчасти рост дохода этого самого бизнеса сформировали в России условия для развития коммерческих ЦОД. Если раньше можно было горько усмехнуться над SLA, ждать пока встанет интернет-магазин на лежащем сервере, фактически доверять провайдеру «в тёмную», ...

Подборка: 4 полезных сервиса для потенциальных иммигрантов в США, Европу и другие страны

Я решил собрать в одном месте список онлайн-сервисов, которые будут полезны тем, кто всерьез задумался об иммиграции. Тема переезда в Европу, США или другие приятные регионы мира довольно часто поднимается на Хабре. Для статьи я отобрал четыре проекта. На удивление, ...