Greptile config

.greptile/config.json

@@ -0,0 +1,58 @@
+{
+  "strictness": 2,
+  "commentTypes": ["logic", "syntax", "style"],
+  "triggerOnUpdates": true,
+  "statusCheck": true,
+  "ignorePatterns": "**/testdata/**\n**/*.golden\n**/go.sum\n**/vendor/**\n**/node_modules/**",
+  "instructions": "gomapper (github.com/KARTIKrocks/gomapper) is a Go code-generation tool, not a runtime reflection library: it parses a target package with golang.org/x/tools/go/packages (internal/loader), matches struct fields between a source and destination type (internal/matcher), and renders Go source via text/template (internal/generator) in one of three modes — func (pure functions, zero deps), register (calls into the sibling mapper library's runtime registry), or both. The generated file is committed by the end user and marked 'DO NOT EDIT'. Because there is no runtime reflection, the correctness bar is different from a typical library: bugs here mean gomapper silently emits the wrong Go source, or emits non-deterministic source across identical runs, rather than misbehaving at runtime. Prioritize field-matching correctness, cross-mode (func/register/both) output parity, and generation determinism over style. See .greptile/rules.md for rationale and the mechanisms behind each rule.",
+  "rules": [
+    {
+      "id": "silent-unmapped-field-mismatch",
+      "rule": "A destination field that matcher.matchField cannot resolve is dropped into Unmapped and rendered as a `// TODO: unmapped field` comment unless -strict is passed — the generated struct literal then simply omits that field and it silently takes its zero value at runtime, with no compiler error. Any change to buildSourceIndices, matchField's tag/name lookup order, or the map:\"...\" tag key handling must be checked against this failure mode: a typo'd map:\"...\" tag value, a renamed source or destination field, or a case mismatch (without -ci) must fall through to Unmapped/TODO, never to a mismatched or partially-matched field silently pointing at the wrong source data.",
+      "scope": ["internal/matcher/*.go"],
+      "severity": "high"
+    },
+    {
+      "id": "numeric-conversion-narrowing",
+      "rule": "makeMapping and tryDerefMapping/tryAddrOfMapping accept any pair for which types.ConvertibleTo returns true, and the templates render that as a bare Go conversion (ConvType(src.Field)) with no range check. types.ConvertibleTo is satisfied by narrowing numeric conversions (int64→int32, uint→int8, float64→float32) just as readily as widening ones, and Go's explicit conversion silently truncates or wraps on overflow rather than erroring. Flag any new code path that treats ConvertibleTo as sufficient justification for a numeric conversion without at least distinguishing narrowing from widening (e.g. via reflect-free bit-size comparison on the underlying types.Basic kinds).",
+      "scope": ["internal/matcher/matcher.go"],
+      "severity": "medium"
+    },
+    {
+      "id": "generation-determinism",
+      "rule": "Generated output is committed to the caller's repository and re-run by `go generate`; two runs over unchanged input must byte-for-byte match. Destination fields are matched in struct-declaration order (deterministic), but processNestedDstTags ranges over idx.byTag (a map) to build NestedDstAssignments, and the -ci fallback in matchField ranges over srcByName (a map) to find a case-insensitive match — both iterate in Go's randomized map order. Any new or modified code path that decides emitted-code order, or picks among multiple ambiguous candidate matches, by ranging over a map must sort its keys (or iterate a slice) first, or it will produce a different generated file on every run for the same input.",
+      "scope": ["internal/matcher/matcher.go"],
+      "severity": "medium"
+    },
+    {
+      "id": "generation-mode-parity",
+      "rule": "registryFieldExpr, pureFieldExpr, and sliceLoopExpr in internal/generator/templates.go each independently re-implement the same decision tree over FieldMapping's flags (Deref, AddrOf, IsStructMap, IsSliceMap, NeedsConv, and their combinations) for register mode, func mode, and slice element handling respectively. A new FieldMapping flag or flag combination added in internal/matcher/matcher.go must be threaded through every template variant that switches on those flags — func mode, register mode, both mode, and the slice-loop precompute block — with matching behavior. Do not approve a matcher.go change that adds a new mapping shape without a corresponding update (or an explicit note that it's a no-op) in all three template functions.",
+      "scope": ["internal/generator/templates.go", "internal/matcher/matcher.go"],
+      "severity": "high"
+    },
+    {
+      "id": "nil-safe-flag-coverage",
+      "rule": "The -nil-safe flag's entire contract is 'no generated pointer dereference panics on nil.' nilSafeBlock guards field-level Deref (plain and struct-nested), and sliceLoopExpr's SliceElemDeref branches carry their own `if _v != nil` guards. Every code path in matcher.go that sets Deref=true (tryDerefMapping) or SliceElemDeref=true (trySliceMapping) at the field or slice-element level must have a matching nil-guarded branch in nilSafeBlock or sliceLoopExpr when NilSafe is set — a new dereferencing path that isn't wired into the nil-safe templates will compile fine but silently defeats -nil-safe for that one field, which is worse than the flag not existing because it creates false confidence.",
+      "scope": ["internal/generator/templates.go", "internal/matcher/matcher.go"],
+      "severity": "high"
+    },
+    {
+      "id": "nested-struct-pair-completeness",
+      "rule": "tryStructMapping / tryDerefMapping / tryAddrOfMapping / trySliceMapping mark a field IsStructMap and emit a call to Map<Src>To<Dst> (func mode) or mapper.Map[Dst](...) (register mode) purely from type shape, without checking whether that Src:Dst pair was actually requested via -pairs/-src+-dst (and bidirectional expansion) in main.go. If the nested pair isn't in the type-pairs list, gomapper still succeeds and writes a file that calls an undefined function, and the failure only surfaces later as a `go build` error with no mention of gomapper or the missing pair. A change to how type pairs are collected or expanded in main.go (parseTypePairs, expandBidirectional) should preserve or improve this check, not silently make it easier to omit a required pair.",
+      "scope": ["main.go", "internal/matcher/matcher.go"],
+      "severity": "medium"
+    },
+    {
+      "id": "golden-file-sync",
+      "rule": "testdata/basic/expected_gen.go.golden and testdata/advanced/expected_gen.go.golden are compared byte-for-byte (after stripping the //go:build ignore line) against fresh gomapper output in integration_test.go — embedded has no golden file and is instead checked via targeted strings.Contains assertions in TestIntegration_EmbeddedStructPromotedFields. A change to internal/matcher or internal/generator that alters generated output for basic or advanced testdata must update the corresponding .golden file in the same change, not leave it to fail CI; a change affecting embedded's output must update the assertions in TestIntegration_EmbeddedStructPromotedFields instead. New golden output must still be valid, compilable Go reflecting the intended fix, not just whatever the tool now happens to emit.",
+      "scope": ["internal/matcher/*.go", "internal/generator/*.go", "testdata/**/expected_gen.go.golden", "integration_test.go"],
+      "severity": "low"
+    },
+    {
+      "id": "unexported-and-embedded-field-symmetry",
+      "rule": "buildSourceIndices and matchDstFields both skip a field when !f.Exported or f.Embedded is true, so only promoted (flattened) fields from an embedded struct participate in matching — never the embedded field itself as a unit, and never an unexported field on either side. Any refactor of these two skip conditions must keep them symmetric between source and destination; a version that skips unexported/embedded fields on one side but not the other would let an unassignable or unintended field slip into a mapping.",
+      "scope": ["internal/matcher/matcher.go", "internal/loader/loader.go"],
+      "severity": "low"
+    }
+  ]
+}

