Skip to content

Commit 2471a2f

Browse files
authored
refactor: migrate fmt.Print to ui helpers, tighten fmtprint archtest to near-hard rule (#113)
* refactor: migrate fmt.Print to ui helpers, tighten fmtprint archtest to near-hard rule * chore: trigger CI
1 parent f92dd40 commit 2471a2f

19 files changed

Lines changed: 157 additions & 260 deletions

File tree

docs/HARNESS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Three regulation categories:
4646
| Arch. | `no-raw-http` | L1 | `internal/archtest/http_test.go` |
4747
| Arch. | `no-os-getenv-home` | L1 | `internal/archtest/envhome_test.go` |
4848
| Arch. | `dryrun` — destructive ops must check `DryRun` | L1 | `internal/archtest/dryrun_test.go` |
49+
| Arch. | `no-raw-fmtprint` — UI output via `ui.*` helpers, not raw `fmt.Print*` | L1 | `internal/archtest/fmtprint_test.go` |
4950
| Behav. | L1 unit + integration + contract (faked runners *and* real brew/git/npm in temp dirs) | pre-push, CI | `make test-unit` |
5051
| Behav. | L2 contract schema (against openboot-contract repo) | CI | `.github/workflows/test.yml` `contract` job |
5152
| Behav. | L3 e2e binary | release | `make test-e2e` |
Lines changed: 0 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -1,134 +1,6 @@
11
# Baseline for archtest rule "fmtprint".
22
# Each line is <file>:<line> of a known existing violation.
33
# Regenerate: ARCHTEST_UPDATE_BASELINE=1 go test ./internal/archtest/...
4-
internal/brew/brew_install.go:133
5-
internal/brew/brew_install.go:221
6-
internal/brew/brew_install.go:231
7-
internal/brew/brew_install.go:238
8-
internal/brew/brew_install.go:270
9-
internal/brew/brew_install.go:274
10-
internal/brew/brew_install.go:276
11-
internal/cli/install.go:368
12-
internal/cli/install.go:372
13-
internal/cli/install.go:393
14-
internal/cli/install.go:405
15-
internal/cli/install.go:477
16-
internal/cli/install.go:484
17-
internal/cli/root.go:86
18-
internal/cli/snapshot.go:256
19-
internal/cli/sync_helpers.go:45
20-
internal/cli/sync_helpers.go:50
21-
internal/cli/sync_helpers.go:54
22-
internal/cli/sync_helpers.go:60
23-
internal/cli/sync_helpers.go:62
24-
internal/cli/sync_helpers.go:66
25-
internal/cli/sync_helpers.go:72
26-
internal/cli/sync_helpers.go:79
27-
internal/cli/sync_helpers.go:81
28-
internal/cli/sync_helpers.go:85
29-
internal/cli/sync_helpers.go:86
30-
internal/cli/sync_helpers.go:87
31-
internal/cli/sync_helpers.go:95
32-
internal/cli/update.go:104
33-
internal/diff/format.go:14
34-
internal/diff/format.go:16
35-
internal/diff/format.go:19
36-
internal/diff/format.go:97
37-
internal/diff/format.go:99
38-
internal/diff/format.go:102
39-
internal/diff/format.go:105
40-
internal/diff/format.go:107
41-
internal/diff/format.go:116
42-
internal/diff/format.go:118
43-
internal/diff/format.go:122
44-
internal/diff/format.go:125
45-
internal/diff/format.go:127
46-
internal/diff/format.go:136
47-
internal/diff/format.go:138
48-
internal/diff/format.go:142
49-
internal/diff/format.go:146
50-
internal/diff/format.go:149
51-
internal/diff/format.go:158
52-
internal/diff/format.go:160
53-
internal/diff/format.go:164
54-
internal/diff/format.go:167
55-
internal/diff/format.go:169
56-
internal/diff/format.go:176
57-
internal/diff/format.go:182
58-
internal/diff/format.go:190
59-
internal/diff/format.go:193
60-
internal/diff/format.go:204
61-
internal/diff/format.go:209
62-
internal/dotfiles/dotfiles.go:79
63-
internal/dotfiles/dotfiles.go:116
64-
internal/dotfiles/dotfiles.go:130
65-
internal/dotfiles/dotfiles.go:203
66-
internal/dotfiles/dotfiles.go:207
67-
internal/dotfiles/dotfiles.go:271
68-
internal/dotfiles/dotfiles.go:437
69-
internal/dotfiles/dotfiles.go:440
70-
internal/dotfiles/dotfiles.go:444
71-
internal/dotfiles/dotfiles.go:446
72-
internal/installer/installer.go:48
73-
internal/installer/installer.go:50
74-
internal/installer/installer.go:54
75-
internal/installer/installer.go:128
76-
internal/installer/installer.go:134
77-
internal/installer/installer.go:140
78-
internal/installer/installer.go:151
79-
internal/installer/installer.go:158
80-
internal/installer/installer.go:183
81-
internal/installer/installer.go:192
82-
internal/installer/installer.go:204
83-
internal/installer/installer.go:214
84-
internal/installer/installer.go:216
85-
internal/installer/installer.go:220
86-
internal/installer/installer.go:231
87-
internal/installer/installer.go:241
88-
internal/installer/installer.go:302
89-
internal/installer/installer.go:304
90-
internal/installer/installer.go:307
91-
internal/installer/installer.go:337
92-
internal/installer/step_git.go:12
93-
internal/installer/step_git.go:17
94-
internal/installer/step_git.go:23
95-
internal/installer/step_git.go:36
96-
internal/installer/step_packages.go:77
97-
internal/installer/step_packages.go:83
98-
internal/installer/step_packages.go:134
99-
internal/installer/step_packages.go:139
100-
internal/installer/step_packages.go:161
101-
internal/installer/step_packages.go:207
102-
internal/installer/step_packages.go:209
103-
internal/installer/step_packages.go:211
104-
internal/installer/step_packages.go:229
105-
internal/installer/step_shell.go:17
106-
internal/installer/step_shell.go:40
107-
internal/installer/step_shell.go:58
108-
internal/installer/step_shell.go:89
109-
internal/installer/step_system.go:20
110-
internal/installer/step_system.go:24
111-
internal/installer/step_system.go:38
112-
internal/installer/step_system.go:48
113-
internal/installer/step_system.go:52
114-
internal/installer/step_system.go:58
115-
internal/installer/step_system.go:60
116-
internal/installer/step_system.go:69
117-
internal/installer/step_system.go:76
118-
internal/installer/step_system.go:90
119-
internal/installer/step_system.go:94
1204
internal/macos/macos.go:94
1215
internal/macos/macos.go:141
1226
internal/macos/macos.go:153
123-
internal/npm/npm.go:112
124-
internal/npm/npm.go:128
125-
internal/npm/npm.go:131
126-
internal/npm/npm.go:164
127-
internal/npm/npm.go:182
128-
internal/shell/shell.go:200
129-
internal/shell/shell.go:203
130-
internal/updater/updater.go:189
131-
internal/updater/updater.go:234
132-
internal/updater/updater.go:239
133-
internal/updater/updater.go:251
134-
internal/updater/updater.go:255

internal/archtest/fmtprint_test.go

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,19 @@ func isOsStderr(gf goFile, expr ast.Expr) bool {
3232
return local != "" && ident.Name == local
3333
}
3434

35+
// isAddressOf reports whether expr is a unary & expression (e.g. &sb).
36+
// Writes to an address-of receiver are local buffer writes (strings.Builder,
37+
// bytes.Buffer, etc.), not user-visible output — exempt from the rule.
38+
func isAddressOf(expr ast.Expr) bool {
39+
unary, ok := expr.(*ast.UnaryExpr)
40+
return ok && unary.Op.String() == "&"
41+
}
42+
3543
// findFmtPrint finds forbidden fmt.Print*/fmt.F* calls in gf.
3644
// - fmt.Print, fmt.Println, fmt.Printf: always forbidden (stdout only).
3745
// - fmt.Fprint, fmt.Fprintln, fmt.Fprintf: forbidden unless first argument
38-
// is os.Stderr (stderr is acceptable for debug/error output).
46+
// is os.Stderr (stderr is acceptable for debug/error output) or an
47+
// address-of expression like &sb (local buffer, not user output).
3948
func findFmtPrint(gf goFile) []callSite {
4049
local := importedAs(gf.file, "fmt")
4150
if local == "" {
@@ -49,7 +58,7 @@ func findFmtPrint(gf goFile) []callSite {
4958
"Printf": true,
5059
}
5160
// writerFuncs take an io.Writer as first arg — forbidden unless that writer
52-
// is os.Stderr.
61+
// is os.Stderr or a local buffer (&sb, &buf, etc.).
5362
writerFuncs := map[string]bool{
5463
"Fprint": true,
5564
"Fprintln": true,
@@ -77,11 +86,12 @@ func findFmtPrint(gf goFile) []callSite {
7786
return true
7887
}
7988
if writerFuncs[fn] {
80-
// Only flag if the first argument is NOT os.Stderr.
81-
if len(call.Args) == 0 || !isOsStderr(gf, call.Args[0]) {
82-
p := gf.fset.Position(call.Pos())
83-
out = append(out, callSite{file: gf.path, line: p.Line})
89+
// Exempt: os.Stderr (debug/error output) and &var (local buffer).
90+
if len(call.Args) > 0 && (isOsStderr(gf, call.Args[0]) || isAddressOf(call.Args[0])) {
91+
return true
8492
}
93+
p := gf.fset.Position(call.Pos())
94+
out = append(out, callSite{file: gf.path, line: p.Line})
8595
return true
8696
}
8797
return true

internal/brew/brew_install.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func InstallWithProgress(ctx context.Context, cliPkgs, caskPkgs []string, dryRun
130130
skipped := total - len(newCli) - len(newCask)
131131
if skipped > 0 {
132132
ui.Muted(fmt.Sprintf(" %d already installed, %d to install", skipped, len(newCli)+len(newCask)))
133-
fmt.Println()
133+
ui.Println()
134134
}
135135

136136
if len(newCli)+len(newCask) == 0 {
@@ -218,7 +218,7 @@ func retryFailedJobs(ctx context.Context, allFailed []failedJob, installedFormul
218218
return nil
219219
}
220220

221-
fmt.Printf("\nRetrying %d failed packages...\n", len(allFailed))
221+
ui.Printf("\nRetrying %d failed packages...\n", len(allFailed))
222222

223223
for _, f := range allFailed {
224224
var errMsg string
@@ -228,14 +228,14 @@ func retryFailedJobs(ctx context.Context, allFailed []failedJob, installedFormul
228228
errMsg = installFormulaWithError(ctx, f.name)
229229
}
230230
if errMsg == "" {
231-
fmt.Printf(" ✔ %s (retry succeeded)\n", f.name)
231+
ui.Printf(" ✔ %s (retry succeeded)\n", f.name)
232232
if f.isCask {
233233
*installedCasks = append(*installedCasks, f.name)
234234
} else {
235235
*installedFormulae = append(*installedFormulae, aliasMap[f.name])
236236
}
237237
} else {
238-
fmt.Printf(" ✗ %s (still failed)\n", f.name)
238+
ui.Printf(" ✗ %s (still failed)\n", f.name)
239239
}
240240
}
241241

@@ -267,13 +267,13 @@ func handleFailedJobs(failed []failedJob) {
267267
return
268268
}
269269

270-
fmt.Println()
270+
ui.Println()
271271
ui.Error(fmt.Sprintf("%d packages failed to install:", len(failed)))
272272
for _, f := range failed {
273273
if f.errMsg != "" {
274-
fmt.Printf(" - %s (%s)\n", f.name, f.errMsg)
274+
ui.Printf(" - %s (%s)\n", f.name, f.errMsg)
275275
} else {
276-
fmt.Printf(" - %s\n", f.name)
276+
ui.Printf(" - %s\n", f.name)
277277
}
278278
}
279279
}

internal/cli/install.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -365,11 +365,11 @@ func runSyncInstall(source *syncpkg.SyncSource, pickRaw string) error { //nolint
365365
}
366366
}
367367

368-
fmt.Println()
368+
ui.Println()
369369
plan := buildInstallPlan(diff, rc)
370370
result, execErr := syncpkg.Execute(plan, false)
371371

372-
fmt.Println()
372+
ui.Println()
373373
if result.Installed > 0 {
374374
ui.Success(fmt.Sprintf("Installed %d package(s)", result.Installed))
375375
}
@@ -390,7 +390,7 @@ func runSyncInstall(source *syncpkg.SyncSource, pickRaw string) error { //nolint
390390
// the top of a sync-source install. Warns in yellow if > 90 days stale.
391391
func printSyncSourceHeader(source *syncpkg.SyncSource) {
392392
label := sourceLabel(source)
393-
fmt.Println()
393+
ui.Println()
394394
if source.SyncedAt.IsZero() {
395395
ui.Info(fmt.Sprintf("→ Syncing with %s", label))
396396
} else {
@@ -402,7 +402,7 @@ func printSyncSourceHeader(source *syncpkg.SyncSource) {
402402
ui.Info(fmt.Sprintf("→ Syncing with %s (last synced %s)", label, rel))
403403
}
404404
}
405-
fmt.Println()
405+
ui.Println()
406406
}
407407

408408
// applyPickFlagToRemoteConfig filters rc to the packages named in pickRaw.
@@ -474,14 +474,14 @@ func filterStrings(in []string, keep map[string]bool) []string {
474474
// config. Returns the rc to install (possibly filtered by user's picks) and
475475
// whether the user wants to continue. When cancelled, returns (nil, false, nil).
476476
func promptCustomizeAndApply(rc *config.RemoteConfig) (*config.RemoteConfig, bool, error) {
477-
fmt.Println()
477+
ui.Println()
478478
ui.Info(fmt.Sprintf("→ %s/%s", rc.Username, rc.Slug))
479479
ui.Muted(fmt.Sprintf(" CLI tools: %d", len(rc.Packages)))
480480
ui.Muted(fmt.Sprintf(" Apps: %d", len(rc.Casks)))
481481
if len(rc.Npm) > 0 {
482482
ui.Muted(fmt.Sprintf(" npm: %d", len(rc.Npm)))
483483
}
484-
fmt.Println()
484+
ui.Println()
485485

486486
choice, err := ui.SelectOption(
487487
fmt.Sprintf("Install %d packages?", len(rc.Packages)+len(rc.Casks)+len(rc.Npm)),

internal/cli/root.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/openbootdotdev/openboot/internal/config"
1313
"github.com/openbootdotdev/openboot/internal/logging"
14+
"github.com/openbootdotdev/openboot/internal/ui"
1415
"github.com/openbootdotdev/openboot/internal/updater"
1516
)
1617

@@ -83,7 +84,7 @@ var versionCmd = &cobra.Command{
8384
Use: "version",
8485
Short: "Print version information",
8586
Run: func(cmd *cobra.Command, args []string) {
86-
fmt.Printf("OpenBoot v%s\n", version)
87+
ui.Printf("OpenBoot v%s\n", version)
8788
},
8889
}
8990

internal/cli/snapshot.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func captureJSONSnapshot() error {
253253
return fmt.Errorf("marshal snapshot: %w", err)
254254
}
255255
fmt.Fprintln(os.Stderr, "✓ Snapshot complete")
256-
fmt.Println(string(data))
256+
ui.Println(string(data))
257257
return nil
258258
}
259259

internal/cli/sync_helpers.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,57 +42,57 @@ func printInstallDiff(d *syncpkg.SyncDiff) {
4242
len(d.MissingNpm) > 0 || len(d.MissingTaps) > 0
4343

4444
if hasPkgAdditions {
45-
fmt.Printf(" %s\n", ui.Green("Packages to install"))
45+
ui.Printf(" %s\n", ui.Green("Packages to install"))
4646
printMissing("Formulae", d.MissingFormulae)
4747
printMissing("Casks", d.MissingCasks)
4848
printMissing("NPM", d.MissingNpm)
4949
printMissing("Taps", d.MissingTaps)
50-
fmt.Println()
50+
ui.Println()
5151
}
5252

5353
if len(d.MacOSChanged) > 0 {
54-
fmt.Printf(" %s\n", ui.Green("macOS Changes"))
54+
ui.Printf(" %s\n", ui.Green("macOS Changes"))
5555
for _, p := range d.MacOSChanged {
5656
desc := p.Desc
5757
if desc == "" {
5858
desc = fmt.Sprintf("%s.%s", p.Domain, p.Key)
5959
}
60-
fmt.Printf(" %s: %s %s %s\n", desc, p.LocalValue, ui.Yellow("→"), p.RemoteValue)
60+
ui.Printf(" %s: %s %s %s\n", desc, p.LocalValue, ui.Yellow("→"), p.RemoteValue)
6161
}
62-
fmt.Println()
62+
ui.Println()
6363
}
6464

6565
if d.Shell != nil {
66-
fmt.Printf(" %s\n", ui.Green("Shell Changes"))
66+
ui.Printf(" %s\n", ui.Green("Shell Changes"))
6767
if d.Shell.ThemeChanged {
6868
localTheme := d.Shell.LocalTheme
6969
if localTheme == "" {
7070
localTheme = "(none)"
7171
}
72-
fmt.Printf(" Theme: %s %s %s\n", localTheme, ui.Yellow("→"), d.Shell.RemoteTheme)
72+
ui.Printf(" Theme: %s %s %s\n", localTheme, ui.Yellow("→"), d.Shell.RemoteTheme)
7373
}
7474
if d.Shell.PluginsChanged {
7575
localPlugins := strings.Join(d.Shell.LocalPlugins, ", ")
7676
if localPlugins == "" {
7777
localPlugins = "(none)"
7878
}
79-
fmt.Printf(" Plugins: %s %s %s\n", localPlugins, ui.Yellow("→"), strings.Join(d.Shell.RemotePlugins, ", "))
79+
ui.Printf(" Plugins: %s %s %s\n", localPlugins, ui.Yellow("→"), strings.Join(d.Shell.RemotePlugins, ", "))
8080
}
81-
fmt.Println()
81+
ui.Println()
8282
}
8383

8484
if d.DotfilesChanged {
85-
fmt.Printf(" %s\n", ui.Green("Dotfiles"))
86-
fmt.Printf(" Repo: %s %s %s\n", fallbackStr(d.LocalDotfiles, "(none)"), ui.Yellow("→"), d.RemoteDotfiles)
87-
fmt.Println()
85+
ui.Printf(" %s\n", ui.Green("Dotfiles"))
86+
ui.Printf(" Repo: %s %s %s\n", fallbackStr(d.LocalDotfiles, "(none)"), ui.Yellow("→"), d.RemoteDotfiles)
87+
ui.Println()
8888
}
8989
}
9090

9191
func printMissing(category string, missing []string) {
9292
if len(missing) == 0 {
9393
return
9494
}
95-
fmt.Printf(" %s (%d): %s\n", category, len(missing), strings.Join(missing, ", "))
95+
ui.Printf(" %s (%d): %s\n", category, len(missing), strings.Join(missing, ", "))
9696
}
9797

9898
func fallbackStr(s, def string) string {

internal/cli/update.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func runListBackups() error {
101101
}
102102
ui.Header(fmt.Sprintf("Backups in %s", dir))
103103
for _, n := range names {
104-
fmt.Println(" " + n)
104+
ui.Println(" " + n)
105105
}
106106
return nil
107107
}

0 commit comments

Comments
 (0)