From 86e49fa768daa45e3fe8feb4828a52cd68ac5a35 Mon Sep 17 00:00:00 2001 From: Jose Alekhinne Date: Fri, 3 Apr 2026 20:50:09 -0700 Subject: [PATCH 01/13] docs: add audit conventions guide, tighten TestNoMagicStrings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add docs/reference/audit-conventions.md — contributor-facing guide covering all 34 internal/audit/ tests with before/after examples and fix patterns. Remove 4 overly broad exemptions from TestNoMagicStrings: format verbs ("%d", "%x"), URL scheme prefixes ("https://"), HTML entities ("<"), and internal/err/ package exemption. Fix all 17 surfaced violations by replacing fmt.Sprintf with strconv/hex stdlib calls and extracting magic strings to config constants (cfgHTTP.Prefix*, cfgGit.FlagLastN, cfgWarn.CopilotClose, cfgWarn.JSONEncode). Spec: specs/ast-audit-contributor-guide.md Signed-off-by: Jose Alekhinne --- .context/TASKS.md | 7 + docs/reference/audit-conventions.md | 839 +++ internal/audit/magic_strings_test.go | 31 +- .../cli/journal/core/source/format/format.go | 5 +- internal/cli/journal/core/source/list.go | 3 +- .../cli/journal/core/wikilink/wikilink.go | 7 +- internal/cli/pad/core/export/timestamp.go | 4 +- internal/cli/system/core/version/version.go | 8 +- internal/cli/task/cmd/complete/run.go | 6 +- internal/cli/trace/cmd/tag/run.go | 4 +- internal/cli/trace/core/file/file.go | 2 +- internal/cli/trace/core/show/show.go | 2 +- internal/config/git/git.go | 3 + internal/config/http/http.go | 12 + internal/config/warn/warn.go | 6 + internal/journal/parser/copilot_cli.go | 3 +- internal/memory/state.go | 4 +- internal/write/archive/snapshot.go | 13 +- internal/write/bootstrap/bootstrap.go | 3 +- site/reference/audit-conventions/index.html | 4559 +++++++++++++++++ site/search.json | 2 +- specs/ast-audit-contributor-guide.md | 30 + 22 files changed, 5496 insertions(+), 57 deletions(-) create mode 100644 docs/reference/audit-conventions.md create mode 100644 site/reference/audit-conventions/index.html create mode 100644 specs/ast-audit-contributor-guide.md diff --git a/.context/TASKS.md b/.context/TASKS.md index 31100f257..e54809aa0 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -25,6 +25,13 @@ TASK STATUS LABELS: `#in-progress`: currently being worked on (add inline, don't move task) --> +### Misc + +- [ ] gitnexus analyze --embeddings --skill : we need a make target, but also this + butchers AGENTS.md and CLAUDE.md so those files need to be reverted and + GITNEXUS.md should be updated accordingly. The make target should remind + the user about it. + ### Architecture Docs - [ ] Publish architecture docs to docs/: copy ARCHITECTURE.md, DETAILED_DESIGN domain files, and CHEAT-SHEETS.md to docs/reference/. Sanitize intervention points into docs/contributing/. Exclude DANGER-ZONES.md and ARCHITECTURE-PRINCIPAL.md (internal only). Spec: specs/publish-architecture-docs.md #priority:medium #added:2026-04-03-150000 diff --git a/docs/reference/audit-conventions.md b/docs/reference/audit-conventions.md new file mode 100644 index 000000000..47b5b9d79 --- /dev/null +++ b/docs/reference/audit-conventions.md @@ -0,0 +1,839 @@ +# Audit Conventions: Common Patterns and Fixes + +This guide documents the code conventions enforced by `internal/audit/` +AST tests. Each section shows the violation pattern, the fix, and the +rationale. When a test fails, find the matching section below. + +All tests skip `_test.go` files. The patterns apply only to production +code under `internal/`. + +--- + +## Variable Shadowing (bare `err :=` reuse) + +**Test:** `TestNoVariableShadowing` + +When a function has multiple `:=` assignments to `err`, each shadows +the previous one. This makes it impossible to tell which error a later +`if err != nil` is checking. + +**Before:** + +```go +func Run(cmd *cobra.Command) error { + data, err := os.ReadFile(path) + if err != nil { + return err + } + + result, err := json.Unmarshal(data) // shadows first err + if err != nil { + return err + } + + err = validate(result) // shadows again + return err +} +``` + +**After:** + +```go +func Run(cmd *cobra.Command) error { + data, readErr := os.ReadFile(path) + if readErr != nil { + return readErr + } + + result, parseErr := json.Unmarshal(data) + if parseErr != nil { + return parseErr + } + + validateErr := validate(result) + return validateErr +} +``` + +**Rule:** Use descriptive error names (`readErr`, `writeErr`, `parseErr`, +`walkErr`, `absErr`, `relErr`) so each error site is independently +identifiable. + +--- + +## Import Name Shadowing + +**Test:** `TestNoImportNameShadowing` + +When a local variable has the same name as an imported package, the +import becomes inaccessible in that scope. + +**Before:** + +```go +import "github.com/ActiveMemory/ctx/internal/session" + +func process(session *entity.Session) { // param shadows import + // session package is now unreachable here +} +``` + +**After:** + +```go +import "github.com/ActiveMemory/ctx/internal/session" + +func process(sess *entity.Session) { + // session package still accessible +} +``` + +**Rule:** Parameters, variables, and return values must not reuse +imported package names. Common renames: `session` -> `sess`, +`token` -> `tok`, `config` -> `cfg`, `entry` -> `ent`. + +--- + +## Magic Strings + +**Test:** `TestNoMagicStrings` + +String literals in function bodies are invisible to refactoring tools +and cause silent breakage when the value changes in one place but not +another. + +**Before (string literals):** + +```go +func loadContext() { + data := filepath.Join(dir, "TASKS.md") + if strings.HasSuffix(name, ".yaml") { + // ... + } +} +``` + +**After:** + +```go +func loadContext() { + data := filepath.Join(dir, config.FilenameTask) + if strings.HasSuffix(name, config.ExtYAML) { + // ... + } +} +``` + +**Before (format verbs — also caught):** + +```go +func EntryHash(text string) string { + h := sha256.Sum256([]byte(text)) + return fmt.Sprintf("%x", h[:8]) +} +``` + +**After:** + +```go +func EntryHash(text string) string { + h := sha256.Sum256([]byte(text)) + return hex.EncodeToString(h[:cfgFmt.HashPrefixLen]) +} +``` + +**Before (URL schemes — also caught):** + +```go +if strings.HasPrefix(target, "https://") || + strings.HasPrefix(target, "http://") { + return target +} +``` + +**After:** + +```go +if strings.HasPrefix(target, cfgHTTP.PrefixHTTPS) || + strings.HasPrefix(target, cfgHTTP.PrefixHTTP) { + return target +} +``` + +**Exempt from this check:** + +- Empty string `""`, single space `" "`, indentation strings +- Regex capture references (`$1`, `${name}`) +- `const` and `var` definition sites (that's where constants live) +- Struct tags +- Import paths +- Packages under `internal/config/`, `internal/assets/tpl/` + +**Rule:** If a string is used for comparison, path construction, or +appears in 3+ files, it belongs in `internal/config/` as a constant. +Format strings belong in `internal/config/` as named constants +(e.g., `cfgGit.FlagLastN`, `cfgTrace.RefFormat`). User-facing prose +belongs in `internal/assets/` YAML files accessed via `desc.Text()`. + +**Common fix for `fmt.Sprintf` with format verbs:** + +| Pattern | Fix | +|---------|-----| +| `fmt.Sprintf("%d", n)` | `strconv.Itoa(n)` | +| `fmt.Sprintf("%d", int64Val)` | `strconv.FormatInt(int64Val, 10)` | +| `fmt.Sprintf("%x", bytes)` | `hex.EncodeToString(bytes)` | +| `fmt.Sprintf("%q", s)` | `strconv.Quote(s)` | +| `fmt.Sscanf(s, "%d", &n)` | `strconv.Atoi(s)` | +| `fmt.Sprintf("-%d", n)` | `fmt.Sprintf(cfgGit.FlagLastN, n)` | +| `"https://"` | `cfgHTTP.PrefixHTTPS` | +| `"<"` | config constant in `config/html/` | + +--- + +## Direct Printf Calls + +**Test:** `TestNoPrintfCalls` + +`cmd.Printf` and `cmd.PrintErrf` bypass the write-package formatting +pipeline and scatter user-facing text across the codebase. + +**Before:** + +```go +func Run(cmd *cobra.Command, args []string) { + cmd.Printf("Found %d tasks\n", count) +} +``` + +**After:** + +```go +func Run(cmd *cobra.Command, args []string) { + write.TaskCount(cmd, count) +} +``` + +**Rule:** All formatted output goes through `internal/write/` which +uses `cmd.Print`/`cmd.Println` with pre-formatted strings from +`desc.Text()`. + +--- + +## Raw Time Format Strings + +**Test:** `TestNoRawTimeFormats` + +Inline time format strings (`"2006-01-02"`, `"15:04:05"`) drift when +one call site is updated but others are missed. + +**Before:** + +```go +func formatDate(t time.Time) string { + return t.Format("2006-01-02") +} +``` + +**After:** + +```go +func formatDate(t time.Time) string { + return t.Format(cfgTime.DateFormat) +} +``` + +**Rule:** All time format strings must use constants from +`internal/config/time/`. + +--- + +## Direct Flag Registration + +**Test:** `TestNoFlagBindOutsideFlagbind` + +Direct cobra flag calls (`.Flags().StringVar()`, etc.) scatter flag +wiring across dozens of `cmd.go` files. Centralizing through +`internal/flagbind/` gives one place to audit flag names, defaults, +and description key lookups. + +**Before:** + +```go +func Cmd() *cobra.Command { + var output string + c := &cobra.Command{Use: cmd.UseStatus} + c.Flags().StringVarP(&output, "output", "o", "", + "output format") + return c +} +``` + +**After:** + +```go +func Cmd() *cobra.Command { + var output string + c := &cobra.Command{Use: cmd.UseStatus} + flagbind.StringFlagShort(c, &output, flag.Output, + flag.OutputShort, cmd.DescKeyOutput) + return c +} +``` + +**Rule:** All flag registration goes through `internal/flagbind/`. +If the helper you need doesn't exist, add it to `flagbind/flag.go` +before using it. + +--- + +## TODO Comments + +**Test:** `TestNoTODOComments` + +TODO, FIXME, HACK, and XXX comments in production code are invisible +to project tracking. They accumulate silently and never get addressed. + +**Before:** + +```go +// TODO: handle pagination +func listEntries() []Entry { +``` + +**After:** + +Remove the comment and add a task to `.context/TASKS.md`: + +``` +- [ ] Handle pagination in listEntries (internal/task/task.go) +``` + +**Rule:** Deferred work lives in TASKS.md, not in source comments. + +--- + +## Dead Exports + +**Test:** `TestNoDeadExports` + +Exported symbols with zero references outside their definition file +are dead weight. They increase API surface, confuse contributors, and +cost maintenance. + +**Fix:** Either delete the export (preferred) or demote it to +unexported if it's still used within the file. + +If the symbol existed for historical reasons and might be needed again, +move it to `quarantine/deadcode/` with a `.dead` extension. This +preserves the code in git without polluting the live codebase: + +``` +quarantine/deadcode/internal/config/flag/flag.go.dead +``` + +Each `.dead` file includes a header: + +```go +// Dead exports quarantined from internal/config/flag/flag.go +// Quarantined: 2026-04-02 +// Restore from git history if needed. +``` + +**Rule:** If a test-only allowlist entry is needed (the export exists +only for test use), add the fully qualified symbol to `testOnlyExports` +in `dead_exports_test.go`. Keep this list small — prefer eliminating +the export. + +--- + +## Core Package Structure + +**Test:** `TestCoreStructure` + +`core/` directories under `internal/cli/` must contain only `doc.go` +and test files at the top level. All domain logic lives in subpackages. +This prevents `core/` from becoming a god package. + +**Before:** + +``` +internal/cli/dep/core/ + go.go # violation — logic at core/ level + python.go # violation + node.go # violation + types.go # violation +``` + +**After:** + +``` +internal/cli/dep/core/ + doc.go # package doc only + golang/ + golang.go + golang_test.go + doc.go + python/ + python.go + python_test.go + doc.go + node/ + node.go + node_test.go + doc.go +``` + +**Rule:** Extract each logical unit into its own subpackage under +`core/`. Each subpackage gets a `doc.go`. The subpackage name should +match the domain concept (`golang`, `check`, `fix`, `store`), not a +generic label (`util`, `helper`). + +--- + +## Cross-Package Types + +**Test:** `TestCrossPackageTypes` + +When a type defined in one package is used from a different module +(e.g., `cli/doctor` importing a type from `cli/notify`), the type +has crossed its module boundary. Cross-cutting types belong in +`internal/entity/` for discoverability. + +**Before:** + +```go +// internal/cli/notify/core/types.go +type NotifyPayload struct { ... } + +// internal/cli/doctor/core/check/check.go +import "github.com/ActiveMemory/ctx/internal/cli/notify/core" +func check(p core.NotifyPayload) { ... } +``` + +**After:** + +```go +// internal/entity/notify.go +type NotifyPayload struct { ... } + +// internal/cli/doctor/core/check/check.go +import "github.com/ActiveMemory/ctx/internal/entity" +func check(p entity.NotifyPayload) { ... } +``` + +**Exempt:** Types inside `entity/`, `proto/`, `core/` subpackages, +and `config/` packages. Same-module usage (e.g., `cli/doctor/cmd/` +using `cli/doctor/core/`) is not flagged. + +--- + +## Type File Convention + +**Test:** `TestTypeFileConvention`, `TestTypeFileConventionReport` + +Exported types in `core/` subpackages should live in `types.go` (the +convention from CONVENTIONS.md), not scattered across implementation +files. This makes type definitions discoverable. +`TestTypeFileConventionReport` generates a diagnostic summary of all +type placements for triage. + +**Exception:** `entity/` organizes by domain (`task.go`, `session.go`), +`proto/` uses `schema.go`, and `err/` packages colocate error types +with their domain context. + +--- + +## DescKey / YAML Linkage + +**Test:** `TestDescKeyYAMLLinkage` + +Every DescKey constant must have a corresponding key in the YAML asset +files, and every YAML key must have a corresponding DescKey constant. +Orphans in either direction mean dead text or runtime panics. + +**Fix for orphan YAML key:** Delete the YAML entry, or add the +corresponding `DescKey` constant in `config/embed/{text,cmd,flag}/`. + +**Fix for orphan DescKey:** Delete the constant, or add the +corresponding entry in the YAML file under +`internal/assets/commands/text/`, `cmd/`, or `flag/`. + +If the orphan YAML entry was once valid but the feature was removed, +move the YAML entry to a `.dead` file in `quarantine/deadcode/`. + +--- + +## Package Doc Quality + +**Test:** `TestPackageDocQuality` + +Every package under `internal/` must have a `doc.go` with a meaningful +package doc comment (at least 8 lines of real content). One-liners and +file-list patterns (`// - foo.go`, `// Source files:`) are flagged +because they drift as files change. + +**Template:** + +```go +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +// Package mypackage does X. +// +// It handles Y by doing Z. The main entry point is [FunctionName] +// which accepts A and returns B. +// +// Configuration is read from [config.SomeConstant]. Output is +// written through [write.SomeHelper]. +// +// This package is used by [parentpackage] during the W lifecycle +// phase. +package mypackage +``` + +--- + +## Inline Regex Compilation + +**Test:** `TestNoInlineRegexpCompile` + +`regexp.MustCompile` and `regexp.Compile` inside function bodies +recompile the pattern on every call. Compiled patterns belong at +package level. + +**Before:** + +```go +func parse(s string) bool { + re := regexp.MustCompile(`\d{4}-\d{2}-\d{2}`) + return re.MatchString(s) +} +``` + +**After:** + +```go +// In internal/config/regex/regex.go: +// DatePattern matches ISO date format (YYYY-MM-DD). +var DatePattern = regexp.MustCompile(`\d{4}-\d{2}-\d{2}`) + +// In calling package: +func parse(s string) bool { + return regex.DatePattern.MatchString(s) +} +``` + +**Rule:** All compiled regexes live in `internal/config/regex/` as +package-level `var` declarations. Two tests enforce this: +`TestNoInlineRegexpCompile` catches function-body compilation, and +`TestNoRegexpOutsideRegexPkg` catches package-level compilation +outside `config/regex/`. + +--- + +## Doc Comments + +**Test:** `TestDocComments` + +All functions (exported and unexported), structs, and package-level +variables must have a doc comment. Config packages allow group doc +comments for `const` blocks. + +**Before:** + +```go +func buildIndex(entries []Entry) map[string]int { +``` + +**After:** + +```go +// buildIndex maps entry names to their position in the +// ordered slice for O(1) lookup during reconciliation. +// +// Parameters: +// - entries: ordered slice of entries to index +// +// Returns: +// - map[string]int: name-to-position mapping +func buildIndex(entries []Entry) map[string]int { +``` + +**Rule:** Every function, struct, and package-level `var` gets a doc +comment in godoc format. Functions include `Parameters:` and +`Returns:` sections. Structs with 2+ fields document every field. +See CONVENTIONS.md for the full template. + +--- + +## Line Length + +**Test:** `TestLineLength` + +Lines in non-test Go files must not exceed 80 characters. This is a +hard check, not a suggestion. + +**Before:** + +```go +_ = trace.Record(fmt.Sprintf(cfgTrace.RefFormat, cfgTrace.RefTypeTask, matchedNum), state.Dir()) +``` + +**After:** + +```go +ref := fmt.Sprintf( + cfgTrace.RefFormat, cfgTrace.RefTypeTask, matchedNum, +) +_ = trace.Record(ref, state.Dir()) +``` + +**Rule:** Break at natural points: function arguments, struct fields, +chained calls. Long strings (URLs, struct tags) are the rare +acceptable exception. + +--- + +## Literal Whitespace + +**Test:** `TestNoLiteralWhitespace` + +Bare whitespace string and byte literals (`"\n"`, `"\r\n"`, `"\t"`) +must not appear outside `internal/config/token/`. All other packages +use the token constants. + +**Before:** + +```go +output := strings.Join(lines, "\n") +``` + +**After:** + +```go +output := strings.Join(lines, token.Newline) +``` + +**Rule:** Whitespace literals are defined once in +`internal/config/token/`. Use `token.Newline`, `token.Tab`, +`token.CRLF`, etc. + +--- + +## Magic Numeric Values + +**Test:** `TestNoMagicValues` + +Numeric literals in function bodies need constants, with narrow +exceptions. + +**Before:** + +```go +if len(entries) > 100 { + entries = entries[:100] +} +``` + +**After:** + +```go +if len(entries) > config.MaxEntries { + entries = entries[:config.MaxEntries] +} +``` + +**Exempt:** `0`, `1`, `-1`, `2`–`10`, strconv radix/bitsize args +(`10`, `32`, `64` in `strconv.Parse*`/`Format*`), octal permissions +(caught separately by `TestNoRawPermissions`), and `const`/`var` +definition sites. + +--- + +## Inline Separators + +**Test:** `TestNoInlineSeparators` + +`strings.Join` calls must use token constants for their separator +argument, not string literals. + +**Before:** + +```go +result := strings.Join(parts, ", ") +``` + +**After:** + +```go +result := strings.Join(parts, token.CommaSep) +``` + +**Rule:** Separator strings live in `internal/config/token/`. + +--- + +## Stuttery Function Names + +**Test:** `TestNoStutteryFunctions` + +Function names must not redundantly include their package name as a +PascalCase word boundary. Go callers already write `pkg.Function`, +so `pkg.PkgFunction` stutters. + +**Before:** + +```go +// In package write +func WriteJournal(cmd *cobra.Command, ...) { +``` + +**After:** + +```go +// In package write +func Journal(cmd *cobra.Command, ...) { +``` + +**Exempt:** Identity functions like `write.Write` / `write.write`. + +--- + +## Mixed Visibility + +**Test:** `TestNoMixedVisibility` + +Files with exported functions must not also contain unexported +functions. Public API and private helpers live in separate files. + +**Before:** + +``` +load.go + func Load() { ... } // exported + func parseHeader() { ... } // unexported — violation +``` + +**After:** + +``` +load.go + func Load() { ... } // exported only +parse.go + func parseHeader() { ... } // private helper +``` + +**Exempt:** Files with exactly one function, `doc.go`, test files. + +--- + +## Stray err.go Files + +**Test:** `TestNoStrayErrFiles` + +`err.go` files must only exist under `internal/err/`. Error +constructors anywhere else create a broken-window pattern where +contributors add local error definitions when they see a local +`err.go`. + +**Fix:** Move the error constructor to `internal/err//`. + +--- + +## CLI Cmd Structure + +**Test:** `TestCLICmdStructure` + +Each `cmd/$sub/` directory under `internal/cli/` may contain only +`cmd.go`, `run.go`, `doc.go`, and test files. Extra `.go` files +(helpers, output formatters, types) belong in the corresponding +`core/` subpackage. + +**Before:** + +``` +internal/cli/doctor/cmd/root/ + cmd.go + run.go + format.go # violation — helper in cmd dir +``` + +**After:** + +``` +internal/cli/doctor/cmd/root/ + cmd.go + run.go +internal/cli/doctor/core/format/ + format.go + doc.go +``` + +--- + +## DescKey Namespace + +**Test:** `TestUseConstantsOnlyInCobraUse`, +`TestDescKeyOnlyInLookupCalls`, +`TestNoWrongNamespaceLookup` + +Three tests enforce DescKey/Use constant discipline: + +1. `Use*` constants appear only in cobra `Use:` struct field + assignments — never as arguments to `desc.Text()` or elsewhere. +2. `DescKey*` constants are passed only to `assets.CommandDesc()`, + `assets.FlagDesc()`, or `desc.Text()` — never to cobra `Use:`. +3. No cross-namespace lookups — `TextDescKey` must not be passed to + `CommandDesc()`, `FlagDescKey` must not be passed to `Text()`, etc. + +--- + +## YAML Examples / Registry Linkage + +**Test:** `TestExamplesYAMLLinkage`, `TestRegistryYAMLLinkage` + +Every key in `examples.yaml` and `registry.yaml` must match a known +entry type constant. Prevents orphan entries that are never rendered. + +**Fix:** Delete the orphan YAML entry, or add the corresponding +constant in `config/entry/`. + +--- + +## Other Enforced Patterns + +These tests follow the same fix approach — extract the operation to +its designated package: + +| Test | Violation | Fix | +|------|-----------|-----| +| `TestNoNakedErrors` | `fmt.Errorf`/`errors.New` outside `internal/err/` | Add error constructor to `internal/err//` | +| `TestNoRawFileIO` | Direct `os.ReadFile`, `os.Create`, etc. | Use `io.SafeReadFile`, `io.SafeWriteFile`, etc. | +| `TestNoRawLogging` | Direct `fmt.Fprintf(os.Stderr, ...)` | Use `log/warn.Warn()` or `log/event.Append()` | +| `TestNoExecOutsideExecPkg` | `exec.Command` outside `internal/exec/` | Add command to `internal/exec//` | +| `TestNoCmdPrintOutsideWrite` | `cmd.Print*` outside `internal/write/` | Add output helper to `internal/write//` | +| `TestNoRawPermissions` | Octal literals (`0644`, `0755`) | Use `config/fs.PermFile`, `config/fs.PermExec`, etc. | +| `TestNoErrorsAs` | `errors.As()` | Use `errors.AsType()` (generic, Go 1.23+) | +| `TestNoStringConcatPaths` | `dir + "/" + file` | Use `filepath.Join(dir, file)` | + +--- + +## General Fix Workflow + +When an audit test fails: + +1. **Read the error message.** It includes `file:line` and a + description of the violation. +2. **Find the matching section above.** The test name maps directly + to a section. +3. **Apply the pattern.** Most fixes are mechanical: extract to the + right package, rename a variable, or replace a literal with a + constant. +4. **Run `make test` before committing.** Audit tests run as part of + `go test ./internal/audit/`. +5. **Don't add allowlist entries as a first resort.** Fix the code. + Allowlists exist only for genuinely unfixable cases (test-only + exports, config packages that are definitionally exempt). diff --git a/internal/audit/magic_strings_test.go b/internal/audit/magic_strings_test.go index 1d938bd3c..836b803dc 100644 --- a/internal/audit/magic_strings_test.go +++ b/internal/audit/magic_strings_test.go @@ -33,15 +33,13 @@ var exemptStringPackages = []string{ "internal/config/", "internal/config", "internal/assets/tpl", - "internal/err/", } // TestNoMagicStrings flags magic string literals in non-test // Go files under internal/. // // Exempt: empty string, single space, indentation strings, -// single characters, format verbs, regex replacements, HTML -// entities, URL scheme prefixes, config/tpl/err packages, +// regex capture references, config/tpl/err packages, // file-level const/var definitions, import paths, struct tags. // // Test files are exempt. @@ -127,27 +125,11 @@ func checkMagicString( return } - // Format verbs ("%s", "%d %s", etc.). - if isFormatString(s) { - return - } - // Regex capture group references. if isRegexRef(s) { return } - // HTML entities (<, >, etc.). - if strings.HasPrefix(s, "&") && - strings.HasSuffix(s, ";") { - return - } - - // URL scheme prefixes. - if strings.HasSuffix(s, "://") { - return - } - *violations = append(*violations, posString(pkg.Fset, lit.Pos())+ ": magic string "+lit.Value, @@ -165,17 +147,6 @@ func isExemptStringPackage(pkgPath string) bool { return false } -// fmtVerb matches a printf format directive. -var fmtVerb = regexp.MustCompile( - `%[-+#0 ]*\d*\.?\d*[sdvqwfegxXobctpT]`, -) - -// isFormatString reports whether s contains a printf -// format directive. -func isFormatString(s string) bool { - return fmtVerb.MatchString(s) -} - // regexRef matches regex capture group references. var regexRef = regexp.MustCompile(`\$\d|\$\{`) diff --git a/internal/cli/journal/core/source/format/format.go b/internal/cli/journal/core/source/format/format.go index 5bf8eac5b..7bb962971 100644 --- a/internal/cli/journal/core/source/format/format.go +++ b/internal/cli/journal/core/source/format/format.go @@ -10,6 +10,7 @@ import ( "encoding/json" "fmt" "html" + "strconv" "strings" "github.com/ActiveMemory/ctx/internal/assets/read/desc" @@ -270,7 +271,7 @@ func JournalEntryPart( sb.WriteString(tpl.MetaDetailsClose + nl + nl) // Token stats as collapsible HTML table - turnStr := fmt.Sprintf("%d", s.TurnCount) + turnStr := strconv.Itoa(s.TurnCount) io.SafeFprintf(&sb, tpl.MetaDetailsOpen, turnStr) io.SafeFprintf(&sb, tpl.MetaRow+nl, desc.Text(text.DescKeyLabelMetaTurns), turnStr) @@ -282,7 +283,7 @@ func JournalEntryPart( tpl.MetaRow+nl, desc.Text(text.DescKeyLabelMetaTokens), tokenSummary) if totalParts > 1 { io.SafeFprintf(&sb, tpl.MetaRow+nl, desc.Text(text.DescKeyLabelMetaParts), - fmt.Sprintf("%d", totalParts)) + strconv.Itoa(totalParts)) } sb.WriteString(tpl.MetaDetailsClose + nl + nl) diff --git a/internal/cli/journal/core/source/list.go b/internal/cli/journal/core/source/list.go index 31e436678..13246e6d3 100644 --- a/internal/cli/journal/core/source/list.go +++ b/internal/cli/journal/core/source/list.go @@ -8,6 +8,7 @@ package source import ( "fmt" + "strconv" "strings" goTime "time" @@ -154,7 +155,7 @@ func RunList(cmd *cobra.Command, opts Opts) error { time.DateTimeFmt, ) dur := srcFmt.Duration(s.Duration) - turns := fmt.Sprintf("%d", s.TurnCount) + turns := strconv.Itoa(s.TurnCount) tokens := "" if s.TotalTokens > 0 { tokens = sharedFmt.Tokens(s.TotalTokens) diff --git a/internal/cli/journal/core/wikilink/wikilink.go b/internal/cli/journal/core/wikilink/wikilink.go index 0a94f3abb..35736de08 100644 --- a/internal/cli/journal/core/wikilink/wikilink.go +++ b/internal/cli/journal/core/wikilink/wikilink.go @@ -14,6 +14,7 @@ import ( "github.com/ActiveMemory/ctx/internal/assets/read/desc" "github.com/ActiveMemory/ctx/internal/config/embed/text" "github.com/ActiveMemory/ctx/internal/config/file" + cfgHTTP "github.com/ActiveMemory/ctx/internal/config/http" "github.com/ActiveMemory/ctx/internal/config/obsidian" "github.com/ActiveMemory/ctx/internal/config/regex" "github.com/ActiveMemory/ctx/internal/config/token" @@ -40,9 +41,9 @@ func ConvertMarkdownLinks(content string) string { target := parts[2] // Skip external links - if strings.HasPrefix(target, "http://") || - strings.HasPrefix(target, "https://") || - strings.HasPrefix(target, "file://") { + if strings.HasPrefix(target, cfgHTTP.PrefixHTTP) || + strings.HasPrefix(target, cfgHTTP.PrefixHTTPS) || + strings.HasPrefix(target, cfgHTTP.PrefixFile) { return match } diff --git a/internal/cli/pad/core/export/timestamp.go b/internal/cli/pad/core/export/timestamp.go index 28d6d18cc..d8d1ee052 100644 --- a/internal/cli/pad/core/export/timestamp.go +++ b/internal/cli/pad/core/export/timestamp.go @@ -7,7 +7,7 @@ package export import ( - "fmt" + "strconv" "time" "github.com/ActiveMemory/ctx/internal/config/token" @@ -21,6 +21,6 @@ import ( // Returns: // - string: Label in the form "-