Enable NullAway static null-safety analysis for util module#10046
Open
Apisapple wants to merge 30 commits intobesu-eth:mainfrom
Open
Enable NullAway static null-safety analysis for util module#10046Apisapple wants to merge 30 commits intobesu-eth:mainfrom
Apisapple wants to merge 30 commits intobesu-eth:mainfrom
Conversation
- Replace javax.annotation.Nullable and javax.annotation.CheckForNull with org.jspecify.annotations.Nullable across 18 Java files - Update platform constraint to org.jspecify:jspecify:1.0.0 - Replace compileOnly com.google.code.findbugs:jsr305 with compileOnly org.jspecify:jspecify in affected modules Signed-off-by: Mykim <38449976+Apisapple@users.noreply.github.com>
Migrate JSR305 nullness annotations to JSpecify
Signed-off-by: mykim <kimminyong2034@gmail.com>
- Add NullAway 0.12.4 to util errorprone dependencies - Annotate nullable fields/returns in 6 util classes with @nullable - Fix nullable dereferences: RollingFileWriter, StackTraceMatchFilter, PlatformDetector - Enable NullAway:ERROR for util main compile, OFF for tests - Add optional CI job (run-nullaway label) for gradual monitoring - All util compilation passes with NullAway ERROR by default Fixes: - MemoryBoundCache: mark getIfPresent() return as @nullable - ExceptionUtils: annotate rootCause() for nullable input/output - RollingFileWriter: guard Path.getParent() null dereference - PlatformDetector: make static fields @nullable, add fallback to UNKNOWN - BesuVersionUtils: mark VERSION/COMMIT fields as @nullable - StackTraceMatchFilter: fix nullable message comparison, builder fields Signed-off-by: mykim <kimminyong2034@gmail.com>
…besu into feature/nullaway-util
…ilter Signed-off-by: mykim <kimminyong2034@gmail.com>
… null Replace null return with UNKNOWN to satisfy NullAway @nonnull contract. Signed-off-by: mykim <kimminyong2034@gmail.com>
Feature/nullaway util
- Return null to account for existing code that expects and handles null values. Signed-off-by: mykim <kimminyong2034@gmail.com>
- Remove the unnecessary -PenableNullAway flag Signed-off-by: mykim <kimminyong2034@gmail.com>
- Update the Javadoc to reflect the actual behavior of the getGlibc function. Signed-off-by: mykim <kimminyong2034@gmail.com>
Signed-off-by: mykim <kimminyong2034@gmail.com>
Signed-off-by: mykim <kimminyong2034@gmail.com>
Signed-off-by: mykim <kimminyong2034@gmail.com>
Feature/fix nullaway util
Adjust indentation of GRADLEW_UNIT_TEST_ARGS in .github/workflows/pre-review.yml under unitTests.env to align with surrounding keys. Signed-off-by: mykim <kimminyong2034@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Enable NullAway (via Error Prone) for the util module and resolve the surfaced null-safety issues by adding JSpecify nullability contracts and small behavior tweaks.
Changes:
- Turn on NullAway at
ERRORseverity for:utilproduction compilation and disable it for:utiltest compilation (pilot). - Add NullAway (and transitive artifacts) to dependency verification metadata.
- Apply null-safety fixes and nullable annotations across several util classes (platform detection, logging filter, IO writer, cache, version/helpers).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| util/src/main/java/org/hyperledger/besu/util/platform/PlatformDetector.java | Annotate cached fields nullable; harden property/version normalization; adjust JNA signature nullability. |
| util/src/main/java/org/hyperledger/besu/util/log4j/plugin/StackTraceMatchFilter.java | Make config fields nullable; avoid NPE on throwable message; adjust matching/toString behavior. |
| util/src/main/java/org/hyperledger/besu/util/io/RollingFileWriter.java | Guard against Path#getParent() returning null before mkdirs. |
| util/src/main/java/org/hyperledger/besu/util/cache/MemoryBoundCache.java | Mark getIfPresent return value as nullable. |
| util/src/main/java/org/hyperledger/besu/util/ExceptionUtils.java | Accept nullable input and return nullable root cause. |
| util/src/main/java/org/hyperledger/besu/util/BesuVersionUtils.java | Mark version/commit as nullable and propagate via accessors. |
| util/build.gradle | Enable/disable NullAway per source set; add NullAway to errorprone configuration. |
| gradle/verification-metadata.xml | Add verification entries for NullAway and transitive dependencies. |
Comment on lines
+38
to
+41
| private static final @Nullable String VERSION; | ||
| private static final String OS = PlatformDetector.getOS(); | ||
| private static final String VM = PlatformDetector.getVM(); | ||
| private static final String COMMIT; | ||
| private static final @Nullable String COMMIT; |
Contributor
Author
There was a problem hiding this comment.
It looks like we may need to modify the test code to apply this change, so I need to confirm whether it's okay to proceed with those updates.
Comment on lines
+81
to
83
| public static @Nullable String shortVersion() { | ||
| return VERSION; | ||
| } |
Comment on lines
+123
to
125
| public static @Nullable String commit() { | ||
| return COMMIT; | ||
| } |
| public class StackTraceMatchFilter extends AbstractFilter { | ||
| private final String stackContains; | ||
| private final String messageEquals; | ||
| private final @Nullable String stackContains; |
| private StackTraceMatchFilter( | ||
| final String stackContains, | ||
| final String messageEquals, | ||
| final @Nullable String stackContains, |
Comment on lines
+78
to
82
| return (messageEquals == null || Objects.equals(t.getMessage(), messageEquals)) | ||
| && stackContains != null | ||
| && Arrays.stream(t.getStackTrace()) | ||
| .map(StackTraceElement::getClassName) | ||
| .anyMatch(cn -> cn.contains(stackContains)) |
Comment on lines
+54
to
+60
| final Path parentPath = firstOutputFile.getParent(); | ||
| if (parentPath != null) { | ||
| final File parentDir = parentPath.toFile(); | ||
| if (!parentDir.exists()) { | ||
| //noinspection ResultOfMethodCallIgnored | ||
| parentDir.mkdirs(); | ||
| } |
Comment on lines
+331
to
+334
| final Pattern pattern = Pattern.compile("[-+]?[\\d]*\\.?[\\d]+"); | ||
| final Matcher matcher = pattern.matcher(rawGlibcVersion); | ||
|
|
||
| return matcher.find() ? matcher.group() : null; | ||
| return matcher.find() ? matcher.group() : UNKNOWN; |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR description
This PR enables NullAway static null-safety analysis for the util module as a pilot and fixes all violations surfaced by the check.
Summary of changes
Developer setup
Validation
Fixed Issue(s)
fixes #10004
Thanks for sending a pull request! Have you done the following?
Locally, you can run these tests to catch failures early: