feature/spotbugs #150
Merged
feature/spotbugs #150
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| */ | ||
| @Nullable | ||
| public Class<?> getDerivedClass(@Nonnull final JsonObject jsonObject, @Nonnull final Class<?> parentClass) { | ||
| public Class<?> getDerivedClass(@Nonnull final JsonObject jsonObject, @Nullable final Class<?> parentClass) { |
There was a problem hiding this comment.
Probably off-topic but probably also doesn't hurt: if a parameter should be null ever (i.e. marked with Nonnull), then it might be a good practice to check that explicitly in the method (e.g. Objects.requireNonNull())
There was a problem hiding this comment.
yep, I'm planning to do a bulk pass for that as part of this static analysis effort. We didn't use to have any annotations not long ago...
There was a problem hiding this comment.
see the latest commits, I added a lot of defensive programming for nonnull annotations. Also corrected a few annotations that should've been Nullable instead
This comment has been minimized.
This comment has been minimized.
baywet
force-pushed
the
feature/spotbugs
branch
from
111d345 to
ab2ff30
Compare
zengin
approved these changes
Feb 18, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #148
related #149
This one is set to fail the build on infringements as we only have 60+ errors at this time.
It runs a light level of static analysis, (conditions structures, null checks, redundancy, etc...).
I'm providing examples of fixes to the team to get some early feedback.
I think the typo it caught in the defensive condition clearly demonstrates the value. It an honest mistake that's easy to make and easy to miss on pull requests that can have bad consequences and be hard to debug/find.