.greptile/files.json

@@ -0,0 +1,26 @@
+{
+  "files": [
+    {
+      "path": "README.md",
+      "description": "User-facing flag reference, field-matching priority order, and generation-mode descriptions — the contract the code must actually implement."
+    },
+    {
+      "path": "doc.go",
+      "description": "Package-level (command-level) doc comment restating install, usage, flags, field matching, and nil-safe mode; should stay consistent with README.md."
+    },
+    {
+      "path": "internal/matcher/matcher.go",
+      "description": "The field-matching engine: tag/name resolution order, type compatibility checks (assignable/convertible/deref/addr-of/slice/nested-struct), and where a field becomes 'unmapped'. Central to nearly every rule in config.json.",
+      "scope": ["internal/**/*.go", "main.go"]
+    },
+    {
+      "path": "internal/generator/templates.go",
+      "description": "The three parallel template decision trees (func mode, register mode, slice loop) and the nil-safe/addr-of precompute blocks that must stay in sync with matcher.go's FieldMapping flags.",
+      "scope": ["internal/generator/**/*.go", "internal/matcher/**/*.go"]
+    },
+    {
+      "path": ".golangci.yml",
+      "description": "The linter ruleset already enforced in CI (note revive's 'exported' doc-comment check is explicitly disabled here) — avoid duplicating what golangci-lint already flags."
+    }
+  ]
+}

