Skip to content

Style: Add checkstyle rule to prevent using Objects.requireNonNull#4617

Merged
rdblue merged 1 commit into
apache:masterfrom
tal66:master
Apr 24, 2022
Merged

Style: Add checkstyle rule to prevent using Objects.requireNonNull#4617
rdblue merged 1 commit into
apache:masterfrom
tal66:master

Conversation

@tal66
Copy link
Copy Markdown
Contributor

@tal66 tal66 commented Apr 22, 2022

Hi, this closes #4613.

running the checks on this simple example, :

    public static void main(String[] args)  {
        String str = null;

        // not allowed
        Objects.nonNull(str);
        Objects.nonNull(Integer.valueOf(123));
        Objects.requireNonNull(str);
        Objects.requireNonNullElse(str, "123");
        Objects.requireNonNullElseGet(str, () -> "1");

        // allowed
        Objects.isNull(str);
    }

generates: <error line="..." severity="error" message="Prefer using Preconditions.checkNotNull(Object, String)." source=... for the 'not allowed' lines above.

@github-actions github-actions Bot added the INFRA label Apr 22, 2022
Copy link
Copy Markdown
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this locally with Objects.requireNonNull(obj) and Objects.requireNonNull(obj, "error message) and both of them failed checkstyle.

LGTM provided tests are passing. Thanks @tal66!

Copy link
Copy Markdown
Contributor

@singhpk234 singhpk234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well, Thanks @tal66

@rdblue rdblue merged commit 0d2c1cf into apache:master Apr 24, 2022
@rdblue
Copy link
Copy Markdown
Contributor

rdblue commented Apr 24, 2022

Thanks, @tal66!

@nastra
Copy link
Copy Markdown
Contributor

nastra commented Jun 2, 2022

just curious, why wouldn't we want to allow something like if (Objects.nonNull(x))? The checkstyle message suggests to use Preconditions.checkNotNull instead, but that's not really the same. I can see why we'd want to use Preconditions.checkNotNull rather than Objects.requireNonNull but not why we'd prevent usage of Objects.nonNull, since it's just returning a boolean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Checkstyle Rule for Objects.notNull

5 participants