FindBugs помогает узнать Java лучше

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

В этом плане статический анализатор похож на опытного товарища, который сидит рядом и смотрит, как вы пишете код. Он не только подсказывает вам: «вот здесь ты ошибся, когда копипастил», но и говорит: «нет, так писать нельзя, вон сам в документацию глянь». Такой товарищ полезней самой документации, потому что он подсказывает только те вещи, с которыми вы реально сталкиваетесь в работе, и молчит о тех, которые вам никогда не пригодятся.

В этом посте я расскажу о некоторых тонкостях Java, о которых я узнал в результате использования статического анализатора FindBugs. Возможно, какие-то вещи окажутся неожиданными и для вас. Важно, что все примеры не умозрительны, а основаны на реальном коде.

Тернарный оператор ?:

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

Type var = condition ? valTrue : valFalse;

и

Type var;
if(condition)
  var = valTrue;
else
  var = valFalse;

Оказалось, что тут есть тонкость. Так как тернарный оператор может быть частью сложного выражения, его результатом должен быть конкретный тип, определённый на этапе компиляции. Поэтому, скажем, при истинном условии в if-форме компилятор приводит valTrue сразу к типу Type, а в форме тернарного оператора сперва приводит к общему типу valTrue и valFalse (несмотря на то, что valFalse не вычисляется), а затем уже результат приводит к типу Type. Правила приведения оказываются не совсем тривиальными, если в выражении участвуют примитивные типы и обёртки над ними (Integer, Double и т. д.) Все правила подробно описаны в JLS 15.25. Посмотрим на некоторые примеры.

Number n = flag ? new Integer(1) : new Double(2.0);

Что будет в n, если flag установлен? Объект Double со значением 1.0. Компилятору смешны наши неуклюжие попытки создать объект. Так как второй и третий аргумент — обёртки над разными примитивными типами, компилятор разворачивает их и приводит к более точному типу (в данном случае double). А уже после выполнения тернарного оператора для присваивания снова выполняется боксинг. По сути код эквивалентен такому:

Number n;
if( flag )
    n = Double.valueOf((double) ( new Integer(1).intValue() ));
else
    n = Double.valueOf(new Double(2.0).doubleValue());

С точки зрения компилятора код не содержит проблем и прекрасно компилируется. Но FindBugs выдаёт предупреждение:

BX_UNBOXED_AND_COERCED_FOR_TERNARY_OPERATOR: Primitive value is unboxed and coerced for ternary operator in TestTernary.main(String[])

A wrapped primitive value is unboxed and converted to another primitive type as part of the evaluation of a conditional ternary operator (the b? e1: e2 operator). The semantics of Java mandate that if e1 and e2 are wrapped numeric values, the values are unboxed and converted/coerced to their common type (e.g, if e1 is of type Integer and e2 is of type Float, then e1 is unboxed, converted to a floating point value, and boxed. See JLS Section 15.25.

Разумеется, FindBugs предупреждает и о том, что Integer.valueOf(1) эффективнее, чем new Integer(1), но это уж все и так знают.

Или такой пример:

Integer n = flag ? 1 : null;

Автор хочет поместить null в n, если флаг не установлен. Думаете, сработает? Да. Но давайте усложним:

Integer n = flag1 ? 1 : flag2 ? 2 : null;

Казалось бы, особой разницы нет. Однако теперь, если оба флага сброшены, данная строчка генерирует NullPointerException. Варианты для правого тернарного оператора — int и null, поэтому результирующий тип Integer. Варианты для левого — int и Integer, поэтому по правилам Java результат — int. Для этого надо совершить unboxing, вызвав intValue, что и выдаёт исключение. Код эквивалентен такому:

Integer n;
if( flag1 )
    n = Integer.valueOf(1);
else {
    if( flag2 )
        n = Integer.valueOf(Integer.valueOf(2).intValue());
    else
        n = Integer.valueOf(((Integer)null).intValue());
}

Здесь FindBugs выдаёт два сообщения, которых достаточно, чтобы заподозрить ошибку:

BX_UNBOXING_IMMEDIATELY_REBOXED: Boxed value is unboxed and then immediately reboxed in TestTernary.main(String[])
NP_NULL_ON_SOME_PATH: Possible null pointer dereference of null in TestTernary.main(String[])
There is a branch of statement that, if executed, guarantees that a null value will be dereferenced, which would generate a NullPointerException when the code is executed.

Ну и последний пример на эту тему:

double[] vals = new double[] {1.0, 2.0, 3.0};
double getVal(int idx) {
    return (idx < 0 || idx >= vals.length) ? null : vals[idx];
}

Неудивительно, что этот код не работает: как функция, возвращающая примитивный тип, может вернуть null? Удивительно, что он без проблем компилируется. Ну почему компилируется — вы уже поняли.

DateFormat

Для форматирования даты и времени в Java рекомендуется пользоваться классами, реализующими интерфейс DateFormat. Например, это выглядит так:

public String getDate() {
    return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(new Date());
}

Зачастую класс многократно использует один и тот же формат. Многим придёт в голову идея оптимизации: зачем каждый раз создавать объект формата, когда можно пользоваться общим экземпляром?

private static final DateFormat format = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");

public String getDate() {
    return format.format(new Date());
}

Вот так красиво и здорово, только, к сожалению, не работает. Точнее работает, но изредка ломается. Дело в том, что в документации к DateFormat написано:

Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.

И это действительно так, если посмотреть внутреннюю реализацию SimpleDateFormat. В процессе выполнения метода format() объект пишет в поля класса, поэтому одновременное использование SimpleDateFormat из двух потоков приведёт с некоторой вероятностью к неправильному результату. Вот что пишет FindBugs по этому поводу:

STCAL_INVOKE_ON_STATIC_DATE_FORMAT_INSTANCE: Call to method of static java.text.DateFormat in TestDate.getDate()
As the JavaDoc states, DateFormats are inherently unsafe for multithreaded use. The detector has found a call to an instance of DateFormat that has been obtained via a static field. This looks suspicous.

For more information on this see Sun Bug #6231579 and Sun Bug #6178997.

Подводные камни BigDecimal

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

System.out.println(new BigDecimal(1.1));

Делать так действительно никто не запрещает, вот только результат может показаться неожиданным: 1.100000000000000088817841970012523233890533447265625. Так происходит, потому что примитивный double хранится в формате IEEE754, в котором невозможно представить 1.1 идеально точно (в двоичной системе счисления получается бесконечная периодическая дробь). Поэтому там хранится максимально близкое значение к 1.1. А конструктор BigDecimal(double) напротив работает точно: он идеально преобразует заданное число в IEEE754 к десятичному виду (конечная двоичная дробь всегда представима в виде конечной десятичной). Если же хочется представить в виде BigDecimal именно 1.1, то можно написать либо new BigDecimal("1.1"), либо BigDecimal.valueOf(1.1). Если число не выводить сразу, а поделать с ним какие-то операции, можно и не понять, откуда берётся ошибка. FindBugs выдаёт предупреждение DMI_BIGDECIMAL_CONSTRUCTED_FROM_DOUBLE, в котором даются те же советы.

А вот ещё одна штука:

BigDecimal d1 = new BigDecimal("1.1");
BigDecimal d2 = new BigDecimal("1.10");
System.out.println(d1.equals(d2));

Фактически d1 и d2 представляют собой одно и то же число, но equals выдаёт false, потому что он сравнивает не только значение чисел, но и текущий порядок (число знаков после запятой). Это написано в документации, но мало кто будет читать документацию к такому знакомому методу как equals. Такая проблема может всплыть далеко не сразу. Сам FindBugs об этом, к сожалению, не предупреждает, но есть популярное расширение к нему — fb-contrib, в котором данный баг учтён:

MDM_BIGDECIMAL_EQUALS

equals() being called to compare two java.math.BigDecimal numbers. This is normally a mistake, as two BigDecimal objects are only equal if they are equal in both value and scale, so that 2.0 is not equal to 2.00. To compare BigDecimal objects for mathematical equality, use compareTo() instead.

Переводы строк и printf

Нередко программисты, перешедшие на Java после Си, с радостью открывают для себя PrintStream.printf (а также PrintWriter.printf и т. д.). Мол, отлично, это я знаю, прямо как в Си, ничего нового учить не надо. На самом деле есть отличия. Одно из них кроется в переводах строк.

В языке Си есть разделение на текстовые и бинарные потоки. Вывод символа 'n' в текстовый поток любым способом автоматически будет преобразован в системно-зависимый перевод строки ("rn" на Windows). В Java такого разделения нет: надо передавать в выходной поток правильную последовательность символов. Это автоматически делают, например, методы семейства PrintStream.println. Но при использовании printf передача 'n' в строке формата — это просто 'n', а не системно-зависимый перевод строки. К примеру, напишем такой код:

System.out.printf("%sn", "str#1");
System.out.println("str#2");

Перенаправив результат в файл, увидим:

Таким образом можно получить странную комбинацию переводов строки в одном потоке, что выглядит неаккуратно и может снести крышу какому-нибудь парсеру. Ошибку можно долго не замечать, особенно если вы преимущественно работаете на Unix-системах. Для того, чтобы вставить правильный перевод строки с помощью printf, используется специальный символ форматирования "%n". Вот что пишет FindBugs по этому поводу:

VA_FORMAT_STRING_USES_NEWLINE: Format string should use %n rather than n in TestNewline.main(String[])

This format string include a newline character (n). In format strings, it is generally preferable better to use %n, which will produce the platform-specific line separator.

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

Автор: lany

Источник