В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время кратко рассказать о том, как мы начали поддерживать язык Java, как далеко мы продвинулись и что у нас в планах на будущее. Конечно, в этой статье будут перечислены первые испытания анализатора на проектах с открытым кодом.
PVS-Studio
Вот краткое описание PVS-Studio для Java-разработчиков, которые раньше о нем не слышали.
Этот инструмент предназначен для обнаружения ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках C, C ++, C # и Java. Он работает в среде Windows, Linux и macOS.
PVS-Studio выполняет статический анализ кода и формирует отчет, который помогает разработчику находить и устранять дефекты. Тем, кому интересно, как именно PVS-Studio ищет ошибки, предлагаю ознакомиться со статьей Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей.
Начало
Я мог бы придумать умный рассказ о том, как мы размышляли о том, какой следующий язык будет поддерживать в PVS-Studio. О разумном выборе Java, в основе которого лежит высокая популярность этого языка и т. Д.
Однако, как это бывает в жизни, выбор был сделан не на основе глубокого анализа, а путем эксперимента :). Да, мы задумались о направлении дальнейшего развития анализатора PVS-Studio. Мы рассматривали такие языки, как: Java, PHP, Python, JavaScript, IBM RPG. Мы даже склонялись к языку Java, но окончательный выбор не был сделан. Для тех, кто остановился на незнакомой IBM RPG, хочу направить вас на эту заметку, из которой все станет ясно.
В конце 2017 года мой коллега Егор Бредихин просмотрел готовые библиотеки парсинга кода (иными словами - парсеры) на предмет новых интересных для нас направлений развития. В итоге он наткнулся на несколько проектов по синтаксическому анализу кода Java. Ему удалось быстро изготовить прототип анализатора с парой диагностик на базе Spoon. Более того, стало ясно, что мы сможем использовать в анализаторе Java некоторые механизмы анализатора C ++ с помощью SWIG. Мы посмотрели, что у нас получилось, и поняли, что наш следующий анализатор будет для Java.
Мы хотели бы поблагодарить Егора за его труд и усердную работу над анализатором Java. Сам процесс разработки он описал в статье Разработка нового статического анализатора: PVS-Studio Java.
А как насчет конкурентов?
По всему миру существует множество бесплатных и коммерческих статических анализаторов кода для Java. Нет смысла перечислять их все в статье. Я просто оставлю ссылку на Список инструментов для статического анализа кода (см. Раздел Java и многоязычие).
Однако я знаю, что в первую очередь нас спросят об IntelliJ IDEA, FindBugs и SonarQube (SonarJava).
IntelliJ IDEA
В IntelliJ IDEA встроен очень мощный статический анализатор кода. Более того, анализатор развивается, его авторы внимательно следят за нашей деятельностью. Итак, IntelliJ IDEA - сложный для нас файл cookie. Нам не удастся превзойти IntelliJ IDEA в диагностических возможностях, по крайней мере, пока. Поэтому сконцентрируемся на других наших преимуществах.
Статический анализ в IntelliJ IDEA - это прежде всего одна из особенностей среды, которая накладывает на нее определенные ограничения. Что касается нас, у нас есть свобода в том, что мы можем делать с нашим анализатором. Например, мы можем быстро адаптировать его к конкретным потребностям клиентов. Быстрая и глубокая поддержка - наше конкурентное преимущество. Наши клиенты напрямую общаются с разработчиками, работающими над той или иной частью PVS-Studio.
В PVS-Studio есть много возможностей интегрировать его в цикл разработки больших старых проектов. Например, наша интеграция с SonarQube. Также в него входит массовое подавление предупреждений анализатора, что позволяет сразу начать использовать инструмент в большом проекте для отслеживания ошибок только в новом или измененном коде. PVS-Studio может быть встроен в процесс непрерывной интеграции. Думаю, эти и другие возможности помогут нашему анализатору найти себе место под солнцем в мире Java.
FindBugs
Проект FindBugs заброшен. Тем не менее, об этом стоит упомянуть в связи с тем, что это, пожалуй, самый известный бесплатный статический анализатор Java-кода.
SpotBugs можно назвать преемником FindBugs. Однако он менее популярен, и пока не ясно, что с ним будет.
В целом, мы думаем, что хотя FindBugs был и остается чрезвычайно популярным, а также бесплатным анализатором, нам не следует на нем останавливаться. Этот проект просто незаметно станет историей.
P.S. Кстати, теперь PVS-Studio можно бесплатно использовать и при работе с открытыми проектами.
SonarQube (SonarJava)
Мы считаем, что не конкурируем с SonarQube, а дополняем его. PVS-Studio интегрируется в SonarQube, что позволяет разработчикам находить больше ошибок и потенциальных уязвимостей безопасности в своих проектах. Мы регулярно рассказываем, как интегрировать инструмент PVS-Studio и другие анализаторы в SonarQube на мастер-классах, которые проводим в рамках различных конференций.
Как запустить PVS-Studio для Java
Мы сделали доступными для пользователей наиболее популярные способы интеграции анализатора в систему сборки:
- Плагин для Maven;
- Плагин для Gradle;
- Плагин для IntelliJ IDEA
На этапе тестирования мы встретили множество пользователей, у которых есть самописные системы сборки, особенно в сфере мобильной разработки. Им понравилась возможность запустить анализатор напрямую, перечислив источники и путь к классам.
Подробную информацию обо всех способах запуска анализатора вы можете найти на странице документации Как запустить 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.
Писать об ошибках в этих проектах - непростая задача. Дело в том, что эти проекты очень качественные. Собственно, в этом нет ничего удивительного. Наши наблюдения показывают, что статические анализаторы кода всегда хорошо тестируются и проверяются с помощью других инструментов.
Несмотря на все это, мне придется начинать именно с этих проектов. У меня не будет второго шанса написать о них. Уверен, что после выпуска PVS-Studio для Java разработчики перечисленных проектов возьмут на вооружение PVS-Studio и начнут использовать его для регулярных или, по крайней мере, периодических проверок своего кода. Например, я знаю, что Тагир Валеев, один из разработчиков JetBrains, работает над статическим анализатором кода IntelliJ IDEA, в момент написания статьи уже играет с бета-версией 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] Литерал 0.2 типа double сравнивается со значением типа int. НазваниеCapitalizationInspection.java 169
Дело в том, что функция должна возвращать истину, если менее 20% слов начинаются с заглавной буквы. Собственно проверка не работает, потому что происходит целочисленное деление. В результате деления мы можем получить только два значения: 0 или 1.
Функция вернет false, только если все слова начинаются с заглавной буквы. Во всех остальных случаях операция деления приведет к 0, и функция вернет истину.
IntelliJ IDEA, подозрительный цикл
public int findPreviousIndex(int current) {
int count = myPainter.getErrorStripeCount();
int foundIndex = -1;
int foundLayer = 0;
if (0 <= current && current < count) {
current--;
for (int index = count - 1; index >= 0; index++) { // <=
int layer = getLayer(index);
if (layer > foundLayer) {
foundIndex = index;
foundLayer = layer;
}
}
....
}Предупреждение PVS-Studio: V6007 [CWE-571] Выражение index ›= 0 всегда истинно. Updater.java 184
Сначала посмотрите на условие (0 ‹= current && current‹ count). Он выполняется только в том случае, если значение переменной count больше 0.
Теперь посмотрим на цикл:
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, Копировать-Вставить
@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] Слева и справа от оператора «||» есть идентичные подвыражения «LoadingOrder.BEFORE_STR_OLD.equalsIgnoreCase (str)». Проверьте строки: 127, 128. ExtensionOrderConverter.java 127
Старый добрый эффект последней строчки. Разработчик бросил пистолет и, умножив строчку кода, забыл исправить. В результате строка str дважды сравнивается с BEFORE_STR_OLD. Скорее всего, одно из сравнений должно быть с AFTER_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] Слева и справа от оператора «&&» есть идентичные подвыражения ‘! StringUtil.endsWithChar (name,’ ”’) ’. 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] Выражение ‘i‹ endOffset ’всегда истинно. EnterAfterJavadocTagHandler.java 183
Подвыражение i ‹endOffset в условии оператора if не имеет смысла. Переменная i всегда меньше, чем endOffset в любом случае, что следует из условия выполнения цикла.
Скорее всего, разработчик хотел уберечься от переполнения строки при вызове функций:
- text.charAt (я + 1)
- CharArrayUtil.regionMatches (текст, 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] Выражение «buffer.length ()› 0 »всегда истинно. 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») всегда возвращает false. Действительно, если символы «\ 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] Выражение «0» .equals (text) ’всегда ложно. 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] Выражение «count‹ 4 »всегда истинно. Util.java 394
Теоретически поиск тега xml должен производиться только в первых четырех строках файла. Но из-за того, что кто-то забыл увеличить переменную count, будет прочитан весь файл.
Во-первых, это может быть очень медленная операция, а во-вторых, где-то в середине файла может быть обнаружено что-то, что будет восприниматься как тег 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] Значение присваивается переменной «приоритет», но не используется. FindNonShortCircuit.java 197
Значение переменной priority устанавливается в зависимости от значения переменной sawNullTestVeryOld. Однако это не имеет значения. После этого переменной priority в любом случае будет присвоено другое значение. Очевидная ошибка в логике функции.
SonarQube, Копировать-Вставить
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 Странно, что тело метода setUpdatedAtFromDefinition полностью эквивалентно телу другого метода setUpdatedAtFromMetadata. Проверить строки: 396, 405. RuleDto.java 396
Поле определение используется в методе setUpdatedAtFromMetadata. Скорее всего, следует использовать поле метаданные. Это очень похоже на последствия неудачного копирования-вставки.
SonarJava, дубликаты при инициализации карты
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] Элемент с таким же ключом «JavaPunctuator.PLUSEQU» уже добавлен. Проверить строки: 104, 100. KindMaps.java 104
Одна и та же пара "ключ-значение" задается в карте дважды. Скорее всего, это произошло случайно и на самом деле ошибки нет. Однако этот код в любом случае нужно проверять, потому что, возможно, кто-то забыл добавить какую-то другую пару.
Вывод
Зачем писать заключение, когда это так очевидно ?! Предлагаю всем вам скачать PVS-Studio прямо сейчас и попробовать проверить свои рабочие проекты на языке Java! Скачать PVS-Studio.
Всем спасибо за внимание. Надеюсь, что вскоре мы порадуем наших читателей серией статей о проверке различных Java-проектов с открытым кодом.
