docs: architecture overview - #82
Merged
Merged
Conversation
gocanto
added a commit
that referenced
this pull request
Jul 27, 2026
* ci: run tests for the release-refactor integration branch * refactor(ts): TS-1 — move kernel/syntax/io/hosts modules into subdirectories (#64) * build(ts): make sidecar module resolution subdirectory-capable Replace the enumerated #sidecar imports maps in both package.json files with wildcard patterns (#sidecar/*.js and #sidecar/* -> ./src/*.ts). The .js entry is kept and listed first: Node's longest-suffix pattern precedence needs it so #sidecar/foo.js resolves to ./src/foo.ts. Make the test runner glob recursive ('src/**/*.test.ts', quoted so Node's test runner expands it), re-root the alias-specifiers scan at the test's own directory (import.meta.dirname) instead of a single module's resolved path, and copy sidecar sources recursively in stage-ts-assets.sh while still excluding *.test.ts. No source files move; the current flat layout keeps working. * refactor(ts): move kernel/syntax/io/hosts modules into subdirectories Mechanically relocate sidecar modules into kernel/, syntax/, io/, and hosts/ subdirectories (each with its co-located tests) and retarget every #sidecar/<name> specifier to #sidecar/<subdir>/<name>. Delete types.ts: its Edit type moves into syntax/edits.ts and its Node re-export is dropped in favour of importing Node directly from #sidecar/syntax/node-schema. No class-shape, logic, or exported-symbol changes. * refactor(ts): convert parser/ast/edits services to instance classes; add SourceDocument (#65) Convert the static-namespace core services in the TS sidecar into real instance classes and introduce the SourceDocument value object. - syntax/sources.ts -> syntax/source-parser.ts: Sources -> SourceParser - syntax/ast.ts -> syntax/ast-reader.ts: Ast -> AstReader, absorbing the node helpers from SourceText (sourceOf, callParens, unwrapChainExpression) - syntax/edits.ts: Edits -> EditApplier (Edit type unchanged) - syntax/source-document.ts: new frozen value object (of/withText factories) absorbing SourceText's text-coordinate queries (lineStart, lineIndent, indentUnit, slice) - node-schema.ts: hasCommentBetween becomes a ParsedSourceDto instance method - syntax/source-text.ts deleted Passes stay static this stage; each constructs the services as private static readonly fields (temporary scaffolding until DI in TS-3). Behavior is unchanged; 167 tests pass. * refactor(ts): TS-3 — FormattingPass contract, pipeline machinery, policies (#66) * refactor(ts): add FormattingPass contract and pass-pipeline machinery Introduce the FormattingPass interface (computeEdits over a SourceDocument) and the declarative pipeline primitives that replace segment.ts's hardcoded sequence: IterationBudget (once / untilStable), PipelineStep (fixed-point loop with early stop), and PassPipeline (left-to-right fold with a reporting label). Unit tests pin budget exhaustion, early stop, no-op passes, and the empty pipeline. * refactor(ts): split rules.ts into three injectable policy classes Extract the module-level Rules class and its eight Sets into instance policies with dependency-object constructors and no static state: VueReactivityIdioms (VUE_PRIMITIVE_CALLS; isVuePrimitiveStatement), ClassMemberPolicy (member-kind Sets; classify / isMethodPair / isPropertyToMethodTransition), and StatementSpacingPolicy (remaining statement Sets; needsBlankLine, injecting members + vue). rules.ts is removed in the rewire commit once its last consumers are gone. * refactor(ts): convert segment passes and block splitter to instances Move the four segment passes to src/passes/ as instance classes implementing FormattingPass with dependency-object constructors: BodyWrapPass, ClassReorderPass (injects ClassMemberPolicy), DeclarationReorderPass, and BlankLinePass (injects StatementSpacingPolicy). BlankLinePass returns zero-width newline inserts instead of positions+insert(); positions are deduplicated so EditApplier byte-matches the former insert (pinned by test, including the repeated-position case). EmbeddedBlocks becomes the injectable EmbeddedBlockSplitter instance. Old static classes are removed in the rewire commit. * refactor(ts): wire the segment pipeline through a factory and file formatter Add PipelineFactory (constructor-injected services + policies; segmentPipeline composes bodyWrap untilStable(5) -> classReorder once -> declarationReorder once -> blankLine once, labelled 'blank-lines'), SourceFileEditor (the unified read->transform->compare->write extracted from both formatFile paths), and FileFormatter (host-aware embedded-block rewrite or direct pipeline apply). Rewire FormatPipeline's segment path through FileFormatter(segmentPipeline) and its validation through the injected EmbeddedBlockSplitter; fluent-chains and file-targets move to the splitter instance with minimal edits (TS-4 owns fluent). Retarget the segment property and blank-line-rule tests at PipelineFactory.segmentPipeline() with assertions unchanged. Delete segment.ts, rules.ts, the four old static passes, and embedded-blocks.ts. * refactor(ts): TS-4 — dissolve the format-pipeline ⇄ fluent-chains cycle (#67) * refactor(ts): split fluent/expanded/drizzle detection into DI passes Extract the chain-detection logic from FluentChains into FluentChainPass, convert ExpandedCalls to ExpandedCallPass with full { parser, ast, edits } injection, and rename DrizzleQueries to DrizzleQueryPass implementing FormattingPass. DrizzleQueryPass injects { parser, edits } at its boundary while its ~30 detection/emission helpers and module Sets stay static pending the TS-5 internal split. Add a shared mapPool concurrency helper. * refactor(ts): give FormatPipeline its final DI shape and a fluent pipeline Move format-pipeline.ts into pipeline/, extract validate() into a SyntaxValidator (deps { sourceFiles, splitter, parser }), and reduce FormatPipeline to { editor, processRunner, validator } with a runPass(FileFormatter, files, mode) generic over both pipelines. Add PipelineFactory.fluentPipeline() composing fluent-chain, Drizzle-query, and expanded-call passes in the exact order the old FluentChains.format used, plus segmentFormatter/fluentFormatter/syntaxValidator builders so the composition root vends the whole formatting graph. * refactor(ts): rewire CLIs onto the fluent pipeline and drop the import cycle Reduce fluent-chains.ts to a thin CLI shim that runs the fluent FileFormatter through the composition root, deleting the FluentChains class and the lazy 'await import(#sidecar/format-pipeline)' cycle hack. Update blank-lines, validate-syntax, and format-all to build the pipeline from { editor, processRunner, validator } and pass FileFormatters to runPass, keeping every stdout label byte-identical. Convert the fluent, Drizzle, expanded, files, and format-all tests to drive the passes and pipelines directly. * refactor(ts): TS-5 — split the drizzle monolith into collaborators (#78) * refactor(ts): extract Drizzle vocabulary, scanner, classifier, and writer Split the DrizzleQueryPass monolith's detection and emission internals into intent-bearing collaborators under src/passes/drizzle/: - DrizzleVocabulary owns every recognised name set behind predicates, built by the sanctioned DrizzleVocabulary.standard() factory. - DrizzleImportScanner collects Drizzle imports into a frozen DrizzleImports value object (localImport/hasNamespace/isEmpty). - DrizzleCallClassifier holds the is/should predicates over {ast, vocabulary}. - DrizzleArgumentWriter holds the format/emit helpers over {ast, vocabulary, classifier}. Logic is moved verbatim, rebinding the former static #ast reads and module-level Sets onto injected instances. Adds direct unit tests for the vocabulary and import scanner seams. * refactor(ts): reduce DrizzleQueryPass to orchestration over collaborators Move the pass to src/passes/drizzle/ and shrink it to pure orchestration: it parses, scans imports via DrizzleImportScanner, walks calls the DrizzleCallClassifier approves, and asks DrizzleArgumentWriter for each edit. The leftover static #ast field is gone; ast now flows in through the constructor. PipelineFactory builds the vocabulary/scanner/classifier/writer graph and injects it. The golden test moves alongside the pass with its assertions unchanged, its plumbing adapted to the collaborator graph. * refactor(ts): TS-6 — unified CLI architecture under src/cli/ (#79) * refactor(ts): drop the unused io/files inventory helper The Files directory-scan utility had no production consumers; only its own test imported it. Remove both. * refactor(ts): unify sidecar CLIs under src/cli/ with a composition root Introduce a single CLI architecture under packages/ts/sidecar/src/cli/: - CliCommand interface: run(argv) returns an exit code, never process.exit. - CompositionRoot: the single production wiring point, composing PipelineFactory's pass graph with the Node IO adapters, FormatPipeline, reporters, and command classes (formatAllCommand, segmentPassCommand, fluentPassCommand, validateSyntaxCommand). - One PassReporter and one SyntaxReporter, replacing FormatAllReporter, SyntaxErrorReporter, and the ad-hoc reporting loops in the blank-lines and fluent-chains entrypoints. Console output bytes are unchanged. - FormatAllCommand owns the segment -> oxfmt -> fluent -> segment -> validate schedule; FormatPassCommand backs both standalone format passes; ValidateSyntaxCommand backs standalone validation. - DTOs moved to cli/: PassCliDto, CliOptionsDto (cli/format-all-cli-dto), SyntaxCliDto. Flag grammar unchanged. - Entry files (blank-lines, fluent-chains, validate-syntax) hold only main() plus the run-as-main guard. Update sidecar.ts pipeline dispatch, the validate-syntax package script, and the moved CLI tests to the new paths. Naming inversion is resolved: reporters are named for what they report, not their former host module. * refactor(ts): TS-7 — final sweep: cycle-free proof, static audit, cleanups (#80) * refactor(ts): break the cli format-all <-> composition-root import cycle Extract FormatAllCommand into cli/format-all-command.ts (importing only its direct deps), matching format-pass-command.ts/validate-syntax-command.ts. cli/format-all.ts is now a pure entry (main + run-as-main guard importing only CompositionRoot); composition-root.ts imports the command module. sidecar.ts still imports #sidecar/cli/format-all. * refactor(ts): convert host scanners and file-target policy to injected instances Removes the last static-namespace classes in the sidecar. VueScript and MarkdownFences become instances injected into EmbeddedBlockSplitter. The static FileTargets class (and its module-private EmbeddedBlockSplitter singleton, the TS-3 compromise) becomes FileTargetPolicy, an instance holding its splitter, constructed once in PipelineFactory and injected into the declaration-aware passes (ExpandedCallPass, DrizzleQueryPass) and the CLI commands. PassCliDto.parse now takes the policy as an explicit argument. * refactor(ts): drop the ParsedSourceDto.from double-cast Widen the failure branch of ParsedSourceDto.from to carry an unparameterised z.ZodError. The schema output omits the DTO's own methods, so the previous `parsed.error as unknown as z.ZodError<ParsedSourceDto>` bridged the phantom generic; since the only caller (SourceParser.parse) discards the typed payload and raises a fresh SourceUnparsable, the unparameterised error is exact and the double cast is gone. * refactor(ts): sweep stale class-name references and type-only imports Reword two test comments that named the pre-refactor BlankLines.insert and FluentChains.format classes to describe the current BlankLinePass reference behaviour and the fluent pipeline order. Make EmbeddedBlockSplitter's scanner imports type-only now that they are used solely as constructor parameter types. * style(ts): apply the sidecar formatter to the TS-7 changes Runs `make format-all` (the repo's own pipeline) over the stage's edits: the new FileTargetPolicy parameter pushes several constructor signatures past the width threshold, so the formatter expands their inline dependency-object types to one member per line and splits the delegated extractBlocks chains. Pure whitespace normalisation, no behaviour change. * refactor(go): G1 — characterization goldens, dead surface, config unification (#71) * test(driver): pin pipeline transcript, report, CLI dispatch, and spacing goldens Add characterization tests capturing current behavior before the G1 config/dead-surface refactor: - orchestrator: byte-for-byte pipeline stderr transcript goldens (success, quiet, TS/Go failure paths) under testdata, color forced off. - report: byte-for-byte text render goldens for check and format modes, alongside the existing json/agent projection goldens. - app + fmtkit-go: byte-for-byte usage/version/exit-code dispatch goldens for both binaries, pinning their deliberately divergent behavior. - spacing: testdata-driven before/after corpus covering statement gaps, selector-call setup, type-decl spacing, type ordering, embed repair, and import aliases. - driver/config: load tests for full round-trip, partial-keeps-defaults, explicit-empty-list override, empty-file defaults, and missing-path error. * refactor: remove dead code surface None of these have production references (verified by grep across packages/, goreleaser, and infra): - driver/cmd/fmtkit-sources: unused binary; goreleaser builds only driver/cmd/fmtkit. Drop its coverage-gate exclusion in infra/task.sh. - engine.filterFiles: unused helper and its internal test. - engine.Report.AllErrors: unused; report projections use Errors + per-result Error directly. - engine.FileResult.Diff + generateDiff + diff.go: the diff was computed in processFile but never read by any report path (json tag was omitempty and never surfaced), so rendered output is unchanged. - vet.Default: only referenced by vet's own tests; callers construct vet.Config directly. * refactor(config): compose driver config from formatter config driver/config previously declared structural twins of formatter/config (Rules/Formatters/Config) and bridged them with a field-copy FormatterConfig() plus seven viper SetDefault calls restating the defaults. Now Config embeds formatterconfig.Config with a mapstructure squash tag and adds only the Vet toggle the CLI owns. Default() composes formatterconfig.Default() with vet enabled; Formatter() returns the embedded config; WithJobs() applies the --jobs override. load.go unmarshals onto a Default()-populated struct and relies on mapstructure leaving absent keys untouched, so the SetDefault lines are gone and the on-disk schema stays byte-compatible. runner.go's one call site is renamed FormatterConfig -> Formatter. * refactor(spacing): share parse state through analyzer types (#72) * refactor(driver): split sourcefiles into gitfiles, filetypes, prettierignore (#73) Split the sourcefiles package's three unrelated engines into intent-bearing packages: - gitfiles: git file discovery (Selection, Tree, Files, ChangedPaths, IntersectChanged) — the runner.go git-set intersection moves here. - filetypes: the extension taxonomy (Filter.Formattable/Lintable). - prettierignore: the .prettierignore matcher (Matcher, Load, Ignores, FilterAbs). sourcefiles shrinks to the Collector composition. Transitional package-level wrappers (Collect, CollectLintable, ChangedPaths, Options, Selection re-export, Run) delegate to the new types so tsruntime and app stay untouched; command.go's flag parser is exported as RunCLI with Run kept as an alias. G5 will adopt the new types directly. * refactor(go): G4 — typed sidecarproto seam + tsruntime reshape (#75) * refactor(g4): extract wire protocol into sidecarproto Introduce driver/internal/sidecarproto as the single source of truth for the Go/TS wire protocol: asset filenames, dispatch modes, override env vars, the per-mode argument vectors, and the sidecar summary parsers. Delegate the orchestrator's TS-owned summary scraping to sidecarproto.ParsePipelineSummary / ParseLintSummary while its Go-report scraping stays put (G6 retires it). * refactor(g4): reshape tsruntime Support into Assets/Invoker/PrettierMigration Split the Support god-type into three cohesive types in one package: Assets owns the extracted toolchain directory, Invoker spawns the pipeline and lint, and PrettierMigration derives an oxfmt config from a project's Prettier setup. All argv/env construction now flows through sidecarproto; the sole os.Getenv site for the override vars is sidecarproto.ReadOverrides, called once by NewInvoker. Fix the three pre-existing errcheck warnings in prettier.go by discarding the Fprintf results per house style. Callers in app/ switch to NewInvoker(assets).Run*/Request with no behavioural change. * style(g4): apply fmtkit self-formatting to new sidecarproto/tsruntime files The spacing rule hoists type definitions to the top of each file; apply it to the newly added sidecarproto sources and the reshaped assets.go so the repo's own formatter is a fixed point. * refactor(go): G5 — typed report, gotool use case, shared command table (#77) * refactor(g5): typed Mode/Format and Renderer in driver/report Replace the stringly Render(w, format, cwd, mode, report) entry point with a Renderer{Root, Mode} whose Render takes a typed report.Format, and move the render helpers to unexported methods. Add report.Mode (ModeCheck/ModeFormat), report.Format with ParseFormat (same unknown-format error), and Combined.ExitCode(Mode) lifted from the CLI's exitCode. Output bytes and JSON tags are unchanged; the text/json/agent goldens still pass byte-identical. * refactor(g5): split cli god-runner into gotool package Rename driver/internal/cli to driver/internal/gotool, split along its concerns: parser.go (ParseInvocation -> typed Invocation, now validating --format via report.ParseFormat), execute.go (reusable Execute/Request/Outcome core G6's Go step will call), and runner.go (thin Runner{Stdout,Stderr,Scope} orchestration: parse -> config.Load -> WithJobs -> Execute -> Renderer.Render -> ExitCode). report.Mode flows end-to-end; cli.Mode and the mode.String() laundering are gone. The changed-files path adopts formatterengine.CollectGoFiles + gitfiles.Tree.IntersectChanged directly. * refactor(g5): command dispatch table with app as composition root Add driver/internal/command (Command/Set/Dispatch/PrintUsage): the two binaries' divergences — umbrella exits 2 and prefixes Go usage 'fmtkit go', standalone fmtkit-go exits 1 and prefixes 'fmtkit' — live in Set.ErrExit/Header/Name and a shared goCommandSet(name, errExit) builder, not in branching code. app.Umbrella and app.GoCLI build the two Sets; the umbrella 'go' subcommand is GoCLI's set rebuilt with name 'fmtkit go' and the umbrella's exit code. cmd/fmtkit and cmd/fmtkit-go become thin shims calling Dispatch. Deletes app/golang.go, app/usage.go and fmtkit-go's duplicated run/printUsage switch. All CLI dispatch goldens (both binaries: usage text, exit codes, version) pass byte-identical. * refactor(g5): delete transitional sourcefiles shims, adopt real types Remove every 'Transitional: G5' shim in sourcefiles — the Selection alias, Options, Collect/CollectLintable, ChangedPaths, the collectorFor helper, and the Run/RunCLI duplication — leaving Collector plus a New constructor and a single typed sources Run entry that builds a Collector directly. Migrate the last callers off the shims: tsruntime.Invoker's collect/collectLintable now build a Collector and Request.Selection is gitfiles.Selection. git grep 'Transitional: G5' is now empty. * refactor(go): G6 — typed pipeline steps + console; delete summarize.go (#81) * refactor(console): extract the ANSI progress logger into a console package Move the orchestrator's inline logger (section/detail/failure/stream rendering plus FORCE_COLOR/NO_COLOR/tty detection) into a dedicated driver/internal/console package. DetectColor now resolves the color mode once and NewPrinter takes the resolved ColorMode, so the printer never reads the environment inline. The orchestrator delegates to console.Printer; rendering is byte-identical (transcript goldens unchanged). * refactor(pipeline): give the pipeline typed steps, delete stdout scraping Replace the orchestrator's Tools func-triple and RunFormat with a generic Step/Result/Detail abstraction: Pipeline runs an ordered []Step, owning only the section/tee/quiet-failure-dump mechanics. The concrete steps (TS lint, TS format, Go format) live in the app composition root, which builds them and frames the run (target header, completion footer), resolving color once via console.DetectColor. The Go step derives its summary details from the typed gotool.Outcome (new Runner.RunReport returns it) instead of scraping the rendered report text; summarize.go and its Go-report regexes are deleted. The TS steps parse their captured output through sidecarproto plus the driver's own [sources]/[lint] bookkeeping notices, as before. Transcript goldens are unchanged and still pass byte-identical, now driven by fake Steps in the orchestrator test. * refactor(pipeline): rename the orchestrator package to pipeline Pure rename now that the package is the generic step runner rather than the format-specific orchestrator: git mv the directory (carrying the transcript goldens unchanged) and rename the package identifier and its importers in app. No behavior change. * style: apply fmtkit self-formatting and fix stale rename references Running the real pipeline over the tree (make format-all) reorders the new files to fmtkit's canonical form: type declarations hoisted to the top of the file, blank lines before statements following assignments. Pure reordering, no behavior change. Also updates two sidecarproto doc comments that still named the old orchestrator package to point at the pipeline steps that now own the Go-report bookkeeping. * docs: describe the post-refactor architecture and its contracts (#82) * docs: fold the architecture overview into the README (#83) * refactor(go): G7 — separate Go and TS behaviour behind language toolchains (#84) * refactor(go): G7 — add the toolchain contract and registry * refactor(go): G7 — move gotool to the golang lane with its format step * refactor(go): G7 — move the TS lane under typescript/ with its steps * refactor(go): G7 — move embedded under typescript/ and retarget staging * refactor(go): G7 — rewire app to build lanes through the toolchain registry * docs: describe the language-lane driver layout (G7) * infra responsability * imports * chore(deps): refresh Go and TS toolchains Go 1.26.4 -> 1.26.5, plus the seven places the version is echoed by hand (README badge and prose, the smoke-test and testutil go.mod fixtures, the vet go.work fixtures, and the Dockerfile fixture string). CI reads the version from go.mod, so those echoes drift silently otherwise. Go modules: go-isatty 0.0.22 -> 0.0.24, go.yaml.in/yaml/v3 3.0.4 -> 3.0.5. Every other module was already current. go mod tidy also drops a stale gopkg.in/check.v1 indirect. TypeScript: oxfmt 0.59.0 -> 0.60.0, oxlint 1.74.0 -> 1.75.0, oxc-parser 0.140.0 -> 0.141.0, vite-plus 0.2.4 -> 0.2.6. The vite-plus bump is coupled to oxc-parser: 0.2.6 pins @oxc-project/types to =0.141.0, so moving one without the other splits the resolution again. pnpm 10.33.0 -> 11.17.0 and Node 25.8.2 -> 25.9.0. Regenerating the lockfile under pnpm 11 also clears two pre-existing drifts: stale @oxfmt/binding-* 0.57.0 entries, and root vite-plus resolving typescript 6.0.3 while both TS packages resolved 7.0.2. pnpm 11 gates dependency build scripts, so pnpm-workspace.yaml now records esbuild explicitly. It has never run in this workspace -- Vite+ resolves the native binary through the @esbuild/<platform> optional dependency -- so it stays off, which preserves current behaviour rather than changing it. oxfmt 0.60.0 was the risk here: the in-process CLI patch anchors on strings in oxfmt's bundle, and a mismatch shows up as a hang rather than a failure. The binary smoke test passes, and the patcher resolved the new content-hashed API module, so the anchors and the shim still hold. * refactor(go): G8 — adopt the Go 1.26 standard library Go 1.26 turns `go fix` into an analyzer-driven fixer. Running it supplies the mechanical half of this change: slices.Contains for two hand-rolled membership loops, strings.SplitSeq where a split slice was only ranged over, a min() clamp, and the loop-variable copies that have been redundant since Go 1.22. The formatter's own spacing rule accepts the blank line those copies leave behind, so they are removed here rather than left to drift. errors.AsType replaces all five errors.As sites, each of which had to declare a target variable first. config/load.go targets a value type rather than a pointer, so it reads the result as a plain bool. The larger piece is embeds.go. It hand-rolled the //go:embed directive grammar twice -- once over string, once over []byte -- as two token-for-token identical functions that existed only because one caller had bytes and the other had a string. go/ast.ParseDirective is that grammar, so both collapse into a single predicate and the byte path delegates to it. The two test tables that shadowed the twins become one that exercises both entry points, which is what should have pinned them in the first place: tested apart, nothing forced them to agree. Before deleting anything, ParseDirective was checked against the pinned cases -- tab separator, bare //go:embed, //go:embedded, //go:embed-fixtures -- since a divergence there would have changed formatter behaviour silently. The rest is stdlib that already existed: cmp.Compare for two mirrored three-way switches, slices.Equal for a reimplemented loop, slices.ContainsFunc for four flag-and-break loops, slices.Sorted(maps.Keys(...)) for a collect-then-sort, and sync.WaitGroup.Go for the one worker pool. assets.go was the last "sort" importer in the module; it now uses slices.Sort, and the import is gone. Two loops were deliberately left alone: collapseEmbedSpacing mutates its own counter to skip a line, and isVarDeclStart shares a skeleton with the embed predicates but has a different prefix and delimiter set. Net -186 lines. Corpus goldens, -race across all three modules, and golangci-lint v2.12.2 all pass unchanged. * refactor(go): G9 — collapse duplicated logic behind shared functions None of this needs generics. Every duplicate here is single-type -- strings end to end -- so a type parameter would only ever instantiate at one type. Plain functions are the right tool, and where a generic looked tempting it was the wrong answer for a concrete reason, noted below. gitfiles and sourcefiles each walked a set of scopes step for step identically: default to ".", join against the tree dir, stat and skip what is missing, list, join, clean, dedupe, sort. sourcefiles added only a keep predicate and a warning for a scope that is not there. That walk now lives once as Tree.Walk, which returns the missing scopes rather than deciding what they mean: the git lane still ignores them, and the TS lane still warns, because its scopes come from the user and a typo should say so. vetStatus existed twice, verbatim, and its own comment admitted the two copies had to agree -- the kind of coupling that holds right up until it doesn't. It is now report.VetStatus, with report.VetSummary alongside it so the status sentences also have one home; the text render wraps them in color, the pipeline step prints them plain. The two error-render loops in text.go were byte-identical across seventeen lines, differing only in which error slice they ranged over. This is the one place a generic really looks right and is not: Go has no field-access constraint, so [E interface{ File string }] is inexpressible, and the workaround takes two accessor closures at every call site -- more code than the two strings it abstracts. renderErrorEntry takes the two strings. vet.go wrapped exec.ExitError twice with only the label differing, so wrapExitError takes the label. The git test helpers were character-identical across two packages, and a testutil package already existed that both ignored. They move there. Deliberately not done: the projection.go pair is four lines each, and a shared helper would be a trap -- a third site builds the same struct from an already-relativized path, so folding relativePath into a helper would apply it twice. The exec.ExitError-to-exit-code extraction in app/exit.go and typescript/step.go is three lines each in two packages with nothing shared between them; G8 already removed the redundant part. Net -81 lines. 22 packages pass under -race, golangci-lint clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Final PR of the refactor series: the architecture doc the repo lacked — package/module map for both halves, the FormattingPass/PipelineFactory contract, the sidecarproto seam and asset flow, the sanctioned-exceptions list, verification/golden rules, and the self-formatting member-order note.