[sergo] Sergo Report: Enforce-Landed + ToLower Var-Alias FN & nolint Multi-Directive Audit - 2026-06-07 #37494
Closed
Replies: 1 comment
-
|
This discussion has been marked as outdated by Sergo - Serena Go Expert. A newer discussion is available at Discussion #37742. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
Summary
Run R29 (2026-06-07). The previous run's enforce-enable recommendation landed within ~1 day, and the nolint-parity work partially landed. Today's deep dive found two new, evidence-backed precision/robustness gaps in the linter framework itself and filed both.
-tolowerequalfoldappended) ✅Reverification results (cached 50%)
cgo.yml:1078now carries 12 flags; the 13 violation sites were converted tostrings.EqualFoldseenmapboolandtolowerequalfoldboth gainedinternal/nolint; missing count 9 → 7. Invariant holds: all 12 enforced linters have nolint. (Still open)inspectBody(lines 64-175) remains escape-blind, but the linter now has nolint, so enforce-via-annotation is newly viable. Open ~06-09; not re-filedThe precision-fix-then-enforce pattern continues to land fast: sg24a1 (R25), sg27a1 (R28) and sg28a1 (R29) all merged within roughly a day of filing.
New findings (exploration 50%)
sg29a1 — tolowerequalfold false negative on var-aliased ToLower/ToUpper
The now-enforced linter only matches a direct
strings.ToLower(...)/ToUpper(...)call as a comparison operand (caseConvArg, lines 78-101). When the result is stored in a variable first, the comparison is invisible — the exact idiom the linter targets slips past enforced CI.Smoking gun:
pkg/parser/yaml_import.goconverted the direct-call compare at line 39 toEqualFold, but leftlower == "copilot-setup-steps.yml"at line 67 untouched (same function) because the linter couldn't see it.pkg/workflow/features.gousesEqualFoldat lines 69/105 yet keepsflagLower == "inline-agents"at line 27.All 4 files / 5 sites
lower == "copilot-setup-steps.yml" || ...yamllower := strings.ToLower(base)flagLower == "inline-agents"flagLower := strings.ToLower(TrimSpace(...))lower == "macos"lower := strings.ToLower(label)lowerField == "engine"lowerField := strings.ToLower(field)Fix: track local
v := strings.ToLower/ToUpper(x)single-assignments; flagv (==|!=) <stringLit>. Literal-operand scoping keeps false-positive risk near zero (self-compare guard unaffected).sg29a2 — internal/nolint suppresses only the first linter in a comma list
The shared
BuildLineIndex(internal/nolint/nolint.go:34-57) usesstrings.HasPrefix(text, "nolint:"+name), so//nolint:gosec,tolowerequalfolddoes not suppresstolowerequalfold, and//nolint:<linter>Xover-matches. This affects all 14 nolint-capable linters. Latent today (every custom directive in-tree is single-linter), but a correctness foot-gun for the first combined directive.Detail & fix
Fix: split the
nolint:payload on,, trim tokens, exact-match against the linter name (orall). ~10 lines; benefits every nolint linter at once. The one multi-linter directive present (glob_validation.go:85//nolint:cyclop,gocognit) targets golangci-lint built-ins, not this framework.Generated tasks
tolowerequalfold+ convert the 5 sites + testdata. (filed)internal/nolint.BuildLineIndex+ unit tests. (filed)Metrics
Run metrics & history
Strategy split: 50% cached (reverify enforce/precision/nolint state of sg26a1/sg28a1/sg28a2) + 50% new exploration (linter-framework precision audit of the freshly-enforced
tolowerequalfoldand the sharedinternal/nolinthelper).Recommendations & next-run focus
//nolint:seenmapboolannotation of the escaping sites without requiring the escape filter — a newly-unblocked path worth flagging if the escape filter stalls again.References: §27083452959
Beta Was this translation helpful? Give feedback.
All reactions