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

Add JSpecify checking for return statements #734

Merged
merged 225 commits into from
Feb 22, 2023

Conversation

NikitaAware
Copy link
Contributor

This pull request includes the following:

  • Check whether the return type of the method matches the type of the actual return statement for the generics for example:
    static A<@Nullable String> test() { return new A<String>(); }
    For the method return type, the type parameter is @nullable String and the type of the type parameter of the actual return statement is NonNull. NullAway will report an error here.

@msridhar msridhar changed the title Generics return type changes Add JSpecify checking for return statements Feb 15, 2023
@msridhar msridhar added the jspecify Related to support for jspecify standard (see jspecify.dev) label Feb 16, 2023
@msridhar
Copy link
Collaborator

@lazaroclapp I already did a few passes on this one. It'd be great if you could take a look, when you have time

Copy link
Collaborator

@lazaroclapp lazaroclapp left a comment

Choose a reason for hiding this comment

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

Some minor nits/suggestions below, but overall this looks good!

Copy link
Collaborator

@lazaroclapp lazaroclapp left a comment

Choose a reason for hiding this comment

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

One incredibly minor nit left from me. Good to land once that's addressed 🙂

@@ -151,8 +151,10 @@ private static void reportInvalidReturnTypeError(
}

/**
* This method returns type of the tree considering that the parameterized typed tree annotations
* are not preserved if obtained directly using ASTHelpers.
* This method returns the type of the tree, including any type use annotations. This is required
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would still introduce a paragraph break after the first line here. Since the first line is a description of what the method does and the rest is additional explanation. It might need a <p> at the beginning of the new paragraph in order for GJF to not reformat it...

Copy link
Collaborator

@msridhar msridhar Feb 22, 2023

Choose a reason for hiding this comment

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

Fixed in f000611

@msridhar msridhar enabled auto-merge (squash) February 22, 2023 18:47
@msridhar msridhar merged commit 1548c69 into uber:master Feb 22, 2023
msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 18, 2023
msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 18, 2023
msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 19, 2023
msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 19, 2023
msridhar added a commit to msridhar/NullAway that referenced this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jspecify Related to support for jspecify standard (see jspecify.dev)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants