Как многие помнят, некоторое время я развивал статический анализатор Java-байткода FindBugs. Однако проблем в FindBugs накопилось столько, что я решил, что будет проще написать новый анализатор байткода. Я не очень творчески назвал его HuntBugs. Разработка ведётся на GitHub. Он пока в ранней стадии разработки, иногда глючит и покрывает примерно 35% диагностик из FindBugs, но при этом добавляет свои интересные штуки. Попробовать можно на вашем Maven-проекте с помощью команды mvn one.util:huntbugs-maven-plugin:huntbugs (отчёт пишется в target/huntbugs/report.html). Альтернативно можно собрать вручную из гита и запустить приложение командной строки one.util.huntbugs.HuntBugs, которому можно подавать на вход JAR-файлы или каталоги с .class-файлами.


Как-нибудь потом, когда проект несколько повзрослеет, я расскажу о нём более подробно. А в этой статье я покажу, чего интересного нашёл HuntBugs в IntelliJ IDEA Community Edition. Я скачал с официального сайта и поставил последнюю версию этой IDE, а затем натравил HuntBugs на файл lib/idea.jar, в котором почти всё и лежит. Я люблю тестировать статический анализ на IDEA, потому что это IDE, в которой самой есть очень неплохой статический анализатор и разработчики им явно пользуются. Интересно посмотреть, что остаётся после него.


Формат этой статьи не особо отличается от того, что делает PVS-Studio: ошибки, куски кода, объяснения. Разумеется, в статью вошло только самое интересное.


Field is assigned to itself


Как правило ошибок типа this.field = this.field уже никто не допускает, даже не самая новая IDE обычно о таких предупредит. Однако HuntBugs умеет смотреть немного глубже. Вот фрагмент кода:


  private int myLastOffsetInNewTree;
  ...

  private int getNewOffset(ASTNode node){
    int optimizedResult = haveNotCalculated;
    ...
    if (myLastNode == prev) {
        ...
        optimizedResult = myLastOffsetInNewTree;

        myLastNode = node;
        myLastOffsetInNewTree = optimizedResult;
        ...
    }
  }

Поле myLastOffsetInNewTree загружено в локальную переменную optimizedResult, а затем почему-то опять сохранено в поле, хотя за это время оно поменяться не могло. Последнее приваивание странное, либо его надо убрать, либо имелось в виду что-то другое.


An integer value is cast to float and passed to the rounding method


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


    final int width = icon.getIconWidth();
    final int height = icon.getIconHeight();
    final int x = (int)Math.ceil((actionButton.getWidth() - width) / 2);
    final int y = (int)Math.ceil((actionButton.getHeight() - height) / 2);

Здесь дважды используется округление вверх (Math.ceil), но аргументом в обоих случаях подаётся целое число, так как в Java деление целого на целое выдаёт целое (с округлением вниз). Вероятно, подразумевалось поделить на 2.0 или иным образом перейти к дробным числам перед делением.Если же текущее поведение устраивает, то (int)Math.ceil следует убрать: эта часть кода бесполезна.


Switch branch is unreachable due to expression range


Весьма любопытный фрагмент кода, который, видимо, кто-то когда-то автоматически сгенерировал, а теперь уже никто не понимает, правильно ли это и что там должно быть:


    int state = getState() & 0xF;

    tokenType = fixWrongTokenTypes(tokenType, state);
    if (...) {

      // TODO: do not know when this happens!
      switch (state) {
        case __XmlLexer.DOCTYPE:
          tokenType = XmlTokenType.XML_DECL_START;
          break;
      }
    }

Константа __XmlLexer.DOCTYPE имеет значение 24, но выше выполняется state = getState() & 0xF, поэтому значение state может быть только от 0 до 15 и ветка switch гарантировано не выполнится. Возможно, когда в очередной раз меняли исходный файл лексера, константы были перегенерированы с другими значениями, а этот файл перегенерировать забыли. Так или иначе, код весьма подозрительный, о чём свидетельствует и комментарий.


Synchronization on getClass() rather than class literal


Этот фрагмент класса MatcherImpl синхронизируется на getClass(). Причём это делается в публичном нефинальном классе, у которого реально есть подкласс Matcher. В результате при выполнении этого кода из подкласса синхронизация будет происходить не по MatcherImpl.class, а по Matcher.class. Проблему усугубляет то, что в этом же классе есть явная синхронизация по MatcherImpl.class и обе критические секции (которые могут оказаться не взаимоисключающими) обновляют одно и то же статическое поле lastMatchData. В результате весь смысл синхронизации теряется. Обычно synchronized(getClass()) — это неправильно, используйте явный литерал класса synchronized(MatcherImpl.class).


Exception created and dropped rather than thrown


Довольно частая в Java ошибка: объект исключения создан, но не бросается. Например, здесь:


public void remove() {
  new OperationNotSupportedException();
}

О таких ситуациях IDEA сама тоже предупреждает. Ещё аналогичное место.


Invariant loop condition


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


boolean r = ...;
while (r) {
  if (!value(b, l + 1)) break;
  if (!empty_element_parsed_guard_(b, "json", c)) break;
  c = current_position_(b);
}

Здесь цикл с условием по локальной переменной r, значение которой в цикле не меняется, поэтому либо в цикл вообще не зайдём, либо никогда не выйдем по условию (только по break). Если это действительно подразумевалось, то в таких случаях лучше писать if(r) { while(true) { ... } }, чтобы подчеркнуть намерение сделать бесконечный цикл.


The switch operator has identical branches


Дублирующиеся ветки оператора switch иногда выглядят разумно, но когда в них большой кусок кода, как здесь, стоит приглядеться:


switch ((((PsiWildcardType)x).isExtends() ? 0 : 1) + (((PsiWildcardType)y).isExtends() ? 0 : 2)) {
case 0: /* ? super T1, ? super T2 */
     if (constraints != null && xType != null && yType != null) {
       constraints.add(new Subtype(yType, xType));
     }
     return balance(xType, yType, balancer, constraints);

case 1: /* ? extends T1, ? super T2 */
     if (constraints != null && xType != null && yType != null) {
       constraints.add(new Subtype(xType, yType));
     }
     return balance(xType, yType, balancer, constraints);

case 2: /* ? super T1, ? extends T2*/
     return null;

case 3: /* ? extends T1, ? extends T2*/
     if (constraints != null && xType != null && yType != null) {
       constraints.add(new Subtype(xType, yType));
     }
     return balance(xType, yType, balancer, constraints);
}

Не сразу заметно, но case 1 и case 3 абсолютно одинаковы (и отличаются от case 0). Если это и имелось в виду, возможно, разумнее объединить case 1 и case 3, чтобы было проще читать и поддерживать код.


The same condition is repeatedly checked


В этом коде зачем-то одно и то же условие проверяется два раза:


if (offsetToScroll < 0) {
  if (offsetToScroll < 0) {
    ...
  }
}

Может, просто внутреннюю проверку надо убрать, а может и что-то другое хотели проверить. Вот ещё аналогичный случай. Или вот ещё интересный случай:


        return o instanceof PsiElement && ((PsiElement)o).isValid() && ((PsiElement)o).isPhysical() ||
               o instanceof ProjectRootModificationTracker ||
               o instanceof PsiModificationTracker ||
               o == PsiModificationTracker.MODIFICATION_COUNT ||
               o == PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT || // <<<
               o == PsiModificationTracker.OUT_OF_CODE_BLOCK_MODIFICATION_COUNT || // <<<
               o == PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT;

А вот тут повторные условия не совсем рядом и их заметить ещё сложнее:


return SUPPORTED_TYPES.contains(token) || StdArrangementTokens.Regexp.NAME.equals(token) 
       || StdArrangementTokens.Regexp.XML_NAMESPACE.equals(token) || KEEP.equals(token)
       || BY_NAME.equals(token) || SUPPORTED_TYPES.contains(token);

Условие SUPPORTED_TYPES.contains(token) проверяется дважды. Разумеется, HuntBugs внимательно следит, чтобы между этими условиями ничего не поменялось. Если бы в промежуточных условиях token переприсваивался, такая конструкция имела бы право на существование.


Numeric comparison is always true or false


Вот тут скорее просто избыточная проверка, чем реальная ошибка:


int size = myPanels.length;
final Dimension preferredSize = super.getPreferredSize();
if (size >= 0 && size <= 20) {
  return preferredSize;
}

В переменную size записана длина массива, которая никогда не может быть отрицательной. Неясно, зачем проверять size >= 0. Даже если ошибки нет, я считаю, что такие проверки надо удалять, потому что они смущают читателя. Неизвестно, может автор подразумевал size > 0, тогда это ошибка.


The chain of private methods is never called


Обычно IDE без труда находят приватные методы, которые никогда не вызываются, и предлагают их удалить. Но вот такой случай определяется не всегда:


@Nullable
private static JsonSchemaObject getChild(JsonSchemaObject current, String name) {
  JsonSchemaObject schema = current.getProperties().get(name);
  if (schema != null) return schema;

  schema = getChildFromList(name, current.getAnyOf()); // <<<
  if (schema != null) return schema;
  ...
}

@Nullable
private static JsonSchemaObject getChildFromList(String name, List<JsonSchemaObject> of) {
  if (of == null) return null;
  JsonSchemaObject schema;
  for (JsonSchemaObject object : of) {
    schema = getChild(object, name); // <<<
    if (schema != null) return schema;
  }
  return null;
}

Эти два приватных метода вызывают друг друга рекурсивно, но вот снаружи их никто не вызывает. HuntBugs видит эту ситуацию и говорит, что оба метода на самом деле не используются.


Useless String.substring(0)


Честно говоря, не ожидал увидеть такую диагностику в production-коде, слишком уж она тривиальная. Но нет, бывают и тривиальные ошибки:


String str = (String)value;
if (str.startsWith("\"")) {
  str = str.substring(0);
  str = StringUtil.trimEnd(str, "\"");
}

Видимо, автор подразумевал удалить первый символ строки, но по какой-то причине написал не substring(1), а substring(0) (этот вызов просто возвращает исходную строку). Это второй случай (помимо dropped exception), когда сама IDEA тоже подсвечивает проблемное место.


Result of integer multiplication promoted to long


Это предупреждение не всегда ведёт к реальной опасности, но тем не менее хочется показать пример:


final long length = myIndexStream.length();
long totalCount = length / INDEX_ENTRY_SIZE; // INDEX_ENTRY_SIZE = 26
for(int i=0; i<totalCount; i++) {
  final long indexOffset = length - (i + 1) * INDEX_ENTRY_SIZE;

Во-первых, уже подозрительно, что переменная цикла имеет тип int, а не long (возможно, на такую ситуацию стоит сделать отдельную диагностику). Если totalCount превышает 231, то цикл никогда не завершится. Но ладно, это возможно только при длине индекса length больше 52 гигабайт, что всё-таки немало. Однако проблемы в этом коде начнутся уже при длине больше 2 гигабайт. Так как i и INDEX_ENTRY_SIZE имеют тип int, то умножение будет выполняться над 32-битными знаковыми целыми и успешно переполнится. Уже после этого результат умножения будет приведён к long и после выполнения вычитания смещение может вполне оказаться больше длины. Вероятно, таких больших кэшей никогда тут не было, но будет неприятно, когда они появятся. Исправить просто — объявить переменную цикла long.


А что с Котлином?


Известно, что часть IntelliJ IDEA написана на Котлине, который также компилируется в Java-байткод. Статические анализаторы байткода формально могут анализировать любой язык, но фактически если анализатор заточен на Java, то для других языков будет много ложных срабатываний. Часто они берутся оттого, что компилятор языка генерирует какие-то специфические конструкции (например, неявные проверки). Иногда, впрочем, такое ложное срабатывание — повод приглядеться к кодогенератору компилятора. Вот, например, класс com.intellij.configurationStore.FileBasedStorageKt. В самом классе есть такая строка:


private val XML_PROLOG = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>".toByteArray()

В классе java.lang.String метода toByteArray(), как известно, нет. Это extension-method Котлина, причём inline-метод (который компилятор встраивает прямо в место использования), по умолчанию он выполняет String.getBytes(Charsets.UTF_8). Давайте посмотрим, во что эта строка скомпилировалась в Котлине. Я не буду показывать прямо байткод, а преобразую его в более понятный код на Java:


String str = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
Charset charset = null;
int mask = 1;
Object obj = null; // зачем нужна эта переменная и какого она типа — я не знаю
// здесь зачем-то nop - пустая операция
if(obj != null) {
    throw new UnsupportedOperationException("Super calls with default arguments not supported in this target, function: toByteArray");
}
if(mask & 1 == 0) {
    charset = kotlin.text.Charsets.UTF_8;
}
// здесь опять зачем-то nop
XML_PROLOG = kotlin.jvm.internal.Intrinsics.checkExpressionValueIsNotNull(((String)str).getBytes(charset), 
                                              "(this as java.lang.String).getBytes(charset)");

Видно, что строка разрослась неимоверно. Переменная mask связана с передачей дефолтного параметра (про это рассказывал Дмитрий Жемеров на JPoint — смотрите слайд 40 и ниже. Здесь, очевидно, много лишнего, и HuntBugs справедливо ругается и на obj != null (сравнение null с null), и на mask & 1. Хотя автор кода совершенно не виноват. Надо полагать, со временем компилятор Котлина будет умнее и будет генерировать меньше мусора.


Заключение


Здесь можно написать обычный текст о важности статического анализа, который пишет Andrey2008 с коллегами после своих статей, но вы и так всё знаете. Интересно, что даже в коде, который разрабатывается с использованием статического анализа, удалось найти немало подозрительных мест, просто проверив его новым инструментом. Разумеется, в статью попало не всё. Помимо ложных сработок есть немало сообщений важных, но скучных. Много сообщений про производительность. Например, конкатенация строк в цикле — 59 штук. Или обход значений Map через keySet()+get(), когда быстрее через values() — 18 штук. Большое количество потенциальных проблем с многопоточностью. Скажем, неатомарные обновления volatile-полей — 50 штук. Или подозрительные сценарии использования wait()/notify() — 8 штук.


Пользуйтесь статическим анализом и следите за новостями!

Поделиться с друзьями
-->

Комментарии (46)


  1. EvgeniyRyzhkov
    25.05.2016 09:32
    +4

    Good News, Everyone!

    Культура использования статических анализаторов в России по странной причине находится в зачаточном состоянии. И чем больше проектов и статей на эту тему, тем лучше всем.

    Для старта обсуждения — какой парсер используете? Чур в исходники не посылать! Хочется общаться с человеком, а не с github.


    1. lany
      25.05.2016 09:35
      +4

      Спасибо за комментарий. Это анализатор байткода, а не анализатор исходного кода, поэтому нужен парсер байткода. Для построения модели по байткоду я пользуюсь Procyon Compiler Tools — весьма приятная штука. Есть баги, но автор их оперативно исправляет. Альтернативно думал попробовать FernFlower от JetBrains, но остановился на проционе.


      1. EvgeniyRyzhkov
        25.05.2016 09:37

        А в чем разница при анализе исходного кода или байт-кода? Какие плюсы и минусы у каждого варианта?


        1. lany
          25.05.2016 09:48
          +5

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


          while(x > 0) {
             ... что-нибудь
             return;
          }

          Безусловный выход в конце цикла — повод задуматься. Но анализатор байткода не может этого увидеть, потому что компиляторы выдают тот же байткод, что и для этого вполне разумного кода:


          if(x > 0) {
             ... что-нибудь
             return;
          }

          Так что, к сожалению, не всё можно сделать.


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


          Думаю, идеальный анализатор Java-кода должен сочетать и байт-код-анализ и анализ исходников (если они есть). Байт-код анализ пригодится для подгрузки скомпилированных библиотек и сбора статистики по ним. Например, важная информация: производит ли метод побочные эффекты. Есть очень много простых методов (геттеры, например), и не хочется при вызове любого из них сбрасывать весь контекст анализа. А так мы с помощью анализа байткода заглянули, что этот метод определён в библиотеке, нигде в дочерних классах не перегружен, и просто возвращает поле. Всё, после этого мы знаем, что вызов этого метода не испортит статически-выведенные факты о полях или каких-то объектах из кучи, и этим фактам можно и после вызова доверять.


          1. EvgeniyRyzhkov
            25.05.2016 09:53

            Спасибо за подробный ответ! Что-то я правда не знаю анализаторов, сочетающих оба подхода. Кто-то может такие назвать?


            1. lany
              25.05.2016 09:57
              +1

              Я тоже не знаю. В HuntBugs изначально заложен такой потенциал, но реализуется ли он когда-нибудь — неизвестно. Проект-то некоммерческий.


            1. VladimirKochetkov
              25.05.2016 12:23
              +2

              Мы (Application Inspector) с год назад сделали у себя proof-of-concept сквозной абстрактной интерпретации исходников и байт-кода в модуле анализа .NET-приложений, но в итоге остановились на варианте с декомпиляцией байт-кода в исходники и сведении задачи к предыдущей. В основном потому, что проблемы работы с исходниками, описанные lany выше (типизация узлов AST, детектирование внешних вызовов и побочных эффектов и т.п.), на тот момент уже были решены для наших абстрактных интерпретаторов Java и C#, как и проблема построения модели символического выполнения по невалидному коду. С другой стороны, эта модель, будучи построенной по декомпилированному байт-коду, вполне себе позволяет проводить анализ control/data-flow кейсов, информация о которых теряется при компиляции (как в примере с выходом из цикла) за счет того, что описывает все возможные потоки вычисления, по которым достаточно легко определить, находимся ли мы в условии выхода из цикла/рекурсии или же просто в условном операторе, например.

              Поэтому, если говорить о статанализе, основанном на абстрактной интерпретации, особых преимуществ у байт-кода перед исходниками, IMO, нет.


            1. VladimirKochetkov
              25.05.2016 12:30
              +2

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


              1. EvgeniyRyzhkov
                25.05.2016 12:53
                +4

                Владимир, привет!

                Раз уж пошла перепись разработчиков анализатора кода, то расскажу про PVS-Studio. Для C# кода мы анализируем именно исходники, а не байт-код. Наша мотивация была такой:

                1. Мы считаем, что в исходнике больше информации, нежели в байт-коде.
                2. Мы умеем работать с исходным кодом, так как есть опыт с C++,.
                3. Мы делаем ставку на C# и бонус в виде легкой проверки других языков нам не очень актуален.

                По этой причине решили работать именно с исходниками.


        1. gibson_dev
          25.05.2016 09:48
          +4

          На java машине есть много языков, и один анализатор байт-кода покроет их все, а анализатор языка — один.


          1. lany
            25.05.2016 09:50
            +4

            Ну да, адаптировать анализатор байткода к новому языку проще. По идее он может даже из коробки приносить пользу. Но, как я написал в статье, количество ложных сработок изначально будет неимоверно выше. Однако бывают не только новые языки, а всякие ещё инструментаторы, annotation-процессоры. Анализатор байткода получит уже реальный код после их работы, который может существенно отличаться от Java-исходника.


        1. 23derevo
          25.05.2016 09:51

          исходный анализировать сильно-сильно сложнее.


          1. EvgeniyRyzhkov
            25.05.2016 09:54

            Почему?


            1. 23derevo
              25.05.2016 09:59

              потому что код на Java существенно сложнее байткода.


              1. Maccimo
                25.05.2016 10:08

                Не обязательно же самому парсер писать.
                Например, есть некоторое движение в сторону поддержки статических анализаторов в javac: http://openjdk.java.net/jeps/190.


                1. lany
                  25.05.2016 10:21
                  +2

                  По моим ощущениям, практических результатов по этому JEP не стоит ждать в ближайшие 5 лет…


          1. lany
            25.05.2016 09:56
            +2

            Думаю, если взять в качестве движка Eclipse JDT, то в принципе на нём не так сложно наколбасить многие диагностики. Но не пробовал.


        1. KvanTTT
          25.05.2016 13:28
          +1

          Добавлю и свое мнение: с помощью байт-кода не получится детектить такие вещи, как известный баг goto fail;, анализировать строки в комментариях, потому что информации о пробелах и комментариях, понятное дело, в байт-коде нет. Насколько помню, ваш анализатор PVS-Studio также способен анализировать такие вещи. Ну и как написали выше в комментариях, при компиляции теряется еще и другая информация, без которой также невозможно будет выявить определенные баги.


          Зато с помощью байт-кода удобно проводить глубокий анализ, в котором анализируются связи между определением переменных и их использованием (потоки данных), проводится межпроцедурный анализ, и т.д. В нем нас по сути интересуют самые базовые вещи, характерные для существующих языков программирования, такие как последовательность, ветвления, циклы, вызовы методов.


          Еще одним преимуществом анализа исходников, а не байт-кода, является то, что если исходники содержат какие-либо синтаксические или семантические ошибки, то байт-код извлечь не получится. А ситуации, в которых что-то не компилится, не ресолвится, не хватает файлов и т.п., могут возникать.


          1. KvanTTT
            25.05.2016 13:43
            +2

            С goto fail не очень удачный пример. Но вот хороший пример с пробелами и забытыми фигурными скобочками из статьи про проверку FreeBSD:


            image


    1. leventov
      25.05.2016 15:33

      Кивание на немытую Россию тут совершенно неуместно. Уверен, если попробовать запустить huntBugs на кодовой базе заграничного Eclipse, он просто задымится и сломается от количества найденных багов


      1. EvgeniyRyzhkov
        25.05.2016 15:35

        Я совсем не про количество ошибок. Я про то, что в мире анализаторы кода давно знают и используют, а в России — нет. Просто люди не в курсе, что есть такие инструменты и их стоит использовать.


        1. leventov
          25.05.2016 15:42
          +3

          Если бы в Eclipse использовали анализаторы кода, наверное там было бы меньше ошибок.


          Не очень понятно, почему вы сделали такое далекоидущее обобщение в комментариях к статье об IntelliJ, проекте, где (думаю не совру) о качестве кода и анализе думают больше, чем в 95% проектов "в мире".


          1. EvgeniyRyzhkov
            25.05.2016 15:46

            Не знаком с кодом Eclipse, не могу ничего про него сказать.


        1. igorp1024
          26.05.2016 12:45

          Любой вменяемый джавист из используемых тулзов Вам выдаст сходу как минимум модный SonarQube. Или PMD, FindBugs, если он достаточно долго в этой отрасли. Проблема, думаю, не в России, а рискну предположить, что в отдельных сферах разработки.


  1. damintsew
    25.05.2016 14:13
    +1

    Очень интересный анализатор! Запущу сегодня — проверить проект.

    По истории коммитов у вас они идут по несколько в день. Вы вечерами его дорабатывается помимо основной работы?
    Честно говоря интересуют техники «совмещения» ибо у меня не всегда хватает сил/времени на свои проекты.


    1. lany
      25.05.2016 14:29
      +4

      Иногда на работе удаётся немного пописать. Иногда дома. Главное не играть в игры и не смотреть фильмы, тогда куча времени на программинг появляется. Плюс я обычно думаю не за компом, а за компом только материализую мысли и отлаживаю их. Можно по дороге с работы домой придумать новый кусок кода достаточно детализированно, а дома его просто вбить из головы.


  1. TheKnight
    25.05.2016 14:54
    +3

    А можно подробнее про проблемы FindBugs, которые настолько сильно мешают, что понадобилось начинать свой анализатор?


    1. lany
      25.05.2016 15:05
      +5

      Во-первых, в него очень глубоко проник старый BCEL, который даже восьмую джаву с большим трудом поддерживает. Сам по себе BCEL — не сахар, а новый несовместим со старым. Если переходить на новый, всё равно изменится полностью всё API и придётся переписывать сторонние плагины. Во-вторых, там много геологических слоёв накопилось. Например, есть кусок, который использует ASM, в то время как основная часть использует BCEL. В этом можно запутаться сильно. В-третьих там огромный глобальный контекст. Куча всего хранится в статических полях, что усложняет интеграцию со сторонними инструментами, утяжеляет контроль за памятью и не даёт никакого шанса на нормальное распараллеливание. В-четвёртых, система приоритетов, которую не до конца вытеснила система рангов (у которой тоже есть проблемы) и сейчас они живут вместе, заставляя пользователей чесать репу при попытке понять, какой баг серьёзнее. В-пятых, способ обхода классов детекторами ведёт к повторным действиям и к неэффективному расходу памяти. Продолжать можно долго.


      Но самое главное — FindBugs очень низкоуровневый. Практически любой анализатор говорит напрямую с байткодом, а не с какой-то более высокоуровневой моделью. Это сильно усложняет код, делая его stateful, и делает анализ менее устойчивым во многих случаях. Введёте промежуточную переменную и что-то поломается. Или, чтобы не поломалось, приходится кусок анализа засовывать прямо в OpcodeStack, что ещё более ужасно. Вон почитайте какой-нибудь класс детектора и увидите, как всё неудобно.


      Не последняя проблема в том, что FindBugs по факту умирает. Из трёх коммитеров за последний год активность проявлял только я. В основном — мелкие фиксы и приём очевидных пулл-реквестов. Мне на себе тащить эту гору легаси неохота, приятнее начать новый проект.


      1. TheKnight
        25.05.2016 19:14

        Спасибо!


  1. agzamovr
    25.05.2016 15:46
    +1

    Плагин для gradle планируется?


    1. lany
      25.05.2016 16:00
      +1

      Обещал один товарищ написать, пока не пишет.


      1. tolkkv
        25.05.2016 20:09
        +3

        На выходных должен сделать :)


  1. udalov
    25.05.2016 15:57
    +3

    Спасибо! Про кодогенерацию в Котлине завёл issue: https://youtrack.jetbrains.com/issue/KT-12497


  1. evkin
    25.05.2016 20:05

    А каковы планы по поводу (плагинов) интеграции в CI? в тот же jenkins/hudson


    1. webkumo
      25.05.2016 20:36

      Так вам же дали готовый мавен-артефакт… Неужели его так сложно прикрутить в этих CI?


      1. evkin
        26.05.2016 00:12

        Вопрос в визуализации данных и изменений относительно различных сборок. На выходе та же кроме report.html генерируется хмл файлик, как в остальных подобных мавен плагинах (тот же junit, Cobertura, FindBugs и т.п.) Но вот вопрос парсинга и отображения этих данных в CI остается открытым (что делают соответствующие плагины CI). Например, моих скилов не факт, что хватит что бы разобраться и реализовать данный функционал для того же jenkins, взяв за основу даже готовые плагины для примера… А вот иметь из коробки, хочется))


    1. lany
      26.05.2016 04:55

      Пулл-реквесты принимаются :-)


  1. marxfreedom
    26.05.2016 04:55

    Еще бы плагин для IDEA, но понимаю что на все нужны силы :)


    1. tolkkv
      26.05.2016 09:10

      Он нужен только для того чтобы перейти к строчке в которой ошибка? Если так то я думаю можно решить проблему и другими способами, подумаю :)


      1. marxfreedom
        26.05.2016 12:15
        +2

        Я думаю даже одна такая функция очень полезна, т.к. экономит время.
        Хотя в findbugs меня всегда напрягало настаивать проект, указывая каталоги с исходниками и с зависимостями. С плагином я об этом забыл — в пару кликов можно проанализировать файл, модуль или весь проект и результаты смотреть в том же окне IDE.


    1. webkumo
      26.05.2016 10:36

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


      1. lany
        26.05.2016 10:39
        +1

        Номера строк вытаскиваются из отладочной информации. Конкретную позицию в строке не покажет, но номер строки обычно неплохо определяется. В общем-то FindBugs-плагин же существует, это из той же оперы.


        1. kuaw26
          26.05.2016 14:13

          Поддержу про плагин к IDEA, пользуюсь FindBugs-плагином — очень удобно.
          Интересно, а его за основу можно взять и «подменить» движок и заменить икноки?


          1. lany
            26.05.2016 14:36

            Думаю, не всё так просто. У меня отличается концепция ранжирования, категоризации варнингов, генерации отчёта. Информация о прогрессе анализа по-другому передаётся. Ну и фильтров пока совсем нет. Можно отключить определённый варнинг целиком, но нельзя отключить, например, в конкретном классе. В FindBugs есть управление отключением детекторов, а я этого делать не хочу: отдельные детекторы — это деталь реализации, чтобы отключить детектор, надо просто отключить все варнинги, которые он умеет генерировать (детектор реально отключится, анализ может быстрее стать). В общем, отличий достаточно много, чтобы взять и переиспользовать. Плюс проект пока молодой и я не могу гарантировать стабильность API. Товарищ взялся писать плагин для Eclipse (вроде в минимальном виде уже работает, но я не пробовал). Если не забросит, на нём обкатаем API, тогда уже будет понятно, что с IDEA делать.


  1. relgames
    26.05.2016 15:30

    HuntBugs report
    Warnings (0)

    Окей…

    А оно не умеет аггрегировать отчеты от подмодулей в Maven? Не очень удобно смотреть в каждом модуле, если модулей много.


    1. lany
      26.05.2016 15:37

      Не умеет пока. Записал, чтобы не забыть, спасибо.