Skip to content

Fix staticcheck lint warnings#33

Merged
JamieMagee merged 1 commit intomainfrom
jamiemagee/fix-staticcheck-issues
Feb 10, 2026
Merged

Fix staticcheck lint warnings#33
JamieMagee merged 1 commit intomainfrom
jamiemagee/fix-staticcheck-issues

Conversation

@JamieMagee
Copy link
Copy Markdown
Member

Fixes 18 of the 24 staticcheck warnings reported by golangci-lint run --enable-only staticcheck.

What changed:

  • S1034config.GetListOfStrings now uses a tagged type switch (switch val := value.(type)) instead of switching on the type and then repeating the assertion inside each case.
  • QF1001 — Replaced !(a || b) with !a && !b (De Morgan's law) in the dialer and four handler guard clauses. Same logic, easier to read.
  • QF1003 — Swapped an if-else chain for a switch on metricType in the metrics collector client.
  • SA4006 — Discarded unused return values (_, _) in docker registry and cache tests where the returned req/resp was immediately overwritten.
  • SA5011 — Added an explicit return after t.Fatalf in the OIDC credential test so staticcheck can prove actual is non-nil past that point.

Not included:

The remaining 6 warnings are SA1019 (deprecated aws-sdk-go v1 imports in the docker registry handler). Migrating to aws-sdk-go-v2 changes the API surface and deserves its own PR.

- Use tagged type switch in config.GetListOfStrings to avoid redundant type assertions (S1034)
- Apply De Morgan's law to negated OR conditions in dialer and handler guard clauses (QF1001)
- Replace if-else chain with switch on metricType in collector client (QF1003)
- Discard unused return values in docker registry and cache tests (SA4006)
- Add explicit return after t.Fatalf in OIDC credential test so staticcheck can see the nil guard (SA5011)
Copilot AI review requested due to automatic review settings February 10, 2026 05:31
@JamieMagee JamieMagee requested a review from truggeri February 10, 2026 05:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces staticcheck noise across the proxy codebase by applying small refactors and test cleanups that preserve existing behavior while eliminating flagged patterns.

Changes:

  • Refactors type checks / conditionals (tagged type switch, De Morgan rewrite, switch on metric type) to satisfy staticcheck quick-fix recommendations.
  • Cleans up tests by discarding unused return values and adding an explicit return after t.Fatalf so staticcheck can prove control flow.
  • Minor readability improvements in request guard clauses and dialer network-type validation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/oidc/oidc_credential_test.go Adds an explicit return after t.Fatalf to satisfy SA5011 flow analysis.
internal/metrics/collector_client.go Replaces an if/else chain with a switch when populating metric payload fields (QF1003).
internal/handlers/python_index.go Applies De Morgan rewrite in handler credential matching guard (QF1001).
internal/handlers/nuget_feed.go Applies De Morgan rewrite in handler credential matching guard (QF1001).
internal/handlers/maven_repository.go Applies De Morgan rewrite in handler credential matching guard (QF1001).
internal/handlers/goproxy_server_handler.go Applies De Morgan rewrite in handler credential matching guard (QF1001).
internal/handlers/docker_registry_test.go Discards unused returned req values in tests where they are overwritten (SA4006).
internal/dialer/dialer.go Rewrites negated disjunction to conjunction for readability (QF1001).
internal/config/config.go Uses a tagged type switch to avoid repeated type assertions (S1034).
internal/cache/handlers_test.go Avoids assigning unused return value from OnResponse (SA4006).
Comments suppressed due to low confidence (1)

internal/config/config.go:71

  • In GetListOfStrings, the local variable named strings shadows the imported strings package in this file, which makes the code harder to read and can lead to confusion if additional strings.* calls are added later. Consider renaming the slice to something like result/out/list.
		strings := make([]string, len(val))
		for i, v := range val {
			if str, ok := v.(string); ok {
				strings[i] = str
			}
		}
		return strings

@JamieMagee JamieMagee enabled auto-merge (squash) February 10, 2026 05:34
@JamieMagee JamieMagee merged commit 4abab3a into main Feb 10, 2026
97 checks passed
@JamieMagee JamieMagee deleted the jamiemagee/fix-staticcheck-issues branch February 10, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants