Вышел FindBugs 3.0.1

Новая версия FindBugs доступна для скачивания на официальном сайте. Несмотря на то что поменялась только третья цифра в номере версии, вас ждёт множество новых интересных детекторов, а также улучшение старых. Если основная фича 3.0.0 заключалась в поддержке Java 8 и новых детекторов практически не было, то в 3.0.1 упор был сделан на функционал. Здесь я хочу вкратце осветить некоторые новые детекторы, разработанные лично мной.

В этой статье ограничимся детекторами бесполезного кода. Мне нравится разрабатывать такие детекторы: они не просто ищут конкретный шаблон (например, вызов метода с заведомо некорректным параметром), а доказывают бесполезность определённых фрагментов кода. Порой таким образом можно найти весьма неожиданные ошибки.

UC_USELESS_CONDITION

Про этот детектор я писал отдельно, но со времени той статьи он был значительно доработан. Теперь полноценно обрабатываются изменяемые переменные, переиспользование регистра, переброс значений из одной переменной в другую, вызовы String.length(), unboxing, проверки длин массивов. Также частично поддерживаются поля. Доработаны приоритеты. Этот детектор позволяет найти очень интересные баги из серии «как получилось, что мы годами этого не замечали?» Например, такой код нашёлся в PMD 5.2.3 (это тоже инструмент для проверки качества кода!):

protected boolean isHexCharacter(char c) {
    return isLatinDigit(c) || ('A' <= c || c <= 'F') || ('a' <= c || c <= 'f');
}

Из-за ошибочного использования || вместо && условие истинно для любых символов (метод всегда вернёт true). Баг существует как минимум с версии 5.0.0 (дальше не копал) — почти три года.

Замечу, кстати, что такие ошибки некоторые не видят, даже если смотрят прямо на них. Вот тут другой разработчик FindBugs подумал, что на условии c != 'n' || c != 'r' произошло ложное срабатывание и даже придумал хитрую теорию про совпадение кода символа и номера регистра с переменной. А на самом деле срабатывание не ложное, код действительно с ошибкой.

Иногда находится ошибочная копипаста. Например, такое нашлось в Lucene 4.10.3:

final int bitsPerStoredFields = fieldsStream.readVInt();
if (bitsPerStoredFields == 0) { ... } 
else if (bitsPerStoredFields > 31) {
  throw new CorruptIndexException("bitsPerStoredFields=" + bitsPerStoredFields + " (resource=" + fieldsStream + ")");
} else { ... }

final int bitsPerLength = fieldsStream.readVInt();
if (bitsPerLength == 0) { ... }
else if (bitsPerStoredFields > 31) { // Здесь UC_USELESS_CONDITION
  throw new CorruptIndexException("bitsPerLength=" + bitsPerLength + " (resource=" + fieldsStream + ")");
} else { ... }

FindBugs теперь видит, что при bitsPerStoredFields > 31 мы уже должны были вывалиться с исключением, поэтому второй раз такая проверка бессмысленна. Видимо, авторы скопировали кусок кода и забыли исправить на bitsPerLength.

UC_USELESS_OBJECT

Это сообщение выводится, если в методе создаётся объект или массив и с ним производятся некоторые манипуляции, которые не дают никаких побочных эффектов, кроме изменения состояния этого объекта. Подобный детектор есть в стороннем плагине FB-contrib (WOC_WRITE_ONLY_COLLECTION_LOCAL) — он находит коллекции, в которые производится только запись. Наш детектор может находить более хитрые ситуации и обрабатывать некоторые пользовательские объекты. Обычно находится ненужный код, который остался после рефакторинга и может быть попросту удалён. Но иногда срабатывание детектора сигнализирует о каких-то глубоких проблемах в алгоритме. Простой пример с массивами из IDEA:

final int[] dx = new int[componentCount];
final int[] dy = new int[componentCount];
for (int i = 0; i < componentCount; i++) {
  final RadComponent component = myDraggedComponentsCopy.get(i);
  dx[i] = component.getX() - dropX;
  dy[i] = component.getY() - dropY;
}

Массивы dx и dy заполняются, но нигде дальше не используются (вероятно, использовались раньше). Весь этот цикл не нужен, и его можно удалить.

А вот тут в Eclipse уже скорее алгоритмическая ошибка:

public void setProjectSettings(IJavaProject project, String destination, String antpath, String[] hrefs) {
    ProjectData data= fPerProjectSettings.get(project);
    if (data == null) {
        data= new ProjectData(); // UC_USELESS_OBJECT
    }
    data.setDestination(destination);
    data.setAntpath(antpath);

    StringBuffer refs= new StringBuffer();
    ...
    data.setHRefs(refs.toString());
}

ProjectData — POJO-класс, и FindBugs в состоянии увидеть, что его сеттеры не меняют глобального состояния системы, модифицируя только сам объект. Стандартный путь выполнения (без захода в if) вопросов не вызывает. Но если мы зашли в if, то дальнейшая часть метода не имеет смысла, так как новый объект никуда не сохраняется. Видимо, предполагалось после создания сохранить его в Map fPerProjectSettings.

RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT

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

Вот пример из IDEA:

protected List<File> getFiles(@Nullable AntDomPattern pattern, Set<AntFilesProvider> processed) {
  ...
  if (singleFile != null && singleFile.isDirectory()) {
    Collections.singletonList(singleFile); // RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT
  }
  return Collections.emptyList();
}

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

Ещё пример из IDEA (не потому что там много ошибок, а потому что сотрудники JetBrains читают этот пост):

protected void doOKAction() {
  SimpleNode node = getSelectedNode();
  if (node instanceof NullNode) node = null;

  if (node != null) {
    if (!(node instanceof MavenProjectsStructure.ProjectNode)) {
      // RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT
      ((MavenProjectsStructure.MavenSimpleNode)node).findParent(MavenProjectsStructure.ProjectNode.class);
    }
  }
  myResult = node != null ? ((MavenProjectsStructure.ProjectNode)node).getMavenProject() : null;

  super.doOKAction();
}

Кажется, хотели подписать node = слева.

Забытый return или присваивание, пожалуй, самые частые причины этого сообщения. Ещё бывает, что программист вызывает метод, думая, что он что-то меняет, а он ничего не меняет. Например, в Apache POI вызывают метод setBoolean, разумно предполагая, что он что-то устанавливает. А он просто вычисляет новое значение и возвращает его (и кто только так метод назвал!).

Случается, что программист вызвал не тот метод, который хотел. Так в самом FindBugs мы нашли и исправили ошибочное использование BitSet.intersects() (проверяет, пересекаются ли два множества) вместо BitSet.and() (пересекает два множества, изменяя первое).

UC_USELESS_VOID_METHOD

Это следствие из предыдущего детектора: так как мы научились неплохо определять, есть ли у метода побочный эффект, мы можем сообщить о непустых void-методах, которые таких эффектов не имеют. Чаще всего это какой-нибудь недописанный код с TODO посередине, но удаётся найти и поистине странные вещи. Вот, к примеру, бесполезный метод из Eclipse:

private void pruneEmptyCategories(CheatSheetCollectionElement parent) {
    Object[] children = parent.getChildren();
    for (int nX = 0; nX < children.length; nX++) {
        CheatSheetCollectionElement child = (CheatSheetCollectionElement) children[nX];
        pruneEmptyCategories(child);
    }
}

Выполняется рекурсивный обход древовидной структуры и больше ничего.

А вот ещё метод merge класса SortedSet в Eclipse:

public void merge(SortedSet other) {
    Object[] array = fKeyHolder.getKeys();
    Object[] otherarray = other.fKeyHolder.getKeys();
    if (otherarray == null)
        return;
    if (array == null) {
        array = otherarray;
        return;
    }
    int ithis = 0, iother = 0, i = 0;
    int mthis = array.length, mother = otherarray.length;
    Object[] tmp = new Object[mthis + mother];
    while (ithis < mthis && iother < mother) {
        int comp = fComp.compare(array[ithis], otherarray[iother]);
        if (comp <= 0) {
            tmp[i++] = array[ithis++];
        } else {
            tmp[i++] = otherarray[iother++];
        }
    }
    while (ithis < mthis) {
        tmp[i++] = array[ithis++];
    }
    while (iother < mother) {
        tmp[i++] = otherarray[iother++];
    }
}

Можете ли вы заметить, что этот метод из 26 строк с тремя циклами и тремя if'ами на самом деле не несёт никакой пользы? FindBugs теперь может.

Заключение

Обновляйте FindBugs, и вы, возможно, найдёте в своих проектах проблемы, о которых раньше не подозревали. Плагин для Eclipse доступен у нас на сайте. Плагин для IDEA тоже обновлён. Не забывайте сообщать нам о ложных срабатываниях и других найденных проблемах.

Автор: lany

Источник