internal/nolint: space-prefixed "// nolint:" directive still silently ignored (issue 59122 auto-expired, bug unfixed)
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 773
Description
Summary
Issue 59122 documented that internal/nolint fails to recognize the single-space-prefixed directive spelling `// nolint:LINTERNAME` (as opposed to the no-space `(nolint/redacted):LINTERNAME`), even though the packages own doc comment claims same-line matching golangci-lint behaviour. That issue was closed as not_planned (auto-expired) rather than fixed. Re-reading the current code confirms the bug is still 100 percent present today - this is a reverse-phantom-reconcile (closed but unfixed), the same pattern previously confirmed for issue 58376/60557.
Root cause (unchanged since 59122)
pkg/linters/internal/nolint/nolint.go lines 59-60:
```go
text := strings.TrimPrefix(comment.Text, "//")
if !strings.HasPrefix(text, "nolint:") {
continue
}
```
For a comment written as `// nolint:largefunc`, comment.Text is the literal string `// nolint:largefunc`. TrimPrefix only strips the two leading slashes, leaving a string with a leading space: ` nolint:largefunc`. HasPrefix(that, "nolint:") is false, so the directive is silently dropped from the index and HasDirectiveForLinter reports the position as unsuppressed even though a human reading the file would clearly see a nolint comment.
Impact
Every one of the 71 registered analyzers depends on this single shared index via analyzerutil (nolint.Analyzer as a Requires dependency). Any contributor who writes the common, more readable `// nolint:linter` spelling (single space after the slashes) gets a silently ignored suppression - the linter fires anyway with no error message explaining why the directive did not take effect. A repo-wide search for the spaced form currently returns zero hits, so the bug has not yet caused a visible false-positive-suppression failure, but that is only because nobody has used the more natural spelling, not because the parser handles it.
Recommendation
Trim optional whitespace after the slashes before the HasPrefix check, for example: `text := strings.TrimPrefix(strings.TrimPrefix(comment.Text, "//"), " ")` (or a small loop/TrimLeft over spaces and tabs), then add a nolint_test.go case exercising `// nolint:linter` (single space) alongside the existing no-space case.
Validation checklist
- Add TestBuildDirectiveIndex case for `// nolint:largefunc` (space after slashes)
- Confirm HasDirectiveForLinter returns true for that spelling
- Confirm the no-space spelling and the multi-directive/comma-separated spelling still pass unchanged
Effort: small, single-file fix plus one test case.
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🤖 Sergo - Serena Go Expert](https://github.com/github/gh-aw/actions/runs/34926336224) · claude · agent · 189.6 AIC · ⌖ 9.58 AIC · ⊞ 6.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Fsergo%22&type=issues)
> - [x] expires on Sep 21, 2026, 7:57 PM UTC-08:00
Contributor guide
Research direction
Start in pkg/linters/internal/nolint/nolint.go around lines 59-60, then read nolint_test.go and its TestBuildDirectiveIndex cases. Add coverage for the spaced // nolint:linter spelling and run the relevant tests. Done means HasDirectiveForLinter recognizes it while the no-space and multi-directive/comma-separated forms still pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 90/100