Skip to content

lint: replace gocyclo with gocognit - #14057

Merged
glours merged 3 commits into
mainfrom
gocognit-eval
Aug 18, 2026
Merged

lint: replace gocyclo with gocognit#14057
glours merged 3 commits into
mainfrom
gocognit-eval

Conversation

@ndeloof

@ndeloof ndeloof commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

What I did

Replaced the gocyclo linter with gocognit (cognitive complexity, threshold 30 — the linter default). Cognitive complexity barely charges guard clauses and early returns, and penalizes nesting instead, which matches how this codebase is written: 8 of the 21 //nolint:gocyclo suppressions become unnecessary, while deeply nested functions gocyclo never flagged are now caught. Remaining functions above the threshold keep a suppression, each marked FIXME so they can be restructured over time; doBuildBake (the worst offender, cognitive complexity 88) is restructured in this PR as a demonstrator, and gocyclo is explicitly disabled so it doesn't sneak back in.

includes refactoring of doBuildBake (hot spot after migration to gocognit, never identified by gocyclo)

Supersedes #14055 (recreated from an in-repo branch to allow a stacked follow-up PR).

Related issue

n/a

🤖 Generated with Claude Code

ndeloof and others added 3 commits August 17, 2026 08:23
Cognitive complexity (gocognit, threshold 30 — the linter default) fits
this codebase better than raw cyclomatic complexity: it barely charges
guard clauses and early returns, and penalizes nesting instead. As a
result 8 of the 21 //nolint:gocyclo suppressions become unnecessary,
while deeply nested functions gocyclo never flagged are now caught.

The 19 functions still above the threshold keep a suppression, each
marked FIXME to complete the migration by restructuring them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
doBuildBake mixed five concerns in a 300-line body (cognitive
complexity 88): progress display setup, translation of the project
into a bake file definition, temp metadata file allocation, bake
command construction, stderr rawjson streaming, and result
collection. Each now lives in its own function; the driver reads as
the sequence of those stages (cognitive complexity 17), and the
FIXME suppression is gone.

The bake variable was named to avoid shadowing the docker/cli
'build' package. No behavior change: prepareBakeBuild emits the same
config, and the stderr loop keeps the decoder-per-line semantics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
gocognit replaced gocyclo on purpose: raw cyclomatic complexity
charges a flat guard clause the same as a deeply nested branch, so our
fail-fast style accumulated //nolint suppressions on functions that
are long but flat. Cognitive complexity penalizes nesting and barely
charges early returns, which keeps the linter's signal on genuinely
tangled code instead.

With 'default: none' the disable entry is functionally redundant, but
it records that decision exactly where someone would re-add the
linter, with a hard stop: golangci-lint rejects a config listing the
same linter in both enable and disable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>

@glours glours left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool thanks!

@glours
glours merged commit d48a499 into main Aug 18, 2026
177 of 179 checks passed
@glours
glours deleted the gocognit-eval branch August 18, 2026 08:42
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.

2 participants