.greptile/rules.md

@@ -0,0 +1,120 @@
+# Style and pattern rationale
+
+Context for the scoped rules in `config.json`. This file is freeform prose read
+alongside the diff; `config.json` is what actually gates comment scope and
+severity.
+
+gomapper has no bug-fix history to draw worked examples from yet — the git log
+is a single `0.0.1` initial release plus dependabot bumps, and `CHANGELOG.md`
+only lists features added, not bugs fixed. The examples below are instead
+mechanisms verified directly against the current code during this review setup
+(March 2026 codebase): each one is a real gap in the code as it stands today,
+not a historical incident. Treat them as "here's exactly how this would break"
+rather than "here's what broke before."
+
+## Silent unmapped fields — the core failure mode
+
+`matcher.matchField` (`internal/matcher/matcher.go`) tries, in order: the
+destination's own `map:"..."` tag, a source field whose tag targets this
+destination name, an exact name match, then (with `-ci`) a case-insensitive
+name match. If none hit, `matchDstFields` records an `UnmappedField` and the
+templates render `// TODO: unmapped field Name (Type)` — the destination
+struct literal simply has no line for that field, so it silently gets its zero
+value. This only becomes a hard error with `-strict`.
+
+That means a typo in a `map:"..."` tag, or a field rename on either side that
+isn't mirrored on the other, doesn't produce a compile error or a wrong-value
+bug you'd catch in a diff — it produces a field that's just missing from the
+generated code, indistinguishable at a glance from an intentionally-skipped
+field. This is the single most consequential correctness property of the
+whole tool, which is why `silent-unmapped-field-mismatch` is the first rule.
+
+## Numeric conversions have no narrowing check
+
+`makeMapping` falls through to `types.ConvertibleTo(src.Type, dst.Type)` for
+any pair that isn't directly assignable, and the templates emit that as
+`ConvType(src.Field)` — a bare Go type conversion. `types.ConvertibleTo`
+returns true for `int64→int32`, `uint64→uint8`, `float64→float32`, and
+`int→uint` just as readily as for safe widening conversions like `int→int64`
+(the one actually used in `examples/basic/types.go`, `Age int` → `Age int64`).
+Go's own explicit conversion silently truncates or wraps on overflow for the
+narrowing direction — there's no generated bounds check, and no signal in the
+output that a given field is narrowing rather than widening. A caller reading
+`mapper_gen.go` sees the same one-line conversion either way.
+
+## Two maps whose iteration order leaks into generated output
+
+`processNestedDstTags` iterates `idx.byTag` — built as a `map[string]loader.StructField`
+in `buildSourceIndices` — to build `NestedDstAssignments`, which the templates
+then range over in the order they arrive. `matchField`'s `-ci` fallback
+similarly ranges over `srcByName` to find a case-insensitive candidate. Go
+map iteration order is randomized per process. Right now this only bites when
+there's more than one dot-notation `map:"Parent.Child"` tag in a single
+struct pair, or more than one source field that case-insensitively matches a
+destination field — both narrow cases today — but the two loops sit exactly
+on the boundary of "generated file must be byte-identical across reruns,"
+which is the property `go generate` idempotency and any golden-file/CI diff
+check depends on. Widening either matching feature without sorting the map
+keys first would turn a narrow edge case into a common one.
+
+## Three template modes, one decision tree, three copies of it
+
+`internal/generator/templates.go` defines `registryFieldExpr` (register mode),
+`pureFieldExpr` (func mode), and `sliceLoopExpr` (slice element rendering,
+shared by both) as three separate template strings, each re-implementing the
+same branch order over `Deref` / `AddrOf` / `IsStructMap` / `IsSliceMap` /
+`NeedsConv`. `-mode both` runs both `pureFieldExpr` and the register path in
+the same file. There is no single source of truth for "what does this
+FieldMapping shape render as" — it's whatever each template's `if/else if`
+chain says, independently. A `matcher.go` change that introduces a new flag
+combination (or changes what an existing combination means) has to be carried
+into all three by hand; the integration tests build golden output for `func`
+mode's `both`/`register` variants but a mismatch that still *compiles* (e.g.
+register mode silently falling through to a subtly different but valid
+expression) would not necessarily fail `go build` the way a missing case
+would.
+
+## `-nil-safe` only protects the paths that were wired up
+
+The flag's promise is unconditional: no generated dereference panics on nil.
+In practice that promise is implemented as two separate opt-in blocks —
+`nilSafeBlock` for field-level `Deref`, and the `if _v != nil` branches inside
+`sliceLoopExpr` for `SliceElemDeref`. Both are template-level, keyed off flags
+set in `matcher.go`. If a future change adds a new way for `Deref` (or a
+slice-element equivalent) to become true — say, a new pointer-unwrapping rule
+in `tryDerefMapping` — and the corresponding nil-guard branch isn't added to
+`nilSafeBlock`/`sliceLoopExpr`, the generated code still compiles, `-nil-safe`
+still runs without error, and the one new field just isn't nil-checked. That's
+strictly worse than not having the flag, because the caller has explicitly
+asked for the safety property and gomapper reported success.
+
+## A missing nested pair fails downstream, not in gomapper
+
+`tryStructMapping` and its slice/pointer variants decide `IsStructMap` purely
+from Go type shape (`areDifferentNamedStructs`) — they never check that the
+nested `Src:Dst` pair is actually among the pairs `main.go` is generating
+functions for. `examples/advanced` deliberately exercises this correctly
+(`-pairs Address:AddressDTO,Order:OrderDTO` includes both), but nothing stops
+someone from running `-pairs Order:OrderDTO` alone: gomapper would still
+"succeed," write a file calling `MapAddressToAddressDTO`, and the first error
+the user sees is a `go build` failure for an undefined function, with no
+indication that the fix is "add Address:AddressDTO to -pairs."
+
+## Golden files are the regression net for most of the above
+
+`integration_test.go` byte-compares fresh output against
+`testdata/{basic,advanced}/expected_gen.go.golden` (stripping the
+`//go:build ignore` line). Unmapped-field rendering, conversion expressions,
+nil-safe blocks, and slice loops are exercised through these fixtures, not
+through unit assertions on template strings. A PR that changes generated
+output for `basic` or `advanced` without touching the matching `.golden` file
+is either untested or (more likely) will fail CI; a PR that updates the
+`.golden` file without a corresponding source change, or that "fixes" a
+golden file to match a bug rather than fixing the bug, defeats the point of
+the fixture.
+
+`embedded` has no golden file — `TestIntegration_EmbeddedStructPromotedFields`
+checks the promoted-field mapping via targeted `strings.Contains` assertions
+instead. A change affecting embedded-struct output needs those assertions
+kept in sync, the same way `basic`/`advanced` need their `.golden` files kept
+in sync.