company_banner

Кто умнее чем IDEA?

    Два года назад я вызвался постоять на стенде нашей компании JetBrains на последней конференции JBreak в Новосибирске. Перед конференцией мне спустили сверху вот такие карточки:



    И сказали, мол, ну раздай каким-нибудь людям на конференции на своё усмотрение. Я запаниковал. Как же я буду людей-то выбирать?


    Тогда я довольно плотно работал с анализом потока данных в статическом анализаторе IntelliJ IDEA для Java. Во-первых, я обкатывал новые фичи, проверяя код самой IDE. Во-вторых, разгребал входящие баг-репорты. Иногда IDEA находила удивительные проблемы, и мне приходилось долго разбираться, чтобы вообще понять, правильное ли предупреждение она выдаёт или это баг.


    Пока шёл первый доклад конференции, я решил из этого материала быстренько сварганить игру. Набрал пачку свежих примеров из головы (в основном из реального кода). В каждом случае стояла задача объяснить предупреждение, которое выдаёт IDEA, или обосновать, что предупреждение ложное. За правильные ответы я и раздавал трёхмесячные лицензии на все наши десктопные продукты.


    Теперь я предлагаю те же самые задачки решить вам. Многие простые, на внимательность. Но есть такие, где надо хорошенько подумать.


    1 Обход списка



    Отгадка

    Предупреждение верное. Мы не знаем ни длину входного списка args, ни длину массива parameters. Однако сколько бы ни было параметров на первой итерации цикла i == 0, а длина списка не может быть меньше нуля по контракту. Это означает, что до второй итерации дело никогда не дойдёт, потому что исключение вылетит уже на первой.


    2 Наилучшая точка



    Отгадка

    Предупреждение верное. Массив variants определён прямо здесь и имеет четыре элемента. Значит, мы точно зайдём в цикл. На первой итерации if (best == null || distance > d) совершенно точно истинно, потому что в начале best == null. Значит, best = variant будет точно выполнено. А variant, конечно, никогда не null. Даже если бы мы не видели инициализаторов массива, мы можем это сказать потому что был вызван метод variant.distance.


    3 Свойства драйверов



    Отгадка

    Предупреждение ошибочное. В переменной value действительно может быть null, но в этом случае массив result будет пустым, и мы в цикл никак не зайдём. IDEA 2018.1 не понимала таких тонкостей, но в скором времени мы её научили. Теперь этого предупреждения нет.


    4 Нулл или не нулл?



    Отгадка

    Этот код прислал пользователь в наш баг-трекер, сказав, что IDEA выдаёт ложное предупреждение. На самом деле предупреждение верное. Входной массив имеет заведомо ненулевую длину, операция map не меняет число элементов стрима (возможно, автор хотел написать filter), а значит, операция findFirst точно что-то найдёт и ветка orElse(null) никогда не выполнится.


    5 Инициализация поля



    Может быть все дочерние классы по контракту устанавливают поле name, и предупреждение ложно?


    Отгадка

    Предупреждение в целом верное. Только стоит писать не "may produce", а "will produce" (в свежих версиях это исправлено). У нас нет шанса инициализировать поле name, какая бы ни была реализация метода init(). Дело в том что объект this не утекает из конструктора. Метод init() вызывается, но на родительском объекте, который передан параметром. Из него мы никак не сможем доступиться до объекта, который сейчас конструируется. Да, init() может устанавливать name родительского объекта, но name текущего ему не доступен, а значит, мы можем быть уверены, что после вызова init() там всё ещё null.


    6 Генерация имён классов



    Отгадка

    Предупреждение верное. Я разбирал этот случай в отдельной статье "Статический анализ → уязвимость → профит"


    7 Индикаторы



    Отгадка

    Предупреждение ложное. Видно, что подсвеченный метод вызовется только при indicator == 2, а это возможно только если и riseBinding, и sinkBinding не равный null. С тех пор, конечно, IDEA поумнела и нормально разбирается в таком коде, не выдавая предупреждения.


    8 Лямбда с побочным эффектом



    Отгадка

    Предупреждение верное. Мы видим лямбду, в которой exception[0] может перезаписаться заведомо ненулевым исключением. Проблема однако в том, что до условного оператора лямбда ни разу не могла быть выполнена. Она используется после условия. Соответственно, exception[0] не мог быть перезаписан. Вероятно, условие и последующий вызов следует переставить местами.


    9 Груви-туплы



    Отгадка

    Предупреждение верное. Тут всё просто: вместо i < initializers.length было написано initializers.length < i. Соответственно индекс массива всегда больше его длины. Учитывая, что этот код существовал давно, скорее всего неправильное условие никогда не выполнялось и до исключения дело не доходило. Интересно, что return в котором подсвечена ошибка — единственный полезный выхлоп всего условия if (parent instanceof GrTuple). То есть десять строчек этого метода никогда не делали никакой полезной работы.


    10 Префиксы



    Отгадка

    Тоже несложно. filePrefix и linePrefix могут быть переприсвоены только одновременно. Им присваиваются элементы массивов filePrefixes и linePrefixes соответственно, в которых нет нуллов. То есть если filePrefix оказался не нуллом, то и в linePrefix нулла быть не может. Предупреждение верное, хотя в целом это не ошибка, просто перестраховочное условие.



    Надеюсь, задачки вам понравились. К чему я всё это вспомнил через два года? Всё потому что 29 февраля в Новосибирске пройдёт новая Java-конференция SnowOne. JetBrains также будет спонсировать эту конференцию, и я сейчас вовсю готовлю новую порцию задачек. Призы будут такие же — трёхмесячная лицензия на все продукты JetBrains. Приходите поиграть!

    JetBrains
    Делаем эффективные инструменты для разработчиков

    Similar posts

    Comments 42

      0
      Спасибо за статью. Раз уж речь про конференции зашла, то у меня такой вопрос: JetBrains Night в этом году будет?
        +2

        Есть такие планы, да.

        0

        3)


        В переменной value действительно может быть null, но в этом случае массив result будет пустым, и мы в цикл никак не зайдём.

        А если в массиве лежит 2 действительных значение, а 3-е — null, мы берём последний, передаём в конструктор и ловим НПЕ там?


        6) Мне кажется, что в этом примере должна быть ещё проверка на пустую строку, иначе можно IOOBE поймать:


        @Test
        void name() {
          String nextClassName = new a1().getNextClassName("azaza", "");
        }
        
        static class a1 {
          int classCounter;
          public String getNextClassName(String fullName, String shortName) {
            if (shortName == null) {
              return "class_" + classCounter++;
            }
        
            int index = 0;
            while (Character.isDigit(shortName.charAt(index))) { // <---
              index++;
            }
            if (index == 0 || index == shortName.length()) {
              return "class" + classCounter++;
            }
            return fullName;
          }
        }

        P. S. Посмотрел статью, на которую ты ссылаешься, там написано


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

        Как такое возможно, если в исходниках имя ява-класса не может состоять из цифр?

          +1

          P. P. S.


          Попробовал написать


          static class 35463 {
          }

          поймал исключение в "Идее" :D

            +1

            Спасибо за баг-репорт. Это новая фича in-place refactoring пока не отлажена, в 2020.1 EAP ещё часто падает. В 2019.3 таких проблем нет.

            +1

            Тогда ошибка будет другой (не Array access) и только в случае, если параметр помечен как @NotNull (либо на основе анализа Идея догадается, что он должен быть не null)

              0

              Вы правы )) я сегодня подтормаживаю иногда

                +2

                Вообще в этом случае мы всё равно по умолчанию ничего не напишем. Мы не предупреждаем обо всех возможных ошибках с nullability, иначе у вас бы был миллион мусорных предупреждений. Мы говорим только о вероятных ошибках. Про элемент массива мы бы сказали, если бы он был объявлен как @Nullable DriverPropertyInfo @Nullable [], то есть элементы были бы тоже аннотированы как Nullable.

              +1
              Как такое возможно, если в исходниках имя ява-класса не может состоять из цифр?

              Ключевая фраза — «по правилам виртуальной машины Java»


              JVMS §4.2.1:


              Class and interface names that appear in class file structures are always represented in a fully qualified form known as binary names (JLS §13.1). Such names are always represented as CONSTANT_Utf8_info structures (§4.4.7) and thus may be drawn, where not further constrained, from the entire Unicode codespace.

              Накодогенерировать можно класс с любым именем.

                0

                Любопытно, почему тогда нельзя в исходниках писать чисто цифровые имена классов?

                  0

                  Ты действительно хочешь жить в мире, где возможен подобный код? :-) Опасайтесь своих желаний!

                    0

                    Ну, мы уже живём в мире, где ??? — это обычная функция...

                      +2

                      Простите, не мы, а вы.

                      0

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

                        0

                        Метод и переменная очевидно не могут называться одними цифрами. Придумывать для типов и для переменных разный набор допустимых символов? Ради чего?

                          0

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


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

                      0
                      Думаю, примерно по той же причине, почему нельзя использовать ключевые слова в качестве имён переменных, полей, методов и классов.

                      Это усложнит парсер языка, отличить имя класса от числа на уровне лексера уже будет невозможно и придётся учитывать контекст на уровне парсера. При этом никаких значимых преимуществ мы не получим.

                      Можно, конечно, было изобрести отдельный синтаксис для таких имён, как это сделали в Kotlin:

                      object `314159265358` {
                      
                        @JvmStatic
                        fun main(args: Array<String>?) {
                          println("Hello, world!")
                        }
                      
                      }
                      


                      Но зачем нам в Java лишний синтаксический мусор?
                  +2
                  Спасибо, что подняли эту тему. Есть фича-реквест иметь возможность скрывать предупреждения в конкретном месте или определённого типа. Например, я могу часто писать так:
                  if (true && ...) {
                     ...
                  }
                  

                  или
                  return;
                  ....
                  

                  и это совершенно нормальный код на этапе отладки или прототипирования. Собственно, думаю что в режиме отладке или прототипирования у большинства проходит 80% работы и чистый код нужен только в самом конце. IDE же пытается поправлять каждую минуту, в том числе ещё не готовые решения, чем с одной стороны загружает процессор, занимая ресурсы, которые можно было бы потратить на какой-нибудь другой анализ, а с другой — отвлекает от разработки. Конечно в каких то случаях это очень полезные функции, но есть запрос иметь возможность в каких-то случаях их отключать, точечно или по правилам.

                  P.S.: Кроме того, статистика по отключаемым правилам поможет вам собрать данные на большей выборке, что разработчики считают лишним, а что действительно помогает в работе.
                    +2

                    Есть же возможность отключения через особые комментарии

                      +2
                      Засорять код для того, чтобы отделаться от IDE в режиме прототипирования? Чтобы внимание рассеивали не только подсветка в нежелательных местах, но и лишний текст? Нет, спасибо, нужно решение в GUI IDE, желательно одним простым чекбоксом, иначе из пушки по воробьям.
                        0
                        А powersave mode не помогает?
                        0

                        Это не на всех инспекция работает, некоторые неотключаемые в принципе. Может с Java плагином не так, но в PHP точно. И ладно если реальная ошибка, а то рабочий код, скопмрованный из мануала, ошибочным объявляет и даже на предупреждение уровень не сменить. Ошибка доступ к приватному свойству извне класса и всё тут. Про биндинг замыкания не слышали.

                          0

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

                      +6

                      В джаве return; посреди метода — это не предупреждение, а ошибка компиляции. Если вы совсем не хотите никаких ошибок видеть, а только подсветку синтаксиса, можно отключить гектора в статусной строке:



                      Только тогда уж оставьте анализ кода при коммите. А то очень часто бывает, что человек думает "ну я же прототипирую, сейчас буду писать коряво, потом исправлю", а в итоге этот корявый код так и живёт десять лет.


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


                      Кстати, вот вы написали if (true && ...), а ведь это бесполезно. Вы хотели сделать ветку всегда истинной, но в итоге не сделали ничего. Надо было написать if (true || ...). Если вы эту ошибку сделали в редакторе Хабра, вы могли её сделать и в редакторе кода без встроенного статического анализатора. А анализатор подсветит по-разному: в первом случае выделит только true, а во втором — всё условие. Опытный пользователь сразу заметит ошибку, даже если использует подобные условия для прототипирования. В любом случае это полезная визуальная напоминалка, что эта ветка выполнится всегда. Она пригодится при отладке.


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

                        0
                        Кстати, вот вы написали if (true && ...), а ведь это бесполезно.
                        Это может быть написано для возможности закомментировать разные части условия (или даже всё условие целиком).
                          0

                          Нет, не может быть. Это совсем бесполезно.

                            +4
                            Ну почему же бесполезно. Например, условие может выглядеть, как if (condition1 && condition2 && condition3), и нам нужно в процессе разработки включать-выключать разные части этого условия. Мы можем написать это в таком виде:
                            if (
                              true
                              && condition1
                              && condition2
                              && condition3
                            )
                            и включать-выключать части условия одним нажатием (однострочными комментариями).

                            Если же написать это в таком виде:
                            if (
                              condition1
                              && condition2
                              && condition3
                            )
                            то при закомментировании condition1 у нас будет ошибка синтаксиса, которую придётся отдельно исправлять.

                            При написании же в таком виде:
                            if (
                              condition1 &&
                              condition2 &&
                              condition3
                            )
                            ошибка синтаксиса будет при закомментировании condition3.

                            Ну т.е. с моей точки зрения, сценарий весьма специфический (это же не SQL, где такое сплошь и рядом), но вполне возможный.
                              0
                              if (
                                true
                                && condition1
                                && condition2
                                && condition3
                              )

                              Не проще ли в вашем примере (раз уж мы находимся в отладчике) просто поменять значение переменной condition1 вместо комментирования/раскомментирования и последующего перезапуска/пересборки?

                                0
                                Если мы находимся в отладчике, и если condition1 — переменная, то возможно и проще. Но почему вы решили, что всё именно так? :) Вместо condition1 скорее всего будет какое-то явно описанное условие, возможно даже сложное.
                                  +1

                                  Или вызов метода какого-то, может вообще из внешней зависимости

                              +3

                              Это бесполезно с точки зрения исполнения программы, но бывает полезным с точки зрения разработки и отладки программы.

                      • UFO just landed and posted this here
                          0
                          Condition 'best == null' is always 'false'

                          Это вторая проверка. При первой проверке переменная best действительно пустая, но так как в цикл мы в любом случае входим, то первая проверка best == null даёт вход в условный блок и запись объекта в best.

                          • UFO just landed and posted this here
                              0

                              Видать, не один я сегодня торможу )

                          +1
                          29 февраля в Новосибирске пройдёт новая Java-конференция SnowOne

                          … на которую уже "все билеты проданы". Обидно, блин :)

                            +1

                            Обидно, когда про конференцию узнаёшь из этой записи (

                            0

                            По 5 квизу — объяснение немного не про то


                            Предупреждение в целом верное. Только стоит писать не "may produce", а "will produce" (в свежих версиях это исправлено). У нас нет шанса инициализировать поле name, какая бы ни была реализация метода init(). Дело в том что объект this не утекает из конструктора. Метод init() вызывается, но на родительском объекте, который передан параметром. Из него мы никак не сможем доступиться до объекта, который сейчас конструируется. Да, init() может устанавливать name родительского объекта, но name текущего ему не доступен, а значит, мы можем быть уверены, что после вызова init() там всё ещё null.

                            Метод init там воообще ни при чём, там порядок вызова конструкторов важен. Т.е. при создании любого объекта дочернего класса — очерёдность вызова конструкторов такова, что первым будет вызван конструктор корневого класса, а значит определить пусть и публичную переменную в дочернем классе мы сможем только после вызова this.name.trim();

                              –1

                              Какие-то все объяснения запутанные.
                              В конструкторе потомка мы не можем передать this в качестве параметра в конструктор предка, в этом вся причина.


                              Во-первых нам не даст это сделать javac, а если сгенерируем класс каким-то другим способом и всё же передадим this в качестве parent, нас зарубит верификатор т.к. пытаемся передать ссылку на неинициализированный экземпляр класса.


                              Если передать из потомка this в конструктор Main, инициализировать name в init() потомка и отключить верификатор, то всё получится.

                              0
                              Тагир, спасибо за отличную дискуссию после выступления на Jetbrains Moscow 2019. Кстати, в этом году, похоже, подобной конференции не будет. Почему, не знаете?
                                0

                                Возможно, из-за неблагоприятной эпидемиологической обстановки. Но точно не знаю.

                              Only users with full accounts can post comments. Log in, please.