Skip to content

Move each builtin command to its own package#15

Merged
AlexandreYang merged 1 commit intomainfrom
dd/refactor/builtins-per-package
Mar 9, 2026
Merged

Move each builtin command to its own package#15
AlexandreYang merged 1 commit intomainfrom
dd/refactor/builtins-per-package

Conversation

@AlexandreYang
Copy link
Copy Markdown
Member

What does this PR do?

Refactors the interp/builtins package so each builtin command lives in its own sub-package under interp/builtins/<command>/. The root builtins package retains only the shared types (HandlerFunc, CallContext, Result) and registry (Register, Lookup).

Motivation

Isolating each builtin into its own package improves modularity: each command has a clear boundary, its own tests, and independent dependencies. It also makes the import allowlist test more maintainable as new builtins are added.

Testing

  • go build ./interp/... — compiles clean
  • go test ./interp/... — all pass (except pre-existing TestHeadPermissionDenied which fails only when running as root)
  • go test ./tests/ — all pass including the updated TestBuiltinImportAllowlist
  • goimports / gofmt — no formatting changes needed

Checklist

  • Tests added/updated
  • Documentation updated (if applicable)

Summary: Each builtin (cat, echo, exit, head, break, continue, true, false) is now a separate Go package that self-registers via init() + builtins.Register(). Shared logic for break/continue lives in builtins/internal/loopctl. The runner wires everything together with blank imports.

Changes:

  • Exported Register in interp/builtins/builtins.go
  • Created per-builtin packages: cat/, echo/, exit/, head/, break/, continue/, true/, false/
  • Created internal/loopctl/ for shared break/continue loop-control logic
  • Moved head tests from builtins/tests/head/ into builtins/head/
  • Added blank imports in interp/runner_exec.go to trigger init() registration
  • Updated TestBuiltinImportAllowlist to walk sub-packages recursively, exempting the parent builtins package and internal/ imports
  • Deleted old flat files (cat.go, echo.go, exit.go, head.go, break_continue.go, true_false.go) and tests/ directory

PR by Bits
View session in Datadog

Comment @DataDog to request changes

Co-authored-by: AlexandreYang <49917914+AlexandreYang@users.noreply.github.com>
@datadog-prod-us1-5
Copy link
Copy Markdown
Contributor

datadog-prod-us1-5 Bot commented Mar 9, 2026

View session in Datadog

Bits Dev status: ✅ Done

CI Auto-fix: Disabled | Enable

Comment @DataDog to request changes

@AlexandreYang AlexandreYang merged commit a3e1b7e into main Mar 9, 2026
8 checks passed
@AlexandreYang AlexandreYang deleted the dd/refactor/builtins-per-package branch March 9, 2026 22:32
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.

1 participant