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

Added support for Apache Validate #769

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Added support for Apache Validate #769

merged 2 commits into from
Jun 21, 2023

Conversation

Asapin
Copy link
Contributor

@Asapin Asapin commented Jun 15, 2023

Added support for Apache Validate: https://github.com/apache/commons-lang/blob/master/src/main/java/org/apache/commons/lang3/Validate.java

We recently started to use these Apache methods to validate some data but quickly found out that NullAway don't support them since they are not properly annotated. Here's a fix that adds support for these methods along with test cases for each method.

I noticed that Apache library generally has different validation methods for String, Map and Collection, but they all end up with the same signature when analyzed by NullAway (probably because these methods use generics).
Examples of such methods:

public static <T extends Collection<?>> T notEmpty(final T collection) {
}

public static <T extends Map<?, ?>> T notEmpty(final T map) {
}

public static <T extends CharSequence> T notEmpty(final T chars) {
}

But I decided to add a separate unit test for each of these methods in case NullAway will change how it generates method's signature in the future

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2023

CLA assistant check
All committers have signed the CLA.

@msridhar
Copy link
Collaborator

@Asapin thanks for the contribution! Can you run ./gradlew googleJavaFormat locally to fix formatting and then push again?

@Asapin
Copy link
Contributor Author

Asapin commented Jun 16, 2023

I tried to run ./gradlew googleJavaFormat locally, but my terminal (PowerShell) created 700+ processes, consumed 22 Gbs of RAM and crashed.
./gradlew.bat googleJavaFormat has the same effect.

And if I run googleJavaFormat task directly from IDEA's Gradle window, it finishes in 2 seconds with the message

> Task :googleJavaFormat FAILED

Detected Java syntax errors in the following files (you can exclude them from this task, see "https://github.com/sherter/google-java-format-gradle-plugin" for details):

and then it prints a list of seemingly all source files that exist in the project.

Is this expected behavior (I doubt, but who knows)? Could it be because I'm using Windows?

Update: tried to run this task in WSL Ubuntu. At first it complained that gradlew contained CRLF. But after fixing it (should I create a separate PR for that?) it again showed the same Task :googleJavaFormat FAILED message

@msridhar
Copy link
Collaborator

For now I just ran formatting on Mac and it works. @Asapin if you can please file an issue on GJF not working on Windows. We should probably fix by just switching to Spotless. I will try to review the change soon.

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.

LGTM, and I appreciate the comprehensive test suite!

@lazaroclapp lazaroclapp merged commit 0466a02 into uber:master Jun 21, 2023
8 checks passed
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants