Unexported fields not working anymore since v2.2.0

[lukasbash]

Since version v2.2.0 all unexported fields (renamed via x-go-name) are now exported even though they have been named lowercase. Reproducible with the example xgoname in this repository. Would need a critical fix for this.

With version 2.1.0 everything works out.

EDIT: Seems to affect fields only, not direct types.

[jamietanna]

Thanks for the report - what's the use-case for having models that produce unexported fields?

Not saying that we won't fix it, but it's a a surprising use-case, so would be interested to hear why you're doing this @lukasbash

[jamietanna]

With the proposed code (from v2.1.0's behaviour), we end up receiving a go vet violation:

% go vet
# github.com/deepmap/oapi-codegen/v2/examples/extensions/x
./gen.go:14:2: struct field accountIdentifier has json tag but is not exported

[lukasbash]

@jamietanna We are using some unexported fields of models during internal processing which offers dynamic implementations on the server side. Sometimes we are using those fields for performance optimizations whereas the clients do not need those fields e.g. when the model is part of a JSON response.

Ironically - with regard to your latest reply - for those unexported fields we usually comply with omitting field tags like:

# ...
            x-oapi-codegen-extra-tags:
              json: "-"
              gorm: "-"
              foo: "-"
              bar: "-"
# ...

What implementation/usage do you suggest? Throwing an error on generation when an unexported field contains a tag? Or do you state that generated files might not be fully lint-compliant and people have to exclude those from linting?

EDIT: The vet violation was not present in v2.1.0? Strange, because I would guess that the tag implementation didn't change for this, did it?

[jamietanna]

The vet violation was not present in v2.1.0?

In the v2.1.0 code we didn't have any cases in the codebase which would rely on producing unexported fields, which is why we wouldn't have seen any violations. I can imagine that if we did, we would've fixed the violation.

It's not something I'd like to recommend is done by default, but I think it'd be OK to add it in as an opt-in feature - would that work for you?

If so, I'd be happy if you want to contribute it? Although we've not yet got a "howto" doc on doing this, https://github.com/oapi-codegen/oapi-codegen/commit/af43038f45d9f63060d24324ec238919e01599de is a recent example that should give a good indication of how to add it in?

[lukasbash]

It's not something I'd like to recommend is done by default [...]

Well over several versions it was the default behavior which is right now a breaking change for me and potentially other codebases 😄 Not really sure if opt-in makes sense as just allowing this behavior does not really break anything, does it? We at our team generally exclude generated files from any linting because the packages that generate the actual source code often times rely on functionality, simplicity and performance and this focus does not always comply to standard linting rules.

Honestly I am most likely not able to contribute, also considering that (as far as I could gather from the changes which removed the behavior) it is actually a bunch of stuff to change.

For now we are back to the latest version that supports non-exported fields. Let's hope either I find some time in the future, or someone else could take care of it 🙁

[jamietanna]

@lukasbash mind a look at https://github.com/oapi-codegen/oapi-codegen/pull/1697 and if that'll cover what you need?

[lukasbash]

@lukasbash mind a look at #1697 and if that'll cover what you need?

Works for me. Awesome fix 👍

I do not really understand why the compatibility flag is needed in the config, as this behavior was always allowed, but nevertheless I hope for a merge and release soon 🤞

[jamietanna]

as this behavior was always allowed

To be clear, it was an accidental feature, not something we expected anyone to rely upon.

Unexported fields can't be used with Go's JSON encoding as you're aware.

And for consumers of the spec, it's unlikely that they're going to be aware that these fields shouldn't be used.

To be clear, this behaviour wasn't something that we were aware of being allowed, or relied upon, and for the reasons mentioned above, this isn't something we'd have wanted folks to rely upon.

However, this should unblock you, and make it possible for you to upgrade now!