Skip to content

Chore/greptile json codec rule #2667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mromaszewicz merged 2 commits into from
Nov 11, 2026
Merged

Chore/greptile json codec rule #2667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd8f25d
chore: fix malformed Greptile config and duplicate rule number
mromaszewicz Nov 11, 2026
af7a932
chore: add a Greptile rule that generated JSON uses the runtime codec
mromaszewicz Nov 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .greptile/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
{
{
"strictness": 2,
"commentTypes": ["logic", "syntax"],
Expand Down
24 changes: 23 additions & 1 deletion .greptile/rules.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ When a PR adds a test, enforce the following:
- The repo is a multi-module monorepo. Cross-module changes (e.g., to `runtime/` consumers) deserve extra scrutiny.
- Generated files are committed; CI fails if `make generate` produces a diff. If a PR's generated files look stale, that will fail CI regardless — but flagging it in review saves a round-trip.

## 5. Schema-merging versions must not regress
## 8. Schema-merging versions must not regress

`compatibility.schema-merging-behavior` selects how `allOf`, `anyOf` and `oneOf` schemas become Go types, in sequential versions (`v1`, `v2`). These versions still receive bug fixes, but they must never regress: a spec that generated working code under a version must keep generating code that compiles against users' existing code and behaves the same way. New behavior goes into a new version, with code of its own.

Expand All @@ -113,3 +113,25 @@ The tests that guard these versions are:
`v3` is the version under development. Its code (`pkg/codegen/merge_schemas_v3.go`, `pkg/codegen/union_v3.go`) and tests (`internal/test/aggregates/matrix/v3/`, `pkg/codegen/*_v3_test.go`) are expected to change as it takes shape; this rule doesn't apply to them.

The code of these versions is `pkg/codegen/merge_schemas_v1.go`, `pkg/codegen/merge_schemas_v2.go` and `pkg/codegen/union_v2.go`, but it also calls code every version shares, such as `generateGoSchema`, `detectEnumViaOneOf`, `schemaIsNullable` and `nonNullTypes` in `pkg/codegen`, and the union and additional-properties templates. A change there can alter these versions' output too, and the tests above are what catch it.

## 9. Generated JSON goes through the runtime codec

Generated code encodes and decodes JSON through the process-wide codec in `github.com/oapi-codegen/runtime` (v1.9.0 or later), so that one `runtime.SetJSONCodec` call applies to all of it: models, clients and servers. A single direct call to `encoding/json` in generated code silently escapes the codec, and no test will notice, because the default codec is `encoding/json`.
Comment thread
mromaszewicz marked this conversation as resolved.
Resolved

Flag any change to a template under `pkg/codegen/templates/`, or to Go code in `pkg/codegen/` that emits generated source (such as `template_helpers.go`), that adds one of these to the generated output:

- `json.Marshal(` or `json.MarshalIndent(`: use `runtime.JSONMarshal` instead.
- `json.Unmarshal(`: use `runtime.JSONUnmarshal` instead.
- `json.NewEncoder(...).Encode(`: use `runtime.JSONMarshalWrite(w, v)` instead.
- `json.NewDecoder(...).Decode(`: use `runtime.JSONUnmarshalRead(r, v)` instead, or `runtime.BindJSONBody` for a request body.

When spot checking `*.gen.go` files, a new `json.Marshal`, `json.Unmarshal`, `json.NewEncoder` or `json.NewDecoder` call is the same finding: trace it back to the template that emitted it.

The strict response in `strict/strict-interface.tmpl` encodes into a buffer with `runtime.JSONMarshalWrite` before it calls `WriteHeader`, so that an encoding error still becomes a 500. Flag a change that writes the body to the `http.ResponseWriter` directly, or that moves the encode after `WriteHeader`.

These are not violations:

- `encoding/json` types and helpers that don't encode or decode an application value: `json.RawMessage`, `json.Number`, `json.Valid`, and the `json.Marshaler` and `json.Unmarshaler` interfaces.
- The `json.NewDecoder(...).UseNumber()` probe in the union `UnmarshalText` in `union.tmpl`. It classifies a text value and depends on `UseNumber`.
- JSON that a web framework handles: request body binding in the echo, gin, fiber and iris strict servers (`ctx.Bind`, `ShouldBindJSON`, `BodyParser`, `Bind().Body`, `ReadJSON`), and `ctx.JSON` responses. These are deliberately outside the codec's reach.
- The generator's own use of `encoding/json` while it runs (for example in `pkg/codegen/merge_schemas.go` and `pkg/codegen/schema.go`), hand-written tests, and hand-written handler code in `examples/` and the docs.
Loading