Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New check: ReturnMissingNullable #622

Open
jiri-pejchal opened this issue Nov 24, 2017 · 3 comments
Open

New check: ReturnMissingNullable #622

jiri-pejchal opened this issue Nov 24, 2017 · 3 comments
Labels

Comments

@jiri-pejchal
Copy link

jiri-pejchal commented Nov 24, 2017

Methods returning null should be annotated @Nullable.

Invalid code:

Date created;
    // failure: missing @Nullable
   public Date getCreated() {
        if (created == null) {
            return null;
        } else {
            return (Date) created.clone();
        }
    }

Valid code:

Date created;
   @Nullable
   public Date getCreated() {
        if (created == null) {
            return null;
        } else {
            return (Date) created.clone();
        }
    }
@rnveach
Copy link
Contributor

rnveach commented Nov 24, 2017

@George-007 Are you expecting users to place @NotNull if the method can never return null? If so, please add this as a case to your examples.
Or are you expecting the check to scan the method to look for return null;? This can create false negatives if the return calls another method, especially if the method is in another class/file.

@romani
Copy link
Member

romani commented Nov 24, 2017

Are you expecting users to place @NotNull if the method can never return null?

Checkstyle can not reliably diagnose this, as we do not have type, class hierarchies, and all other meta information about all methods that are in use in method we will validate.

Or are you expecting the check to scan the method to look for return null;?

checkstyle ca not go out of parser file, Check could clearly demand annotation is "return null" is present, But Check will not find complicated cases ... so it should be clearly defined in Check documentation.

@jiri-pejchal
Copy link
Author

I'm aware that checkstyle can only parse one file at a time - so only the basic case of returning the null literal from methods if it's possible. The check would be limited. Nevertheless returning null is quite common so it would cover a lot of cases in code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants