Skip to content

Commit 280ae48

Browse files
fredbiclaude
fredbi
and
claude
committed
docs: correct the agent instruction files
CLAUDE.md, .github/copilot-instructions.md and two files under .claude/rules described a repository that no longer exists, in ways that misdirect an agent reading them. Every claim below was checked against the code. Both instruction files said "single package". There are two: jsonpointer and jsonpointer/jsonname. They listed jsonname as the external dependency github.com/go-openapi/swag/jsonname; it is a subpackage of this repository, and go.mod has no non-test dependency. Their file tables omitted ifaces.go, options.go and both jsonname files, and their Get and Set signatures omitted opts ...Option. copilot-instructions.md claimed the RFC 6901 "-" array suffix is not implemented. Pointer.Set has supported it as an append since the dash token work; CLAUDE.md already said so. Both claimed "anonymous embedded struct fields are traversed only if tagged", which is backwards: an anonymous field is always walked for the fields it promotes, and its own json tag never becomes a reachable key. Both also stated the json-tag requirement unconditionally, when it only holds for the default NameProvider -- GoNameProvider reaches untagged exported fields. Neither file mentioned that two providers exist, which is the first thing to check when a field is unreachable. Dangling links are removed: docs/ holds only .gitkeep, so docs/MAINTAINERS.md and docs/STYLE.md do not exist, and neither do .claude/plans/ROADMAP.md or .github/CONTRIBUTING.md. contributions.md also told agents to run `go test work ./...`, which is not a command. Both files gain the behavior settled by the previous commit: embedded-pointer traversal, nil embedded pointers reporting an error, the string-keyed-map requirement, and null handling on Set. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Signed-off-by: Frédéric BIDON <[email protected]>
1 parent c71171e commit 280ae48

4 files changed

Lines changed: 73 additions & 17 deletions

File tree

‎.claude/CLAUDE.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,6 @@ similarly named `github.com/go-openapi/swag/jsonname`.
7878

7979
See `.claude/rules/` for contribution, linting, go and technical-writing conventions.
8080
Format with `golangci-lint fmt`, and check a change with `golangci-lint run --new-from-rev master`.
81+
82+
Tests run with `-race`. CI runs `{ubuntu, macos, windows} x {stable, oldstable}`, via the shared
83+
`go-openapi/ci-workflows/.github/workflows/go-test.yml`.

‎.claude/rules/contributions.md‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ paths:
55

66
# Contribution rules (go-openapi)
77

8-
Read `.github/CONTRIBUTING.md` before opening a pull request.
8+
Read the go-openapi contribution guide before opening a pull request, plus the sibling rules in
9+
this directory: go conventions, linting, testing, github workflows and technical writing.
910

1011
## Commit hygiene
1112

@@ -41,12 +42,15 @@ go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest
4142
- Aim for at least 80% coverage of your patch.
4243
- Run the full test suite before submitting:
4344

44-
For mono-repos:
4545
```sh
46-
go test work ./...
46+
go test ./...
4747
```
4848

49-
For single module repos:
49+
In a mono-repo, run it once per module (a Go workspace does not give `go test` a whole-workspace
50+
target).
51+
52+
Run the race detector too, since CI does:
53+
5054
```sh
51-
go test ./...
55+
go test -race ./...
5256
```

‎.claude/rules/linting.md‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,17 @@ golangci-lint run
1010
```
1111

1212
Config: `.golangci.yml` — posture is `default: all` with explicit disables.
13-
See `docs/STYLE.md` for the rationale behind each disabled linter.
13+
Each disable is listed there under `linters.disable`.
14+
15+
Format with `golangci-lint fmt` (not `gofmt` or `gofumpt` directly — the config drives which
16+
formatters run).
17+
18+
Check a change against the base branch before pushing, so pre-existing reports in untouched code
19+
do not drown out your own:
20+
21+
```sh
22+
golangci-lint run --new-from-rev master
23+
```
1424

1525
Key rules:
1626
- Every `//nolint` directive **must** have an inline comment explaining why.

‎.github/copilot-instructions.md‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,42 +6,81 @@ Go implementation of [JSON Pointer (RFC 6901)](https://datatracker.ietf.org/doc/
66
and mutating JSON documents represented as Go values. Works with `map[string]any`, slices, and Go structs
77
(resolved via `json` struct tags and reflection).
88

9-
## Package Layout (single package)
9+
## Package Layout
10+
11+
Two packages in one module (`github.com/go-openapi/jsonpointer`):
1012

1113
| File | Contents |
1214
|------|----------|
13-
| `pointer.go` | Core types (`Pointer`, `JSONPointable`, `JSONSetable`), `New`, `Get`, `Set`, `Offset`, `Escape`/`Unescape` |
14-
| `errors.go` | Sentinel errors: `ErrPointer`, `ErrInvalidStart`, `ErrUnsupportedValueType` |
15+
| `pointer.go` | `Pointer`, `New`, `Get`, `Set`, `Offset`, `GetForToken`, `SetForToken`, `Escape`/`Unescape`, and the reflection helpers |
16+
| `ifaces.go` | `JSONPointable`, `JSONSetable`, `NameProvider` interfaces |
17+
| `options.go` | `Option`, `WithNameProvider`, `SetDefaultNameProvider`, `UseGoNameProvider`, `DefaultNameProvider` |
18+
| `errors.go` | Sentinel errors and their constructors |
19+
| `jsonname/name_provider.go` | `NameProvider` — the default, tag-only resolver (`DefaultJSONNameProvider`) |
20+
| `jsonname/go_name_provider.go` | `GoNameProvider` — the alternative resolver, follows `encoding/json` rules |
1521

1622
## Key API
1723

1824
- `New(string) (Pointer, error)` — parse a JSON pointer string (e.g. `"/foo/0/bar"`)
19-
- `Pointer.Get(document any) (any, reflect.Kind, error)` — retrieve a value
20-
- `Pointer.Set(document, value any) (any, error)` — set a value (document must be pointer/map/slice)
25+
- `Pointer.Get(document any, opts ...Option) (any, reflect.Kind, error)` — retrieve a value
26+
- `Pointer.Set(document, value any, opts ...Option) (any, error)` — set a value (document must be a pointer, map or slice)
2127
- `Pointer.Offset(jsonString string) (int64, error)` — byte offset of token in raw JSON
2228
- `GetForToken` / `SetForToken` — single-level convenience helpers
2329
- `Escape` / `Unescape` — RFC 6901 token escaping (`~0` ↔ `~`, `~1` ↔ `/`)
2430

2531
Custom types can implement `JSONPointable` (for Get) or `JSONSetable` (for Set) to bypass reflection.
2632

33+
Sentinel errors: `ErrPointer`, `ErrInvalidStart`, `ErrUnsupportedValueType`, `ErrDashToken`.
34+
Errors raised by the package wrap `ErrPointer`. Errors returned by a caller's `JSONLookup` or
35+
`JSONSet` are passed through unchanged, so `errors.Is(err, ErrPointer)` is false for those.
36+
37+
## Name providers
38+
39+
Two implementations of the `NameProvider` interface resolve go field names to json names. The
40+
difference drives most "why is my field unreachable" questions:
41+
42+
- `jsonname.NameProvider` (the default, `jsonname.DefaultJSONNameProvider`): only fields carrying a
43+
non-empty `json` tag are reachable.
44+
- `jsonname.GoNameProvider` (opt in with `UseGoNameProvider()` or `WithNameProvider`): follows
45+
`encoding/json`, so untagged exported fields are reachable under their go name.
46+
2747
## Design Decisions
2848

29-
- Struct fields **must** have a `json` tag to be reachable; untagged fields are ignored.
30-
- Anonymous embedded struct fields are traversed only if tagged.
31-
- The RFC 6901 `"-"` array suffix (append) is **not** implemented.
49+
- Under the default provider, struct fields **must** have a `json` tag to be reachable; untagged
50+
fields are ignored. Use `UseGoNameProvider()` for `encoding/json` semantics.
51+
- An anonymous (embedded) field is walked for the fields it promotes, and its own `json` tag is
52+
ignored — tagged or not, the tag never becomes a reachable key.
53+
- Embedding is traversed through a pointer (`*Base`). Resolving a promoted field on a value whose
54+
embedded pointer is nil reports an error; it does not allocate the intermediate struct.
55+
- An anonymous field that is not a struct (e.g. an embedded named slice) promotes nothing and is
56+
unreachable.
57+
- Maps must be keyed by a string type. A named string type is converted; `map[int]V` and friends
58+
cannot be addressed by a pointer and report an error.
59+
- `Set` with a nil value (what `encoding/json` decodes a JSON null into) writes the zero value when
60+
the target can hold nil — interface, pointer, map, slice, chan, func. On a target that cannot
61+
represent null (a `string` field, an element of `[]int`) it reports an error rather than
62+
substituting a zero value. Setting a map member to nil keeps the member; it does not delete it.
63+
- The RFC 6901 `"-"` array suffix **is** supported on `Pointer.Set` as an append operation
64+
(RFC 6902 convention). On `Pointer.Get` and `Pointer.Offset` it is always an error per
65+
RFC 6901 §4. See `ErrDashToken`.
3266

3367
## Dependencies
3468

35-
- `github.com/go-openapi/swag/jsonname` — struct tag to JSON field name resolution
3669
- `github.com/go-openapi/testify/v2` — test-only assertions (zero-dep fork of `stretchr/testify`)
3770

71+
The module has no non-test dependency. `jsonname` is a subpackage of this repository, not the
72+
similarly named `github.com/go-openapi/swag/jsonname`.
73+
3874
## Conventions
3975

4076
- All `.go` files must have SPDX license headers (Apache-2.0).
4177
- Commits require DCO sign-off (`git commit -s`).
42-
- Linting: `golangci-lint run` — config in `.golangci.yml` (posture: `default: all` with explicit disables).
78+
- Linting: `golangci-lint run` — config in `.golangci.yml` (posture: `default: all` with explicit
79+
disables). Format with `golangci-lint fmt`, and check a change with
80+
`golangci-lint run --new-from-rev master`.
4381
- Every `//nolint` directive **must** have an inline comment explaining why.
44-
- Tests: `go test ./...` with `-race`. CI runs on `{ubuntu, macos, windows} x {stable, oldstable}`.
82+
- Tests: `go test ./...` with `-race`. CI runs `{ubuntu, macos, windows} x {stable, oldstable}`,
83+
via the shared `go-openapi/ci-workflows/.github/workflows/go-test.yml`.
4584
- Test framework: `github.com/go-openapi/testify/v2` (not `stretchr/testify`).
4685

4786
See `.github/copilot/` (symlinked to `.claude/rules/`) for detailed rules on Go conventions, linting, testing, and contributions.

0 commit comments

Comments
 (0)