Skip to content

refactor(lint): adopt new rubocop-go DSL across all cops#2687

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/845dcea8c9d00ff1
May 7, 2026
Merged

refactor(lint): adopt new rubocop-go DSL across all cops#2687
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/845dcea8c9d00ff1

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 7, 2026

Adopts the new rubocop-go DSL across all 12 custom cops in lint/. Behaviour is unchanged — same offenses, same anchors, same severities — but the cops read better and a lot of plumbing moved into the library.

What changed

  • cop.Func literal style everywhere. Each cop is now a single &cop.Func{Meta, Scope, Run} value. The type CopX struct { cop.Meta } + NewCopX() constructor + Check method triplet is gone.

  • Declarative scope. File-scope guards (if !p.FileMatches(...) { return }) are gone from Check bodies; cops declare their scope as data and the runner skips out-of-scope files entirely:

    Scope: cop.UnderDir("pkg/tui"),
    Scope: cop.OnlyFile("pkg/runtime/event.go"),
    Scope: cop.And(cop.InPathSegment("pkg/config", isLatest), cop.NotBlackBoxTest()),
  • Library helpers absorbed boilerplate. Cops now use:

    • Pass.ReportMissing(anchor, fmt, names) — replaces the sort + join + empty-check pattern in 5 dispatch-table cops
    • Pass.PointerReceiverMethods(name), Pass.StructType(name), Pass.FirstMethodCall(name), Pass.FuncDecl(name) — direct lookups instead of hand-rolled ForEach... filter loops
    • cop.WalkFuncWithContextScope — the ~120 LOC of context-scope analysis from slog_contextual.go lifted into the library
    • cop.StringField, cop.ParseTagOptions, cop.FieldNames, cop.FieldTag, cop.IsNullaryFunc, cop.CallTo — small helpers for shapes that recurred across cops
  • Dropped the regex in configpath.go in favour of a one-line LastIndex + delegation to versionFromDir, removing a drift point between the two helpers.

  • main.go's registry is now a flat var cops = []cop.Cop{...} of literal vars instead of a []cop.Cop{NewX(), ...} builder.

Numbers

Before After
lint/ LOC 1390 1151 (−17%)
Project-private helper functions ~20 ~12 (8 absorbed into the library)

Validation

  • task build, task test, task lint — all green
  • Negative-tested TUIViewPurity, SlogContextual (parameter-declared / nested funclit / discarded multi-return), and RuntimeSessionScoped — same offenses, same anchors, same wording as before

Note for reviewers

The cops report 4 pre-existing offenses on current main (in pkg/attachment/modelcaps/modelcaps.go, pkg/model/provider/anthropic/federation/federation.go, pkg/model/provider/openai/client.go). They exist on main with the original cops too — verified by checking out the pre-refactor lint/ and re-running. They are real Lint/SlogContextual violations that landed during the time this branch was stale and are not introduced by this refactor; happy to fix in a separate PR.

@dgageot dgageot requested a review from a team as a code owner May 7, 2026 09:19
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

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

Review: Lint Cop Refactor — rubocop-go DSL Adoption

Assessment: 🟢 APPROVE

This is a clean, well-structured refactor. All 12 cops were ported to the cop.Func literal style with correct Scope declarations, and the behavioral equivalence holds throughout. No regressions found.

Coverage summary:

Area Result
versionFromImport regex → LastIndex replacement (configpath.go) ✅ Equivalent — versionFromDir rejects non-vN suffixes identically
ReportMissing called without empty-slice guard ✅ Safe — library guards on len(names) == 0 internally
returnedEventType unary-op check removed ✅ Safe — *ast.UnaryExpr on a CompositeLit can only be & in valid Go
WalkFuncWithContextScope replacing hand-rolled context analysis ✅ Library handles body == nil guard; behavior matches
cop.ParseTagOptions + HasName() replacing manual strings.Cut + "-" check HasName() correctly rejects "" and "-"
Declarative Scope fields vs. in-body guards ✅ All scopes (paths, predicates, NotBlackBoxTest) faithfully match old guards
p.PointerReceiverMethods replacing hand-rolled pointerReceiversWithMethod ✅ Identical semantics per library source
exportedBuiltinNames sort removed ReportMissing sorts its input before joining — message still deterministic

@dgageot dgageot merged commit e4d4256 into docker:main May 7, 2026
8 checks passed
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