Pull to refresh

Comments 13

Подскажите, сработает ли проверка на классы из сторонних зависимостей, подключаемых мавеном, например, которые могут возвращать null?

Проверка срабатывает тот код, который компилируется в вашем проекте. Проверяется с учетом контракта сторонней зависимости.

Т.е. анализатор (в Idea или в maven-плагине) ругнется, если в сторонней зависимости есть аннотации Nonnull / Nullable (любой реализации; в стороннем проекте одна из аннотаций может быть настроена через ее отсутствие). Если сторонняя зависимость не использует эти аннотации, провека все еще может быть настроена через механизм astub вашего проекта, либо для статических методов стороннего проекта можно сделать классы хелперы-обертки с нужными аннотациями Nonnull / Nullable.

Вы можете потестить как поведет себя анализатор на примере сторонней зависимости, который можете подключить через maven. В зависимости расставлены аннотации Nonnull / Nullable. Также можно потестить на методах JDK, его контракт поставляется в org.checkerframework:checker-qual

Можно, конечно, настраивать IDEA и разбрасывать аннотации. А можно перейти на Kotlin.

Можно ещё подключить ErrorProne от гугла и плагин NullAway для него от Uber и получаем сразу статический анализатор + проверку на null в одном флаконе. Работает на этапе компиляции и довольно быстро, и не надо ничего дополнительно настраивать.

Эта схема переносится в maven, чтобы в CI прокинуть? Проверки переносятся согласовано (одинаковый набор ошибок в Idea и CI буду отображаться)?

Да. ErrorProne - это плагин для Maven/Gradle/Bazel, поэтому будет работать и локально и в CI с одними и теми же проверками и ошибками. А NullAway - это, получается, плагин для плагина.

NullAway по умолчанию считает, что все переменные и поля являются non-null, и компиляция будет падать с ошибкой если NullAway:

  • Найдёт место где вы пытаетесь присвоить null к такой переменной

  • Не может доказать, что поле всегда будет инициализированно каким-то значением

  • Вы пытаетесь обратиться к переменной, которая может быть null, без предварительной проверки на null

Мы для себя выработали такой подход: все non-null поля помечаем как final и используем ломбоковский @RequiredArgsConstructor, что бы сгенерировать конструктор принимающий эти поля. К остальным полям мы добавляем @Nullable аннотацию и проставляем значения через обычные сеттеры.
При этом, насколько я понимаю, для NullAway не важно какую именно @Nullable аннотацию вы используете (мы используем спринговую), главное что бы имя аннотации было Nullable.

Единственная проблема, с которой мы столкнулись, это спринговые @ConfigurationProperties:
если объявить поля такого класса как final и использовать @ConstructorBinding, то проперти становятся по факту иммутабельными. Но одно из требований нашего проекта - поддерживать обновление пропертей в рантайме, поэтому иммутабельные проперти нам не подходят. А если не использовать final и @ConstructorBinding, то NullAway ругается, что ему не удаётся доказать, что поля всегда будут инициализированны.
Наше решение - использовать @Validated над классом + javax.validation.constraints.NotNull над non-null полями, и добавлять @SuppressWarnings("NullAway.Init"), что бы NullAway не проверял инициализацию полей (но он всё-равно будет проверять попытки обратиться к @Nullable полям этого класса)

Кстати, Checker Framework - это также статический анализатор. Ваша схема очень похожа на описанную в статье.

Проверил вашу схему, без дополнительной настройки Idea - отображает только проверки 2 NPE из 10 (ссылаюсь на номера проверок в таблице из статьи), так же, как и говорится в статье.

После настройки Idea так, как указано в статье, будут отображаться все 10 потенциальных NPE.

Проверил чеки плагина. Настроил по документации

Конфигурация maven
<properties>
    <maven.compiler.source>11</maven.compiler.source>
    <maven.compiler.target>11</maven.compiler.target>
    <javac.version>9+181-r4173-1</javac.version>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>
