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

chore: PMD rule for @Nonnull / @Nullable on fields of public types #369

Closed
wants to merge 6 commits into from

Conversation

newtork
Copy link
Contributor

@newtork newtork commented Mar 22, 2024

Follow-up #368

Context: mapping `int priority` to `severity`

image

@newtork newtork added please review Request to review a pull request please merge Request to merge a pull request labels Mar 22, 2024
Copy link
Member

@MatKuhr MatKuhr left a comment

Choose a reason for hiding this comment

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

I'm a bit unsure about this one. Those are a lot of warnings, and adding @Nonnull everywhere is kinda annoying / code bloat.

I guess the unwritten rule should be: If it's nullable always annotate it as such. Otherwise it will be assumed to be nonnull.
Unfortunately we can't make a PMD rule for that I guess. Would have to be some static code style analysis that goes through the AST and tries to figure out if null can potentially be assigned to a field not annotated as such..

<property name="xpath">
<value>
<![CDATA[
//ClassOrInterfaceDeclaration[@Public=true()]/ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration[
Copy link
Member

Choose a reason for hiding this comment

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

I'd argue the issue is also relevant for non-public classes..

Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

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

image
There is an IntelliJ warning Suggest @Nullable annotation that we could enable and that catches the NPE in #368

@CharlesDuboisSAP
Copy link
Contributor

lombok says adding @Nonnull everywhere is is massive performance penalty and waste of resources.
They also recommend using an IDE level reminder for this use case.

@CharlesDuboisSAP CharlesDuboisSAP deleted the pmd/nonnull-nullable-declarations branch June 13, 2024 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please merge Request to merge a pull request please review Request to review a pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants