Resolve 🎯T1 and 🎯T3: lint cleanup and Node 24 actions - #96
Merged
Conversation
Addresses 🎯T3. Updates the three Node 20-deprecated pins in go.yml to their current latest major versions: - actions/checkout: v3 → v6 - actions/setup-go: v3 → v6 - golangci/golangci-lint-action: v3 → v9 Pre-existing Node 20 deprecation annotations (flagged on every CI run since PR #91) should disappear. GitHub Actions is forcing Node 24 by default on 2026-06-02 and removing Node 20 from runners on 2026-09-16, so these bumps land well inside the grace window. Known risk: golangci-lint-action@v9 may not support downloading the v1.48 golangci-lint binary we pinned in PR #91. If CI fails on that, the fix is part of 🎯T1 (remove the v1.48 pin entirely after migrating the .golangci.yml config and fixing the code-level findings that newer golangci-lint versions flag). That work is coming next. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removes the v1.48 pin from PR #91 and addresses all findings that newer golangci-lint versions surface against this codebase. Config migration (.golangci.yml): - Migrated from v1 to v2 format via `golangci-lint migrate`. - Removed depguard (never configured; was triggering blanket import- denial errors under depguard's rules-based format). - Removed errcheck.check-blank=true, which was flagging the idiomatic `_, _ = state.Write(...)` in parser/scanner.go's Format method — standard Go style for fmt.Formatter implementations. - Removed dead gocritic settings (linter isn't enabled). - Deprecated exportloopref, govet.check-shadowing, and the github-actions output format are dropped by the migration. Code fixes: - cmd/test.go: testWbnfFile no longer returns an always-nil error (unparam). The single caller is updated. - wbnf/compile.go: four //nolint:gosec comments silence G115 integer overflow warnings on the byte-escape parsing in parseString. The octal and \x cases are genuinely safe (ParseInt bitSize=8 constrains the value); the \u and \U cases have a latent truncation bug that nobody has ever hit because no grammar in the repo uses these escape forms. The latent bug is tracked separately — the nolint comments point at 🎯T4 for follow-up. - parser/parser.go, parser/terms.go, wbnf/compile.go, wbnf/cutpoints.go: four prealloc fixes converting `[]T{}` / `var x []T` patterns to `make([]T, 0, len(...))` where the target length is statically known from the loop bound. Workflow (.github/workflows/go.yml): - Removed the `version: v1.48` pin on golangci-lint-action. The action's default now installs the current golangci-lint, which runs cleanly against the migrated config and the code fixes. Combined with the action version bumps in a97465c (🎯T3), the Go workflow now runs on Node 24-compatible actions with no version pins and no outstanding lint findings. Local verification: `go build ./...`, `go test ./...`, and `golangci-lint run` all clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🎯T1 (CI lints cleanly under current golangci-lint without a version pin): achieved by commits a97465c and a2cd03e. Config migrated from v1 to v2, underlying lint findings fixed, v1.48 pin removed. Local build, tests, and golangci-lint run are all clean. 🎯T3 (CI workflow actions run on Node 24-compatible versions): achieved by commit a97465c. actions/checkout@v3 → v6, actions/setup-go@v3 → v6, golangci/golangci-lint-action@v3 → v9. No more Node 20 deprecation annotations expected. 🎯T4 (parseString correctly handles \x, \u, \U, and octal escapes): new target capturing a latent bug in wbnf/compile.go's string-literal escape parsing, discovered while adding //nolint:gosec comments as part of 🎯T1. The \x/\u/\U cases have off-by-one slice offsets that would panic on any real use, and \u/\U additionally truncate Unicode code points to a single byte via WriteByte. Untested code paths — no grammar in the repo uses these escapes — so the bugs have never been observed in practice. Fix is small but out of scope for a lint cleanup pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
Two small follow-ups from PR #96: - go.yml: reorder steps so checkout runs before setup-go in all three jobs. actions/setup-go@v6 tries to restore the Go module cache by reading go.mod, but the previous order ran it before checkout, so go.mod wasn't on disk yet. The cache-restore emitted a non-fatal annotation on every run ("Dependencies file is not found"). With the correct order, the cache now restores successfully. - .gitignore: add .claude/ for Claude Code's per-project agent state. The directory has been sitting untracked in every working tree this session; adding it to .gitignore stops it showing up in git status. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marcelocantos
added a commit
that referenced
this pull request
Apr 11, 2026
Two small follow-ups from PR #96: - go.yml: reorder steps so checkout runs before setup-go in all three jobs. actions/setup-go@v6 tries to restore the Go module cache by reading go.mod, but the previous order ran it before checkout, so go.mod wasn't on disk yet. The cache-restore emitted a non-fatal annotation on every run ("Dependencies file is not found"). With the correct order, the cache now restores successfully. - .gitignore: add .claude/ for Claude Code's per-project agent state. The directory has been sitting untracked in every working tree this session; adding it to .gitignore stops it showing up in git status. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Two targets resolved, one new target introduced.
Commits
a97465cBump CI actions to Node 24-compatible major versions (🎯T3) —actions/checkout@v3 → v6, actions/setup-go@v3 → v6,
golangci/golangci-lint-action@v3 → v9. Closes the Node 20 deprecation
warnings flagged on every CI run since PR Refine research doc and pin golangci-lint to restore CI #91.
a2cd03eResolve 🎯T1: lint cleanly under current golangci-lint —migrates
.golangci.ymlfrom v1 to v2 format viagolangci-lint migrate,drops
depguard(never configured; broke under the v2 rules format) anderrcheck.check-blank=true(fought idiomatic Format methods). Fixes thecode-level findings newer golangci-lint versions surface: four nolint
comments on gosec G115 warnings in
parseString's byte-escape handling(the octal and
\xcases are genuinely safe;\u/\Uhave a latenttruncation bug tracked separately as 🎯T4), the
testWbnfFileunparamfinding, and four prealloc fixes converting
[]T{}/var x []Ttomake([]T, 0, len(...)). Removes theversion: v1.48pin from PR Refine research doc and pin golangci-lint to restore CI #91 —the action now installs the current golangci-lint.
63abfa0Update targets: retire 🎯T1 and 🎯T3, introduce 🎯T4 —bookkeeping. 🎯T4 captures a latent bug in
parseString's\x/\u/\Uescape handling (off-by-one slice offsets + Unicode truncation).
Untested code paths, never observed in practice, surfaced while silencing
the gosec warnings. Small but out of scope for a lint cleanup pass.
Verification
Local
go build ./...,go test ./..., andgolangci-lint run(v2.11.4)all clean before push. CI on this PR is the final check.