...
<plugin>
    <artifactId>maven-compiler-plugin</artifactId>
    <version>3.8.0</version>
    <configuration>
        <showWarnings>true</showWarnings>
        <compilerArgs>
            <arg>-XDcompilePolicy=simple</arg>
            <arg>-Xplugin:ErrorProne -Xep:NullAway:ERROR -XepOpt:NullAway:AnnotatedPackages=org</arg>
        </compilerArgs>
        <annotationProcessorPaths>
            <path>
                <groupId>com.google.errorprone</groupId>
                <artifactId>error_prone_core</artifactId>
                <version>2.4.0</version>
            </path>
            <path>
                <groupId>com.uber.nullaway</groupId>
                <artifactId>nullaway</artifactId>
                <version>0.8.0</version>
            </path>
            <!-- Add any other annotation processors here,
                 even if they are also on the project dependency classpath. -->
        </annotationProcessorPaths>
    </configuration>
</plugin>

Результат NullAway: нашел 5 из 10 NPE. У Checker Framework, напомню, 10 из 10.

Проверки NullAway
// 1 проверка из таблицы в статье
[ERROR] Sub.java:[32,13] [NullAway] method returns @Nullable, but superclass method org.example.Base.nonNull(java.lang.Integer) returns 
// 8
[ERROR] Sub.java:[34,29] [NullAway] returning @Nullable expression from method with @NonNull return type
// 7
[ERROR] Sub.java:[41,63] [NullAway] assigning @Nullable expression to @NonNull field
// 4
[ERROR] Sub.java:[43,45] [NullAway] passing @Nullable parameter 'null' where @NonNull is required
// 10
[ERROR] Sub.java:[53,42] [NullAway] dereferenced expression i is @Nullable

Для полной уверенности, могли бы вы привести настройку maven в части ErrorProne + NullAway? Возможно вы действительно ничего не настраиваете и Idea отображает все проверки, и, возможно вы по-другому настраиваете maven/gradle, что NullAway находит все 10 потенциальных NPE. Было бы полезно перенять опыт, если так.

Идею вообще никак не настраивали и скорее всего она действительно покажет только 2 проверки из 10. В своё оправдание скажу, что в разношёрстной международной компании даже заставить разрабов установить гитовский pre-hook уже чудо :). Поэтому нужно было максимально простое решение которое бы ломало билд в случае ошибок, что бы можно было ткнуть носом потом.

Настройки в мавене тоже как у вас, разве что версии по новее.

Checker Framework не пробовал, надо будет поковырять на досуге. Тем более, что пишут что Checker Framework и ErrorProne можно использовать вместе.

Понимаю :-) Но вы разработчиков не заставляйте. Если вы лид, то просто закомитьте файлы инспекции в репу так, как сказано в статье. У них Idea начнет работать как вам надо: 1) инспекция начнет ругаться в Idea перед нажатием кнопки Commit, 2) начнет посвечивать NPE в редакторе, 3) автоматически подставлять спринговую Nullable аннотацию при автодополнении по Atl + Enter.

Пользительно, спасибо. Как раз искал что то что бы ругалось как котлин. С выходом java 21 lts у котлина не останется причин для существования :)

Наконец-то дошли руки интегрировать error prone, nullaway и checker framework. Каких-то явных ошибок в проекте не нашлось, в основном всяки слегка попахивающий код, типа неиспользуемых аннотаций @Slf4j.

Но столкнулись с проблемой при использовании Stream API: у нас в нескольких местах есть код вида "list.stream().map(this::canReturnNull).filter(Objects::nonNull).map(this::someOtherMethod)". NullAway в таком случае ничего не говорит (то ли достаточно умный что бы распознать Objects::nonNull, то ли не достаточно умный что бы в принципе со стримами разбираться). Но вот checker фреймворк в таком случае ругается, что someOtherMethod ожидает NonNull, а объекты в стриме - Nullable, хотя все нулл объекты отфильтрованы на предыдущем шаге.

На гитхабе тикеты с описанием этой проблемы существуют минимум с 2017 года, но проблема до сих пор не решена. Может быть знаете как можно обойти эту проблему? Добавлять SuppressWarning'и как-то не очень хочется, потому что в таком случае можно случайно реальную ошибку скрыть.

Попробуйте в сторону jdk.astub посмотреть, описав в ней сигнатуру Objects.nonNull() с возвращаемым типом NonNull

Sign up to leave a comment.

Articles