Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

FindBugs вопреки CDK

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

Помимо анализатора кода для такой статьи требуется подопытный кролик. Необходим достаточно огромный план, но при этом не настоль распространённый, Дабы разработчики безупречно вылизывали код. Я предпочел план Chemistry Development Kit (версия 1.4.19), которым приходилось пользоваться. FindBugs я установил как плагин к Eclipse, потому что мне так привычнее.



FindBugs с настройками по умолчанию нашел 338 задач. Безусловно, я не буду описывать их все, остановлюсь на увлекательных.

1. Неудачная попытка получить случайное правильное число

В классе org.openscience.cdk.math.RandomNumbersTool:

public static int randomInt(int lo, int hi) {
    return (Math.abs(random.nextInt()) % (hi - lo   1))   lo;
}

Способ Random.nextInt() выдаёт всякое число, возможное в int. Способ Math.abs получает модуль числа. Задача в том, что Math.abs не работает для Integer.MIN_VALUE, потому что, как вестимо, число, противоположное этому, не помещается в int. Впрочем Random.nextInt() абсолютно может выдать это число (приблизительно один раз на 4 миллиарда), тогда каждый способ отработает неверно.

2. Итог BufferedReader.readLine() не проверяется на null

Встречается неоднократно, скажем, в таком виде (org.openscience.cdk.io.CIFReader):

private void skipUntilEmptyOrCommentLine(String line) throws IOException {
    // skip everything until empty line, or comment line
    while (line != null && line.length() > 0 && line.charAt(0) != '#') {
        line = input.readLine().trim();
    }
}

Входной поток не проверяется на подготовленность способом ready(), и итог readLine() не проверяется на null. В итоге ненормально сформированный входной файл вызовет NullPointerException.

3. Применяется & взамен &&

Банальная оплошность такого вида (org.openscience.cdk.atomtype.CDKAtomTypeMatcher):

if (atom.getFormalCharge() != null &
    atom.getFormalCharge().intValue() == 1) {...}

Если первая проверка завершилась неудачно, вторая будет исполнена всё равно и приведёт к NullPointerException.

4. Сопоставление объектов String, Integer либо Double по ==

Встречается неоднократно. Скажем, здесь (org.openscience.cdk.AtomType.compare):

return (getAtomTypeName() == type.getAtomTypeName()) &&
        (maxBondOrder == type.maxBondOrder) &&
        (bondOrderSum == type.bondOrderSum);

getAtomTypeName() возвращает String, а bondOrderSum имеет тип Double. Логика приложения абсолютно допускает, что здесь окажутся различные, но равные по equals объекты и сопоставление отработает некорректно.

Вообще неугодно применять объекты Integer, Double и так дальше, если у вас нет отличной поводы их применять.

5. Программист позабыл, что строки константны

Встречаются вызовы способов класса String, которые создают новую строку. Скажем (net.sf.cdk.tools.MakeJavafilesFiles.readBlackList):

while (line != null) {
	line.trim();
	if (line.length() > 0) 
		blacklist.add(line);
	line = reader.readLine();
}

Вызов line.trim() непотребен, так как саму строку line он не меняет, а итог никто не использует. Автор очевидно имел в виду line = line.trim(). Подобно встречаются вызовы String.substring без сохранения итога.

6. Сопоставление объектов различных типов

Нередки сопоставления в духе if( atom.equals("H") ), где atom типа IAtom. Подразумевается, видимо, if( atom.getSymbol().equals("H") ). Вообще это таинственное место, так как таких ошибок огромнее десятка, а, на мой взор, они обязаны крепко влиять на семантику и искажать итог.

7. Применение неинициализированного поля

org.openscience.cdk.dict.EntryReact:

public void setReactionMetadata(String metadata) {
    this.reactionInfo.add( metadata );
}

FindBugs определяет, что приватное поле reactionInfo не инициализируется ни в каком ином способе, следственно данный способ неизменно выдаст NullPointerException.

8. Неправильная инициализация статического поля

К примеру, класс org.openscience.cdk.qsar.AtomValenceTool:

public class AtomValenceTool {
    private static Map<String,Integer> valencesTable = null;
    public static int getValence(IAtom atom) {
        if (valencesTable == null) {
            valencesTable = new HashMap<String, Integer>();
            valencesTable.put("H", 1);
            valencesTable.put("He", 8);
            valencesTable.put("Ne", 8);
            ...
        }
        return valencesTable.get(atom.getSymbol());
    }
}

При вызове из различных потоков допустим race condition, когда valencesTable теснее не null, но ещё не заполнена до конца. Тогда один поток выдаст NullPointerException для абсолютно правильного атома.

9. Нарушение контрактов

Способ equals() должен возвращать false, если довод null. Способ clone() не должен никогда возвращать null. Способ clone() не в final-классе должен super.clone(), а не создавать объект вручную (напротив если унаследовать класс, то clone() сломается). Сходственные вещи могут не приводить к ошибкам, но всё же их следует чураться.

10. Неверное применение регулярного выражения

net.sf.cdk.tools.doclets.CDKIOOptionsTaglet.getClassName:

String path = file.getPath().replaceAll(File.separator, ".");

Способ replaceAll принимает в качестве довода регулярное выражение. Под Windows File.separator — это обратный слэш, тот, что намеренно интерпретируется в регулярных выражениях, следственно данный код упадёт с PatternSyntaxException.

11. Переопределённый способ из родительского конструктора использует неинициализированную переменную

Увлекательная обстановка в классе org.openscience.cdk.debug.DebugAtomContainer. Там объявлено поле

ILoggingTool logger = LoggingToolFactory.createLoggingTool(DebugAtomContainer.class);

и есть способ:

public void addStereoElement(IStereoElement parity) {
    logger.debug("Adding stereo element: ", parity);
    super.addStereoElement(parity);
}

Задача в том, что данный способ вызывается в одном из конструкторов базового класса, когда присваивание значения переменной logger ещё не отработало. В итоге, безусловно, случится NullPointerException.

Завершение

Были и ещё ошибки, но остановимся. Хочу подметить, что CDK — отличная библиотека, которая в целом типично работает. И было обнаружено достаточно много задач не потому, что программисты тупые. Типичные программисты, все так пишут. Легко они, видимо, ещё не пользовались статическими анализаторами кода. А вы пользуйтесь, это благотворно!

Источник: programmingmaster.ru

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB