ci: add NullAway and CodeQL, replacing SonarCloud - #722
Open
soloturn wants to merge 5 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe build enables NullAway with JSpecify annotations. The launcher declares nullable values and adds checks for invalid paths, URLs, resources, and asynchronous state. A GitHub Actions workflow runs Java/Kotlin CodeQL scans on repository events and monthly. ChangesStatic analysis and nullability
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
❌ The last analysis has failed. |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/codeql.yml (1)
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse CodeQL Action v4.
Replace both v3 action references with v4. GitHub documents v4 for current CodeQL workflows. CodeQL Action v3 is scheduled for deprecation in December 2026. Verify GitHub Enterprise Server compatibility first, if applicable. (docs.github.com)
Proposed fix
- uses: github/codeql-action/init@v3 + uses: github/codeql-action/init@v4 ... - uses: github/codeql-action/analyze@v3 + uses: github/codeql-action/analyze@v4🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codeql.yml around lines 30 - 36, Update both CodeQL action references in the workflow’s init and Perform CodeQL Analysis steps from v3 to v4, after verifying compatibility with the supported GitHub Enterprise Server version if applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/codeql.yml:
- Line 22: Update the actions/checkout@v4 step in the workflow to disable
persisted credentials by setting persist-credentials to false, leaving the
checkout behavior otherwise unchanged.
---
Nitpick comments:
In @.github/workflows/codeql.yml:
- Around line 30-36: Update both CodeQL action references in the workflow’s init
and Perform CodeQL Analysis steps from v3 to v4, after verifying compatibility
with the supported GitHub Enterprise Server version if applicable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 865bbd3b-5fa7-4755-9148-49dd51f45c21
📒 Files selected for processing (3)
.github/workflows/codeql.ymlbuild.gradle.ktsgradle.lockfile
soloturn
force-pushed
the
ci/nullaway-codeql
branch
from
August 7, 2026 21:31
f3fe394 to
366e101
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/terasology/launcher/game/GameInstallation.java`:
- Around line 160-163: Update the path handling in matchJar to guard
path.getFileName() before calling toString(), returning false when either
path.getParent() or path.getFileName() is null; preserve the existing directory
and file matching behavior for valid paths.
In `@src/main/java/org/terasology/launcher/remote/DownloadUtils.java`:
- Around line 38-47: In the download validation flow, move the path.getParent()
lookup and null-parent DownloadException check before calling
DownloadUtils.getContentLength(downloadUrl). Keep the existing download URL
validation first, and only issue the remote content-length request after the
destination parent has been validated.
In `@src/main/java/org/terasology/launcher/repositories/Jenkins.java`:
- Around line 12-25: Replace the NullAway.Init suppression on the Gson DTOs in
Jenkins.java with JSpecify `@Nullable` annotations for fields that may be absent,
including builds, URLs, artifacts, change sets, and changelog messages. Update
the release creation and related consumers to handle these nullable fields
safely, or validate the deserialized ApiResult before creating a release; remove
the shared SUPPRESS_NULLAWAY_INIT constant and its usages.
In `@src/main/java/org/terasology/launcher/ui/ApplicationController.java`:
- Line 87: Update the download lifecycle handling in ApplicationController so
downloadTask is cleared for success, failure, and cancellation terminal states.
Add cleanup handlers for failed downloads and cancellation/clear requests, and
only clear the field when the callback’s task is still the current downloadTask,
preserving newer downloads from being overwritten.
In `@src/main/java/org/terasology/launcher/ui/LogViewController.java`:
- Around line 22-23: Update LogViewController initialization so the
ScheduledService starts only from initialize(), after FXMLLoader has injected
logArea, rather than during construction or field initialization. In
appendText(), dispatch all logArea updates through Platform.runLater(...) so
calls originating from Task.call() execute on the JavaFX Application Thread.
In `@src/main/java/org/terasology/launcher/updater/LauncherUpdater.java`:
- Line 31: Update updateAvailable() to avoid passing nullable currentVersion to
latestVersion.isGreaterThan; when currentVersion is null, use a non-null
sentinel Semver representing the baseline version (or return the appropriate
availability result before comparison), while preserving normal comparisons for
non-null versions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1cbd70b2-d72d-4eaa-8bb3-ecdd5da54147
📒 Files selected for processing (33)
src/main/java/org/terasology/launcher/LauncherInitTask.javasrc/main/java/org/terasology/launcher/Terasology.javasrc/main/java/org/terasology/launcher/TerasologyLauncher.javasrc/main/java/org/terasology/launcher/game/DirectPlay.javasrc/main/java/org/terasology/launcher/game/GameInstallation.javasrc/main/java/org/terasology/launcher/game/GameService.javasrc/main/java/org/terasology/launcher/game/GameVersionNotSupportedException.javasrc/main/java/org/terasology/launcher/game/RunGameTask.javasrc/main/java/org/terasology/launcher/io/Installation.javasrc/main/java/org/terasology/launcher/log/TempLogFilePropertyDefiner.javasrc/main/java/org/terasology/launcher/model/GameIdentifier.javasrc/main/java/org/terasology/launcher/model/GameRelease.javasrc/main/java/org/terasology/launcher/model/LauncherVersion.javasrc/main/java/org/terasology/launcher/remote/DownloadException.javasrc/main/java/org/terasology/launcher/remote/DownloadUtils.javasrc/main/java/org/terasology/launcher/remote/RemoteResource.javasrc/main/java/org/terasology/launcher/repositories/GithubRepository.javasrc/main/java/org/terasology/launcher/repositories/Jenkins.javasrc/main/java/org/terasology/launcher/repositories/JenkinsClient.javasrc/main/java/org/terasology/launcher/repositories/JenkinsRepository.javasrc/main/java/org/terasology/launcher/settings/LauncherSettings.javasrc/main/java/org/terasology/launcher/settings/Settings.javasrc/main/java/org/terasology/launcher/ui/AboutViewController.javasrc/main/java/org/terasology/launcher/ui/ApplicationController.javasrc/main/java/org/terasology/launcher/ui/ChangelogViewController.javasrc/main/java/org/terasology/launcher/ui/Dialogs.javasrc/main/java/org/terasology/launcher/ui/FooterController.javasrc/main/java/org/terasology/launcher/ui/LogViewController.javasrc/main/java/org/terasology/launcher/ui/SettingsController.javasrc/main/java/org/terasology/launcher/updater/LauncherUpdater.javasrc/main/java/org/terasology/launcher/util/HostServices.javasrc/main/java/org/terasology/launcher/util/I18N.javasrc/main/java/org/terasology/launcher/util/LauncherDirectoryUtils.java
soloturn
force-pushed
the
ci/nullaway-codeql
branch
from
August 14, 2026 20:50
65b737b to
bedaec9
Compare
soloturn
added a commit
that referenced
this pull request
Aug 16, 2026
- LauncherUpdater.updateAvailable(): guard the null currentVersion case NullAway itself now flags (unparseable local semver) instead of passing it into Semver.isGreaterThan - offer an update rather than silently never checking. - AboutViewController.finishWebView(): mark the WebView parameter @nullable - it's already null-checked internally, NullAway just hadn't seen the call sites before this rebase brought them in. - DownloadUtils.download(): validate the destination's parent directory before issuing the remote getContentLength() call, not after - fail on the local precondition first. - ApplicationController.downloadTask: clear it on every terminal state (failed, cancelled), not just success, guarded by identity so a stale callback from a superseded task can't clobber a newer one. Left two other CodeRabbit findings from the review as-is: matchJar()'s path.getFileName() is unreachable-null whenever parent != null (a Path with a non-null parent always has a non-null last element), so the suggested extra guard is dead code; and the Jenkins.java DTO's @SuppressWarnings("NullAway.Init") vs. per-field @nullable is the scoping tradeoff the PR description already covers - moving it now would be new scope, not a rebase-time fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123STaQeoJ7gjqeQR4ADoms
soloturn
force-pushed
the
ci/nullaway-codeql
branch
from
August 16, 2026 08:06
bedaec9 to
40b59b2
Compare
soloturn
added a commit
that referenced
this pull request
Aug 16, 2026
- matchJar(): add the path.getFileName() guard CodeRabbit suggested. Unreachable in practice (a Path with a non-null parent always has a non-null last element) but harmless, and it's what NullAway itself would want if it ever models java.nio.Path's nullability. - Jenkins.java: replace the blanket @SuppressWarnings("NullAway.Init") on each GSON DTO with per-field @nullable - GSON genuinely leaves fields null when a JSON key is missing, so this was hiding real nullability rather than working around an initialization false positive. Exposed two previously-unguarded dereferences that would NPE on a sparse Jenkins response: JenkinsClient.getArtifactUrl() matching artifact.fileName, and JenkinsRepository's changelog builder streaming ChangeSet.items - both now null-checked. Every other consumer already null-checked or only ever string-concatenates the now-nullable field, so no other call site needed to change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123STaQeoJ7gjqeQR4ADoms
soloturn
added a commit
that referenced
this pull request
Aug 21, 2026
soloturn
added a commit
that referenced
this pull request
Aug 23, 2026
CodeRabbit review on #722: logArea is injected by FXMLLoader after construction, and the log-poll ScheduledService started in the constructor - before that injection point the class's own comment already documents as unsafe to touch logArea from. The appendText() dispatch itself was already correctly wrapped in Platform.runLater(), so that half of the finding no longer applies; moving schedule.start() into initialize() removes the remaining doubt around field-injection timing at effectively no cost.
soloturn
force-pushed
the
ci/nullaway-codeql
branch
from
August 23, 2026 19:26
1332aa4 to
006eec6
Compare
soloturn
added a commit
that referenced
this pull request
Aug 23, 2026
- LauncherUpdater.updateAvailable(): guard the null currentVersion case NullAway itself now flags (unparseable local semver) instead of passing it into Semver.isGreaterThan - offer an update rather than silently never checking. - AboutViewController.finishWebView(): mark the WebView parameter @nullable - it's already null-checked internally, NullAway just hadn't seen the call sites before this rebase brought them in. - DownloadUtils.download(): validate the destination's parent directory before issuing the remote getContentLength() call, not after - fail on the local precondition first. - ApplicationController.downloadTask: clear it on every terminal state (failed, cancelled), not just success, guarded by identity so a stale callback from a superseded task can't clobber a newer one. Left two other CodeRabbit findings from the review as-is: matchJar()'s path.getFileName() is unreachable-null whenever parent != null (a Path with a non-null parent always has a non-null last element), so the suggested extra guard is dead code; and the Jenkins.java DTO's @SuppressWarnings("NullAway.Init") vs. per-field @nullable is the scoping tradeoff the PR description already covers - moving it now would be new scope, not a rebase-time fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123STaQeoJ7gjqeQR4ADoms
soloturn
added a commit
that referenced
this pull request
Aug 23, 2026
- matchJar(): add the path.getFileName() guard CodeRabbit suggested. Unreachable in practice (a Path with a non-null parent always has a non-null last element) but harmless, and it's what NullAway itself would want if it ever models java.nio.Path's nullability. - Jenkins.java: replace the blanket @SuppressWarnings("NullAway.Init") on each GSON DTO with per-field @nullable - GSON genuinely leaves fields null when a JSON key is missing, so this was hiding real nullability rather than working around an initialization false positive. Exposed two previously-unguarded dereferences that would NPE on a sparse Jenkins response: JenkinsClient.getArtifactUrl() matching artifact.fileName, and JenkinsRepository's changelog builder streaming ChangeSet.items - both now null-checked. Every other consumer already null-checked or only ever string-concatenates the now-nullable field, so no other call site needed to change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0123STaQeoJ7gjqeQR4ADoms
soloturn
force-pushed
the
ci/nullaway-codeql
branch
from
August 23, 2026 20:29
49f8c0d to
fceb40b
Compare
NullAway (error-prone plugin): fast null-checks, WARN severity, disabled on tests. CodeQL: weekly semantic scan, free for public repos. SonarCloud: not configured in-repo (GitHub App side), nothing to remove here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
96 warnings, 3 kinds: framework-managed fields (@SuppressWarnings NullAway.Init), genuinely-nullable values (@nullable), and real gaps fixed outright (silent-null returns now throw, cancelled-dialog no longer falls through). Checkstyle: final classes, private ctor, no empty blocks. Also reverts an I18N.getFxImage() regression from the NullAway pass - a caller had its own deliberate null-soft-fail for a missing flag icon. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ctor started polling before FXML injects logArea. appendText already safe via runLater; just needed to move where start() is called. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
LauncherUpdater: guard null currentVersion before Semver.isGreaterThan. AboutViewController: WebView param is @nullable, already null-checked. DownloadUtils: validate parent dir before remote getContentLength(). ApplicationController: clear downloadTask on every terminal state, not just success. matchJar(): add getFileName() guard (unreachable, but harmless). Jenkins.java DTOs: per-field @nullable instead of blanket @SuppressWarnings - GSON leaves fields null on missing keys; exposed two real unguarded derefs (JenkinsClient.getArtifactUrl, changelog builder), now null-checked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment-only, no logic change. Same facts (versions, issue links, CVEs), less prose. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
soloturn
force-pushed
the
ci/nullaway-codeql
branch
from
August 26, 2026 05:32
fceb40b to
cb3b69b
Compare
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
2 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.
Summary
Adds NullAway (error-prone plugin, WARN severity) and CodeQL, replacing SonarCloud - see #720's discussion, SonarCloud has been failing CI with no real findings (infra/token issue, not code).
SonarCloud itself isn't configured in this repo (server-side GitHub App) - nothing here to remove; an org admin needs to uninstall the app or deactivate the project on sonarcloud.io.
Rebased onto master; this branch had accumulated commits since fork-off that later landed separately as #720, #725, #727 - dropped, only the NullAway/CodeQL work remains (5 commits).
Test plan
./gradlew compileJava- clean, 0 NullAway warnings./gradlew check- passes