diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 7dea424..012909b 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -1,72 +1,195 @@ ## Contribution Guidelines +You'll find below general guidelines, which mostly correspond to standard practices for open sourced repositories. + +>**TL;DR** +> +> If you're already an experience go developer on github, then you should just feel at home with us +> and you may well skip the rest of this document. +> +> You'll essentially find the usual guideline for a go library project on github. + +These guidelines are general to all libraries published on github by the `go-openapi` organization. + +You'll find more detailed (or repo-specific) instructions in the [maintainer's docs](../docs). + +## Questions & Issues + +### Asking questions + +You may inquire about anything about this library by reporting a "Question" issue on github. + +### Reporting issues + +Reporting a problem with our libraries _is_ a valuable contribution. + +You can do this on the github issues page of this repository. + +Please be as specific as possible when describing your issue. + +Whenever relevant, please provide information about your environment (go version, OS). + +Adding a code snippet to reproduce the issue is great, and as a big time saver for maintainers. + +### Triaging issues + +You can help triage issues which may include: + +* reproducing bug reports +* asking for important information, such as version numbers or reproduction instructions +* answering questions and sharing your insight in issue comments + +## Code contributions + ### Pull requests are always welcome -We are always thrilled to receive pull requests, and do our best to -process them as fast as possible. Not sure if that typo is worth a pull -request? Do it! We will appreciate it. +We are always thrilled to receive pull requests, and we do our best to +process them as fast as possible. + +Not sure if that typo is worth a pull request? Do it! We will appreciate it. + +If your pull request is not accepted on the first try, don't be discouraged! +If there's a problem with the implementation, hopefully you received feedback on what to improve. + +If you have a lot of ideas or a lot of issues to solve, try to refrain a bit and post focused +pull requests. +Think that they must be reviewed by a maintainer and it is easy to lost track of things on big PRs. + +We're trying very hard to keep the go-openapi packages lean and focused. +These packages constitute a toolkit: it won't do everything for everybody out of the box, +but everybody can use it to do just about everything related to OpenAPI. + +This means that we might decide against incorporating a new feature. + +However, there might be a way to implement that feature *on top of* our libraries. + +### Environment -If your pull request is not accepted on the first try, don't be -discouraged! If there's a problem with the implementation, hopefully you -received feedback on what to improve. +You just need a `go` compiler to be installed. No special tools are needed to work with our libraries. -We're trying very hard to keep go-swagger lean and focused. We don't want it -to do everything for everybody. This means that we might decide against -incorporating a new feature. However, there might be a way to implement -that feature *on top of* go-swagger. +The go compiler version required is always the old stable (latest minor go version - 1). +If you're already used to work with `go` you should already have everything in place. + +Although not required, you'll be certainly more productive with a local installation of `golangci-lint`, +the meta-linter our CI uses. + +If you don't have it, you may install it like so: + +```sh +go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest +``` ### Conventions -Fork the repo and make changes on your fork in a feature branch: +#### Git flow + +Fork the repo and make changes to your fork in a feature branch. + +To submit a pull request, push your branch to your fork (e.g. `upstream` remote): +github will propose to open a pull request on the original repository. + +Typically you'd follow some common naming conventions: -- If it's a bugfix branch, name it XXX-something where XXX is the number of the - issue -- If it's a feature branch, create an enhancement issue to announce your - intentions, and name it XXX-something where XXX is the number of the issue. +- if it's a bugfix branch, name it `fix/XXX-something`where XXX is the number of the + issue on github +- if it's a feature branch, create an enhancement issue to announce your + intentions, and name it `feature/XXX-something` where XXX is the number of the issue. -Submit unit tests for your changes. Go has a great test framework built in; use -it! Take a look at existing tests for inspiration. Run the full test suite on -your branch before submitting a pull request. +> NOTE: we don't enforce naming conventions on branches: it's your fork after all. -Update the documentation when creating or modifying features. Test -your documentation changes for clarity, concision, and correctness, as -well as a clean documentation build. See ``docs/README.md`` for more -information on building the docs and how docs get released. +#### Tests -Write clean code. Universally formatted code promotes ease of writing, reading, -and maintenance. Always run `gofmt -s -w file.go` on each changed file before -committing your changes. Most editors have plugins that do this automatically. +Submit unit tests for your changes. + +Go has a great built-in test framework ; use it! + +Take a look at existing tests for inspiration, and run the full test suite on your branch +before submitting a pull request. + +Our CI measures test coverage and the test coverage of every patch. +Although not a blocking step - because there are so many special cases - +this is an indicator that maintainers consider when approving a PR. + +Please try your best to cover about 80% of your patch. + +#### Code style + +You may read our stance on code style [there](../docs/STYLE.md). + +#### Documentation + +Don't forget to update the documentation when creating or modifying features. + +Most documentation for this library is directly found in code as comments for godoc. + +The documentation for the go-openapi packages is published on the public go docs site: + + + +Check your documentation changes for clarity, concision, and correctness. + +If you want to assess the rendering of your changes when published to `pkg.go.dev`, you may +want to install the `pkgsite` tool proposed by `golang.org`. + +```sh +go install golang.org/x/pkgsite/cmd/pkgsite@latest +``` + +Then run on the repository folder: +```sh +pkgsite . +``` + +This wil run a godoc server locally where you may see the documentation generated from your local repository. + +#### Commit messages Pull requests descriptions should be as clear as possible and include a reference to all the issues that they address. Pull requests must not contain commits from other users or branches. -Commit messages must start with a capitalized and short summary (max. 50 -chars) written in the imperative, followed by an optional, more detailed -explanatory text which is separated from the summary by an empty line. +Commit messages are not required to follow the "conventional commit" rule, but it's certainly a good +thing to follow this guidelinea (e.g. "fix: blah blah", "ci: did this", "feat: did that" ...). + +The title in your commit message is used directly to produce our release notes: try to keep them neat. + +The commit message body should detail your changes. + +If an issue should be closed by a commit, please add this reference in the commit body: + +``` +* fixes #{issue number} +``` + +#### Code review + +Code review comments may be added to your pull request. + +Discuss, then make the suggested modifications and push additional commits to your feature branch. + +Be sure to post a comment after pushing. The new commits will show up in the pull +request automatically, but the reviewers will not be notified unless you comment. + +Before the pull request is merged, +**make sure that you squash your commits into logical units of work** +using `git rebase -i` and `git push -f`. + +After every commit the test suite should be passing. -Code review comments may be added to your pull request. Discuss, then make the -suggested modifications and push additional commits to your feature branch. Be -sure to post a comment after pushing. The new commits will show up in the pull -request automatically, but the reviewers will not be notified unless you -comment. +Include documentation changes in the same commit so that a revert would remove all traces of the feature or fix. -Before the pull request is merged, make sure that you squash your commits into -logical units of work using `git rebase -i` and `git push -f`. After every -commit the test suite should be passing. Include documentation changes in the -same commit so that a revert would remove all traces of the feature or fix. +#### Sign your work -Commits that fix or close an issue should include a reference like `Closes #XXX` -or `Fixes #XXX`, which will automatically close the issue when merged. +The sign-off is a simple line at the end of your commit message, +which certifies that you wrote it or otherwise have the right to +pass it on as an open-source patch. -### Sign your work +We require the simple DCO below with an email signing your commit. +PGP-signed commit are greatly appreciated but not required. -The sign-off is a simple line at the end of the explanation for the -patch, which certifies that you wrote it or otherwise have the right to -pass it on as an open-source patch. The rules are pretty simple: if you -can certify the below (from +The rules are pretty simple: if you can certify the below (from [developercertificate.org](http://developercertificate.org/)): ``` diff --git a/README.md b/README.md index 24889f4..28ac701 100644 --- a/README.md +++ b/README.md @@ -3,14 +3,13 @@ [![Tests][test-badge]][test-url] [![Coverage][cov-badge]][cov-url] [![CI vuln scan][vuln-scan-badge]][vuln-scan-url] [![CodeQL][codeql-badge]][codeql-url] -[![Release][release-badge]][release-url] -[![Go Report Card][gocard-badge]][gocard-url] [![CodeFactor Grade][codefactor-badge]][codefactor-url] -[![License][license-badge]][license-url] +[![Release][release-badge]][release-url] [![Go Report Card][gocard-badge]][gocard-url] [![CodeFactor Grade][codefactor-badge]][codefactor-url] [![License][license-badge]][license-url] -[![GoDoc][godoc-badge]][godoc-url] [![Slack Channel][slack-badge]][slack-url] [![go version][goversion-badge]][goversion-url] ![Top language][top-badge] ![Commits since latest release][commits-badge] + +[![GoDoc][godoc-badge]][godoc-url] [![go version][goversion-badge]][goversion-url] ![Top language][top-badge] ![Commits since latest release][commits-badge] --- @@ -28,13 +27,18 @@ go get github.com/go-openapi/jsonpointer ## Basic usage -See [examples](./examples_test.go) +See also some [examples](./examples_test.go) + +### Retrieving a value ```go import ( "github.com/go-openapi/jsonpointer" ) + + var doc any + ... pointer, err := jsonpointer.New("/foo/1") @@ -50,6 +54,23 @@ See [examples](./examples_test.go) ... ``` +### Setting a value + +```go + ... + var doc any + ... + pointer, err := jsonpointer.New("/foo/1") + if err != nil { + ... // error: e.g. invalid JSON pointer specification + } + + doc, err = p.Set(doc, "value") + if err != nil { + ... // error: e.g. key not found, index out of bounds, etc. + } +``` + ## Change log See @@ -96,7 +117,7 @@ using the special trailing character "-" is not implemented. [doc-badge]: https://img.shields.io/badge/doc-site-blue?link=https%3A%2F%2Fgoswagger.io%2Fgo-openapi%2F [doc-url]: https://goswagger.io/go-openapi -[godoc-badge]: https://pkg.go.dev/github.com/go-openapi/jsonpointer?status.svg +[godoc-badge]: https://pkg.go.dev/badge/github.com/go-openapi/jsonpointer [godoc-url]: http://pkg.go.dev/github.com/go-openapi/jsonpointer [slack-badge]: https://slackin.goswagger.io/badge.svg [slack-url]: https://slackin.goswagger.io diff --git a/docs/MAINTAINERS.md b/docs/MAINTAINERS.md new file mode 100644 index 0000000..9f36c68 --- /dev/null +++ b/docs/MAINTAINERS.md @@ -0,0 +1,44 @@ +# Maintainer's guide + +**DRAFT** + +(to be completed) + +## Repo configuration + +* branch protection +* required PR checks +* auto-merge feature + +## Continuous Integration + +### Code Quality checks + +* meta-linter: golangci-lint +* linter config + +* Code quality assessment: CodeFactor +* Code quality badges + * go report card + * CodeFactor + +### Testing + +* test reports +* test coverage reports + +### Automated updates + +* dependabot + +* go version udpates + +### Vulnerability scanners + +* github CodeQL +* trivy +* govulnscan + +## Releases + +* release notes generator: git-cliff diff --git a/docs/STYLE.md b/docs/STYLE.md new file mode 100644 index 0000000..07e352f --- /dev/null +++ b/docs/STYLE.md @@ -0,0 +1,69 @@ +# Coding style at `go-openapi` + +**DRAFT** + +> **TL;DR** +> +> We've never been super-strict on code style etc. +> But now go-openapi and go-swagger make a large codebase to maintain and keep afloat. +> +> Code quality and the harmonization of rules have thus become something that we need now. + +## Meta-linter + +Universally formatted go code promotes ease of writing, reading, and maintenance. + +You should run `golangci-lint run` before committing your changes. + +Many editors have plugins that do that automatically. + +> We use the `golangci-lint` meta-linter. The configuration lies in `.golangci-lint.yml`. +> You may read for additional reference. + +## Linting rules posture + +Thanks to go's original design, we developers don't have to waste much time arguing about code figures of style. + +We enable all linters published by `golangci-lint` by default, then disable a few ones. + +Here are the reasons why they are disabled: + +```yaml + disable: + - depguard # we don't want to configure rules to constrain import. That's the reviewer's job + - exhaustruct # we don't want to configure regexp's to check type name. That's the reviewer's job + - funlen # we accept cognitive complexity as a meaningful metric, but function length is relevant + - godox # we don't see any value in forbidding TODO's etc in code + - nlreturn # we usually apply this "blank line" rule to make code less compact. We just don't want to enforce it. + - nonamedreturns # we don't see any valid reason why we couldn't used named returns. + - noinlineerr # there is no value added forbidding inlined err + - paralleltest # we like parallel tests. We just don't want this to be enforced everywhere. + - recvcheck # we like the idea of having pointer and non-pointer receivers + - testpackage # we like test packages. We just don't want it to be enforced everywhere. + - tparallel # see paralleltest + - varnamelen # sometimes, we like short variables + - whitespace # no added value + - wrapcheck # although there is some sense with this linter's general idea, it produces too much noise + - wsl # no added value. Noise. + - wsl_v5 # no added value. Noise. +``` + +Enabled linters with relaxed constraints: + +```yaml + settings: + dupl: + threshold: 200 # in a older code base such as ours, we have to be tolerant with a little redundancy + goconst: + min-len: 2 + min-occurrences: 3 + cyclop: + max-complexity: 20 # the default is too low for most of our functions. 20 is a nicer trade-off + gocyclo: + min-complexity: 20 + exhaustive: # when using default in switch, this should be good enough + default-signifies-exhaustive: true + default-case-required: true + lll: + line-length: 180 # we just want to avoid extremely long lines. It is no big deal if a line or two don't fit on your terminal. +``` diff --git a/examples_test.go b/examples_test.go index 8a8dc1a..e4c4222 100644 --- a/examples_test.go +++ b/examples_test.go @@ -5,28 +5,88 @@ package jsonpointer import ( "encoding/json" + "errors" "fmt" ) +var ErrExampleStruct = errors.New("example error") + type exampleDocument struct { Foo []string `json:"foo"` } +func ExampleNew() { + empty, err := New("") + if err != nil { + fmt.Println(err) + + return + } + fmt.Printf("empty pointer: %q\n", empty.String()) + + key, err := New("/foo") + if err != nil { + fmt.Println(err) + + return + } + fmt.Printf("pointer to object key: %q\n", key.String()) + + elem, err := New("/foo/1") + if err != nil { + fmt.Println(err) + + return + } + fmt.Printf("pointer to array element: %q\n", elem.String()) + + escaped0, err := New("/foo~0") + if err != nil { + fmt.Println(err) + + return + } + // key contains "~" + fmt.Printf("pointer to key %q: %q\n", Unescape("foo~0"), escaped0.String()) + + escaped1, err := New("/foo~1") + if err != nil { + fmt.Println(err) + + return + } + // key contains "/" + fmt.Printf("pointer to key %q: %q\n", Unescape("foo~1"), escaped1.String()) + + // output: + // empty pointer: "" + // pointer to object key: "/foo" + // pointer to array element: "/foo/1" + // pointer to key "foo~": "/foo~0" + // pointer to key "foo/": "/foo~1" +} + func ExamplePointer_Get() { var doc exampleDocument if err := json.Unmarshal(testDocumentJSONBytes, &doc); err != nil { // populates doc - panic(err) + fmt.Println(err) + + return } pointer, err := New("/foo/1") if err != nil { - panic(err) + fmt.Println(err) + + return } value, kind, err := pointer.Get(doc) if err != nil { - panic(err) + fmt.Println(err) + + return } fmt.Printf( @@ -43,17 +103,23 @@ func ExamplePointer_Set() { var doc exampleDocument if err := json.Unmarshal(testDocumentJSONBytes, &doc); err != nil { // populates doc - panic(err) + fmt.Println(err) + + return } pointer, err := New("/foo/1") if err != nil { - panic(err) + fmt.Println(err) + + return } result, err := pointer.Set(&doc, "hey my") if err != nil { - panic(err) + fmt.Println(err) + + return } fmt.Printf("result: %#v\n", result) diff --git a/iface_example_test.go b/iface_example_test.go new file mode 100644 index 0000000..1865c48 --- /dev/null +++ b/iface_example_test.go @@ -0,0 +1,147 @@ +package jsonpointer_test + +import ( + "fmt" + + "github.com/go-openapi/jsonpointer" +) + +var ( + _ jsonpointer.JSONPointable = CustomDoc{} + _ jsonpointer.JSONSetable = &CustomDoc{} +) + +// CustomDoc accepts 2 preset properties "propA" and "propB", plus any number of extra properties. +// +// All values are strings. +type CustomDoc struct { + a string + b string + c map[string]string +} + +// JSONLookup implements [jsonpointer.JSONPointable]. +func (d CustomDoc) JSONLookup(key string) (any, error) { + switch key { + case "propA": + return d.a, nil + case "propB": + return d.b, nil + default: + if len(d.c) == 0 { + return nil, fmt.Errorf("key %q not found: %w", key, ErrExampleIface) + } + extra, ok := d.c[key] + if !ok { + return nil, fmt.Errorf("key %q not found: %w", key, ErrExampleIface) + } + + return extra, nil + } +} + +// JSONSet implements [jsonpointer.JSONSetable]. +func (d *CustomDoc) JSONSet(key string, value any) error { + asString, ok := value.(string) + if !ok { + return fmt.Errorf("a CustomDoc only access strings as values, but got %T: %w", value, ErrExampleIface) + } + + switch key { + case "propA": + d.a = asString + + return nil + case "propB": + d.b = asString + + return nil + default: + if len(d.c) == 0 { + d.c = make(map[string]string) + } + d.c[key] = asString + + return nil + } +} + +func Example_iface() { + doc := CustomDoc{ + a: "initial value for a", + b: "initial value for b", + // no extra values + } + + pointerA, err := jsonpointer.New("/propA") + if err != nil { + fmt.Println(err) + + return + } + + // get the initial value for a + propA, kind, err := pointerA.Get(doc) + if err != nil { + fmt.Println(err) + + return + } + fmt.Printf("propA (%v): %v\n", kind, propA) + + pointerB, err := jsonpointer.New("/propB") + if err != nil { + fmt.Println(err) + + return + } + + // get the initial value for b + propB, kind, err := pointerB.Get(doc) + if err != nil { + fmt.Println(err) + + return + } + fmt.Printf("propB (%v): %v\n", kind, propB) + + pointerC, err := jsonpointer.New("/extra") + if err != nil { + fmt.Println(err) + + return + } + + // not found yet + _, _, err = pointerC.Get(doc) + fmt.Printf("propC: %v\n", err) + + _, err = pointerA.Set(&doc, "new value for a") // doc is updated in place + if err != nil { + fmt.Println(err) + + return + } + + _, err = pointerB.Set(&doc, "new value for b") + if err != nil { + fmt.Println(err) + + return + } + + _, err = pointerC.Set(&doc, "new extra value") + if err != nil { + fmt.Println(err) + + return + } + + fmt.Printf("updated doc: %v", doc) + + // output: + // propA (string): initial value for a + // propB (string): initial value for b + // propC: key "extra" not found: example error + // updated doc: {new value for a new value for b map[extra:new extra value]} +} diff --git a/struct_example_test.go b/struct_example_test.go new file mode 100644 index 0000000..31fa6f7 --- /dev/null +++ b/struct_example_test.go @@ -0,0 +1,92 @@ +package jsonpointer_test + +import ( + "errors" + "fmt" + + "github.com/go-openapi/jsonpointer" +) + +var ErrExampleIface = errors.New("example error") + +type ExampleDoc struct { + Promoted EmbeddedDoc `json:"promoted"` + AnonPromoted EmbeddedDoc `json:"-"` + A string `json:"propA"` + Ignored string `json:"-"` + Untagged string + + unexported string +} + +type EmbeddedDoc struct { + B string `json:"propB"` +} + +func Example_struct() { + doc := ExampleDoc{ + Promoted: EmbeddedDoc{ + B: "promoted", + }, + A: "a", + Ignored: "ignored", + unexported: "unexported", + } + + { + // tagged simple field + pointerA, _ := jsonpointer.New("/propA") + a, _, err := pointerA.Get(doc) + if err != nil { + fmt.Println(err) + return + } + fmt.Printf("a: %v\n", a) + } + { + // tagged embedded field is resolved + pointerB, _ := jsonpointer.New("/promoted/propB") + a, _, err := pointerB.Get(doc) + if err != nil { + fmt.Println(err) + return + } + fmt.Printf("b: %v\n", a) + } + + { + // exlicitly ignored by JSON tag. + pointerI, _ := jsonpointer.New("/ignored") + _, _, err := pointerI.Get(doc) + fmt.Printf("ignored: %v\n", err) + } + + { + // unexported field is ignored: use [JSONPointable] to alter this behavior. + pointerX, _ := jsonpointer.New("/unexported") + _, _, err := pointerX.Get(doc) + fmt.Printf("unexported: %v\n", err) + } + + { + // Limitation: anonymous embedded field is not resolved. + pointerC, _ := jsonpointer.New("/propB") + _, _, err := pointerC.Get(doc) + fmt.Printf("anonymous: %v\n", err) + } + + { + // Limitation: untagged exported field is ignored, unlike with json standard MarshalJSON. + pointerU, _ := jsonpointer.New("/untagged") + _, _, err := pointerU.Get(doc) + fmt.Printf("untagged: %v\n", err) + } + + // output: + // a: a + // b: promoted + // ignored: object has no field "ignored": JSON pointer error + // unexported: object has no field "unexported": JSON pointer error + // anonymous: object has no field "propB": JSON pointer error + // untagged: object has no field "untagged": JSON pointer error +}