Fix Go E2E SA5011 nil-deref staticcheck findings#1982
Open
roji wants to merge 2 commits into
Open
Conversation
Restructure nil checks in Go E2E tests so dereferences stay in the proven non-nil branch. This addresses SA5011 findings from newer golangci-lint/staticcheck without suppressing lint rules or pinning older tools. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e4710a8-2700-4e51-ae9c-657aa8215080
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Go E2E SA5011 findings by placing pointer dereferences inside statically proven non-nil branches without changing assertions.
Changes:
- Guard assistant response and RPC result dereferences with
elsebranches. - Apply the same nil-flow pattern consistently across flagged E2E tests.
Show a summary per file
| File | Description |
|---|---|
go/internal/e2e/tools_e2e_test.go |
Guards assistant message access. |
go/internal/e2e/system_message_sections_e2e_test.go |
Guards system-message response access. |
go/internal/e2e/rpc_ui_ephemeral_query_e2e_test.go |
Guards ephemeral query result access. |
go/internal/e2e/rpc_server_plugins_e2e_test.go |
Guards plugin and marketplace pointers. |
go/internal/e2e/rpc_server_misc_e2e_test.go |
Guards users result iteration. |
go/internal/e2e/rpc_server_e2e_test.go |
Guards discovery path pointers. |
go/internal/e2e/event_fidelity_e2e_test.go |
Guards assistant event access. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Medium
Contributor
Cross-SDK Consistency Review ✅This PR only modifies Go E2E test files ( No cross-SDK consistency issues found. The changes:
The fix pattern (
|
Resolve PR conflict state by updating the branch to the latest main and taking the upstream resolved versions in the previously conflicting Go E2E files. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e4710a8-2700-4e51-ae9c-657aa8215080
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.
Summary
Fixes Node E2E test bug that trips the linter and fails our build.
t.Fatalfor nil-flow narrowing in patterns that newer staticcheck flags as possible nil dereferences (SA5011)go/internal/e2e/*_test.gositesValidation
/tmp/golangci-lint run ./internal/e2e/... --timeout=5mgo test ./internal/e2e/... -run '^$'go test ./internal/e2e/...fails in this environment because Copilot CLI binary is not installed; these tests require runningnpm installinnodejs/first