Вебинар: Статический анализ кода в методическом документе ЦБ РФ "Профиль защиты" - 16.02
В этой статье посмотрим на багов-путешественников, найденных с помощью статического анализатора PVS-Studio в LanguageTool — инструменте для проверки орфографии, стилистики и грамматики.

Всем привет! Hello, everyone! Hallo zusammen! Hola a tothom! مرحباً بالجميع!
В нашем блоге мы часто говорим про статический анализ, линтеры и подобные инструменты. Но сегодня мы нашли их довольно интересного представителя! LanguageTool — это многоязычная программа проверки орфографии, стилистики и грамматики, которая помогает исправлять и перефразировать тексты.
Сегодня мы заглянем в её код и посмотрим на интересные вещи, которые нашёл в нём статический анализатор кода PVS-Studio.
Код проекта проверялся на коммите 7778ca1.
Для того, чтобы проанализировать проект, я воспользовался плагином PVS-Studio для среды разработки Visual Studio Code.
Сначала необходимо было собрать проект с помощью плагина Project Manager for Java, а далее нажать на кнопку анализа и дождаться результата:

Примечание. О том, как установить и использовать плагин PVS-Studio для Visual Studio Code, можно прочитать в этом разделе документации.
Забытые разработчиком дескрипторы — довольно частая проблема. Проект LanguageTool эта участь не обошла:
Фрагмент 1
private Dictionary getDictionary(
Supplier<List<byte[]>> lines,
String dictPath,
String infoPath,
boolean isUserDict,
int userDictSize
){
....
InputStream metadata;
if (new File(infoPath).exists()) {
metadata = new FileInputStream(infoPath); // <=
} else {
metadata = getDataBroker().getFromResourceDirAsStream(infoPath);
}
Dictionary dict = Dictionary.read(fsaInStream, metadata);
if (!isUserDict) {
dicPathToDict.put(cacheKey, dict);
} else if (userDictCacheSize != null) {
getUserDictCache().put(userDictName, dict);
}
return dict;
}
Предупреждение PVS-Studio: V6127 The 'metadata' Closeable object is not closed. This may lead to a resource leak. MorfologikMultiSpeller.java 320
Итак, мы создали объект класса InputStream и благополучно забыли про него.
Интерфейс Closeable явно сигнализирует, что этот объект может удерживать системный ресурс, который необходимо явно освободить по завершении работы. В нашем случае FileInputStream удерживает файловый дескриптор. Если ссылка на такой файловый дескриптор будет утеряна без предварительного вызова метода close(), соответствующий ресурс останется удерживаемым операционной системой до тех пор, пока сборщик мусора не вызовет финализатор. Так как мы не можем гарантировать, когда это произойдёт и произойдёт ли вообще, есть вероятность утечки.
Здесь можно предположить, что поток будет закрыт автоматически внутри метода Dictionary.read. Однако документирующий комментарий к этому методу прямо указывает на обратное:
Attempts to load a dictionary from opened streams of FSA dictionary data and associated metadata. Input streams are not closed automatically.
То есть отсутствие явного вызова close() приводит к утечке файлового дескриптора. В условиях длительной работы приложения или частого вызова подобного кода это может исчерпать лимит открытых файловых дескрипторов, что приведёт к критическим ошибкам ввода-вывода и потенциально к отказу в обслуживании. Рекомендуется всегда использовать конструкции, гарантирующие детерминированное освобождение ресурсов, например try-with-resources.
Фрагмент 2
public String getEnclitic(AnalyzedToken token) {
....
if (word.endsWith("ه")) {
suffix = "ه";
....
else if ((word.equals("عني") ||
word.equals("مني")) &&
word.endsWith("ني") // <=
) {
suffix = "ني";
}
....
}
Предупреждение PVS-Studio: V6007 Expression 'word.endsWith("ني")' is always false. ArabicTagger.java 428
В этом коде инструмент работает с арабским языком. И здесь наш анализатор столкнулся с языковым барьером!
Для начала посмотрим на первую часть условия: здесь мы проверяем, что строка равна либо عني, либо مني. И в обоих этих вариантах строка не заканчивается на ني, то есть выражение word.endsWith("ني") всегда равно false, на что нам и указал анализатор. Но!
Строка не заканчивается на эти символы, если мы читаем её так, как привыкли. Но это же арабский язык, и здесь нужно читать справа налево! В таком случае анализатор указал на строку с ошибкой, но неправильно её понял: обе строки, при которых мы можем добраться до выполнения endsWith в этом ife, заканчиваются на ني. Поэтому условие будет всегда истинным, а не всегда ложным. То есть эта проверка избыточна.
Мы никогда не сталкивались с кодом, использующим арабский язык, поэтому и не подозревали о возможных проблемах. Теперь мы о них знаем :)
Фрагмент 3
private String removeOldDiacritics(String s) {
return s
.replace("contrapèl", "contrapel")
.replace("Contrapèl", "Contrapel")
.replace("vés", "ves")
.replace("féu", "feu")
.replace("desféu", "desfeu")
.replace("adéu", "adeu")
.replace("dóna", "dona")
.replace("dónes", "dones")
.replace("sóc", "soc")
.replace("vénen", "venen")
.replace("véns", "véns") // <=
.replace("fóra", "fora")
.replace("Vés", "Ves")
.replace("Féu", "Feu")
.replace("Desféu", "Desfeu")
.replace("Adéu", "Adeu")
.replace("Dóna", "Dona")
.replace("Dónes", "Dones")
.replace("Sóc", "Soc")
.replace("Vénen", "Venen")
.replace("Véns", "Vens")
.replace("Fóra", "Fora");
}
Предупреждение PVS-Studio: V6009 Function 'replace' receives an odd argument. The '" véns "' argument was passed several times. Catalan.java 453
Метод removeOldDiacritics используется в LanguageTool для нормализации написания определённых каталонских слов, т. е. заменяет устаревшие или альтернативные формы с диакритическими знаками (например, è или é) на современные.
Но в этом фрагменте кода закралась ошибка: слово véns мы заменяем на него же. Скорее всего, разработчик копировал исходное слово и заменял символ, но здесь забыл поменять символ во втором аргументе:
....
.replace("véns", "vens")
....
Фрагмент 4
@Nullable
@Override
public RuleMatch acceptRuleMatch(
RuleMatch match,
Map<String, String> arguments,
int patternTokenPos,
AnalyzedTokenReadings[] patternTokens,
List<Integer> tokenPositions
) throws IOException {
int posWord = 0;
....
AnalyzedTokenReadings[] tokens = match.getSentence()
.getTokensWithoutWhitespace();
....
posWord = verbSynth.getLastVerbIndex() +
verbSynth.getNumPronounsAfter() + 1;
primerAdverbi = posWord;
while (
posWord < tokens.length &&
!adverbiFinal.contains(
tokens[posWord]
.getToken()
.toLowerCase()
)
) {
posWord++;
}
if (
posWord == tokens.length ||
!adverbiFinal.contains(
tokens[posWord]
.getToken()
.toLowerCase()
)
) {
return null;
}
darrerAdverbi = posWord;
String darrerAdverbiStr = tokens[darrerAdverbi].getToken(); // <=
....
if (primerAdverbi == -1 || darrerAdverbi == -1) { // <=
return null;
}
....
}
Предупреждение PVS-Studio: V6079 Value of the 'darrerAdverbi' variable is checked after use. Potential logical error is present. DonarseliBeFilter.java 83
Ещё один метод для работы с каталонским языком!
Здесь анализатор ругается на то, что переменная darrerAdverbi использовалась как индекс до её проверки на неравенство -1. И это действительно так: переменная используется буквально парой строк выше проверки.
Но, посмотрев на это срабатывание, в методе можно найти ещё кое-что интересное! Чтобы разобраться в этом, посмотрим, что здесь происходит.
После подготовки данных происходит поиск, с какой позиции начинать искать последнее наречие из заданного множества. Начальная позиция для поиска вычисляется на основе индекса последнего глагола и количества следующих за ним местоимений:
....
posWord = verbSynth.getLastVerbIndex() +
verbSynth.getNumPronounsAfter() + 1;
....
Оба вызываемых метода в теории могут вернуть нам -1, если ни глагол, ни местоимения не будут найдены. В таком случае posWord будет равен -1.
Сразу после этого, безо всяких проверок, мы используем это значение в качестве индекса в правой части условия, до выполнения которой в таком случае мы дойдём, т. к. -1 точно будет меньше, чем tokens.length:
....
if (
posWord == tokens.length ||
!adverbiFinal.contains(
tokens[posWord] // <=
.getToken()
.toLowerCase()
)
....
И в этом условии мы можем получить падение программы с IndexOutOfBoundsException. Таким образом, падение с исключением может произойти ещё раньше, чем указало предупреждение.
Внутри статического анализатора, кстати, подобные ошибки находятся с помощью анализа потока данных — метода получения информации о потенциальном диапазоне значений, которые могут быть вычислены в различных точках программы.
Фрагмент 5
protected List<RuleMatch> getRuleMatches(
String word, int startPos,
AnalyzedSentence sentence,
List<RuleMatch> ruleMatchesSoFar,
int idx,
AnalyzedTokenReadings[] tokens
) throws IOException {
....
//Translator translator = getTranslator(globalConfig);
Translator translator = null; // <=
if (
translator != null && // <=
ruleMatch == null &&
motherTongue != null &&
language.getShortCode().equals("en") &&
motherTongue.getShortCode().equals("de")
) {....}
....
}
Предупреждение PVS-Studio: V6007 Expression 'translator != null' is always false. MorfologikSpellerRule.java 449
Анализатор ругается на то, что условие всегда эквивалентно false. Чтобы понять это, нужно всего-то посмотреть на предыдущую строку: там в первой проверяемой переменной присваивается null.
Однако ещё строкой выше можно заметить закомментированное присваивание этой переменной, где уже было значение. Если заглянуть в историю коммитов, то можно заметить, что её закомментировали ради отключения определённого функционала.
Да, мы не можем назвать это полноценным багом, но это вечно отрицательное условие привело к тому, что в проекте появилось около 30 строк мёртвого кода, который ранее работал при выполнении этого условия.
Иногда недостаточно чистый код — это не просто повод поговорить о красоте, а помощник для багов. Об этом мы писали в другой статье.
Фрагмент 6
private static String getDigitHundredJarStatus(
int digit,
String inflectionCase
) {
if (inflectionCase.equals("jar") ||
inflectionCase.equals("jar")) {
return ArabicNumbersWordsConstants
.arabicJarHundreds.get(digit);
}
return ArabicNumbersWordsConstants
.arabicHundreds.get(digit);
}
Предупреждение PVS-Studio: V6001 There are identical sub-expressions 'inflectionCase.equals("jar")' to the left and to the right of the '||' operator. ArabicNumbersWords.java 215
И снова условие. В этот раз левая и правая часть if похожи как две капли воды. Опять же, закрались сомнения, что такое получилось не в ходе рефакторинга. Но нет, такое условие появилось в результате появления файла в коммите.
Вероятнее всего, условия различались, а при доработке кода разработчик заменил строки средствами IDE, потому что такое срабатывание повторяется ещё пару раз.
V6001 There are identical sub-expressions 'inflectionCase.equals("jar")' to the left and to the right of the '||' operator. ArabicNumbersWords.java 207
V6001 There are identical sub-expressions 'inflectionCase.equals("jar")' to the left and to the right of the '||' operator. ArabicNumbersWords.java 223
Вы когда-нибудь видели код, у которого не получилось?
Фрагмент 7
public static void main(
String[] args
) throws IOException {
....
List<RuleMatch> matches = lt.check(incorrectExample);
for (RuleMatch match : matches) {
if (match.getSuggestedReplacements().isEmpty()) {
....
printRule(rule.getId(), rule, incorrectExample, popularity);
noSuggestion++;
} else {
suggestion++;
}
break; // <=
}
....
}
Предупреждение PVS-Studio: V6037 An unconditional 'break' within a loop. NoSuggestionRuleList.java 97
Цикл создаёт впечатление многократной обработки, однако на практике всегда завершается после первой итерации из‑за break.
Вероятно, разработчик намеревался обрабатывать только первый найденный элемент: такая практика встречается достаточно часто, особенно когда из коллекции требуется лишь первое совпадение.
Тем не менее использование цикла для доступа только к первому элементу выглядит избыточно и может вводить в заблуждение: читатель кода может ожидать более сложной логики, которая на самом деле отсутствует. Это не обязательно ошибка, но такой паттерн снижает читаемость и может стать источником недоразумений при дальнейшем сопровождении.
Фрагмент 8
private final static Map<String, Integer> id2prio = new HashMap<>();
static {
id2prio.put("I_E", 10);
id2prio.put("CHILDISH_LANGUAGE", 8);
id2prio.put("RUDE_SARCASTIC", 6);
id2prio.put("FOR_NOUN_SAKE", 6);
id2prio.put("YEAR_OLD_HYPHEN", 6);
id2prio.put("MISSING_HYPHEN", 5);
id2prio.put("WRONG_APOSTROPHE", 5);
....
id2prio.put("SPURIOUS_APOSTROPHE", 1); // <=
....
id2prio.put("SPURIOUS_APOSTROPHE", 1); // <=
....
}
Предупреждение PVS-Studio: V6033 An item with the same key '"SPURIOUS_APOSTROPHE"' has already been added. English.java 391
В этом фрагменте добавляется большое количество различных значений в хеш-мапу. Действительно большое количество: начинается всё на 283 строке, а заканчивается на 612. И анализатор нашёл в этом огромном полотне два повторяющихся значения.
Справедливости ради, второе значение не отличается от первого, поэтому это не является ошибкой как таковой, но тут же есть случаи, когда происходит наоборот.
Фрагмент 9
....
id2prio.put("SENTENCE_FRAGMENT", -50);
id2prio.put("SENTENCE_FRAGMENT", -51);
id2prio.put("SEEMS_TO_BE", -51);
....
Предупреждение PVS-Studio: V6033 An item with the same key '"SENTENCE_FRAGMENT"' has already been added. English.java 594
В этом фрагменте уже можно предположить, что анализатор ругается на полноценную ошибку. Значения у повторяющихся записей разные. Забавно, что переписанное значение идёт буквально следующей строкой. Вероятнее всего, разработчик скопировал строку c ключом SENTENCE_FRAGMENT и забыл поменять его на SEEMS_TO_BE.
Фрагмент 10
private final Map<String,String> adverb2Adj = new HashMap<String, String>() {{
// irregular ones:
put("well", "good");
put("fast", "fast");
put("hard", "hard");
....
put("jokily", "jokey");
....
put("jokily", "joking");
....
}
Предупреждение PVS-Studio: V6033 An item with the same key '"jokily"' has already been added. AdverbFilter.java 199
Этот фрагмент, как и предыдущие, относится к коду обработки английского языка. В этой хеш-таблице содержится отображение наречий на соответствующие им прилагательные. И для слова jokily приведено сразу два варианта. Но проблема в том, что позже добавленное значение заменит предыдущее.
Чтобы решить эту проблему, можно было бы использовать в качестве значений списки строк, чтобы для одного наречия можно было использовать несколько вариантов прилагательных, но в текущем виде один из вариантов потеряется.
На этом мы завершаем наше путешествие по проекту LanguageTool. Обо всех найденных ошибках мы сообщим разработчикам с помощью Issue в репозитории.
Кстати, этот проект был выбран из нашего репозитория, где вы также можете предложить нам с помощью Pull Request'а проект для проверки.
А ещё можно попробовать статический анализатор PVS-Studio на своём проекте, получив пробную лицензию по этой ссылке.
Чистого вам кода, друзья!
0