Skip to content
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.

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

Use self for member access/assignment in all initializers, decoders, and encoders #700

Conversation

simonjbeaumont
Copy link
Collaborator

Motivation

In order to address #695 we made some targeted fixes (#696, #699) to use explicit-self to avoid clashing with schema property names. This addressed a clash with a specific variable with properties of only certain schemas, but there are more places where we might encounter future issues. Specifically, we have other variable names which might cause a clash (e.g. storage and discriminator), and there are other flavours of initializers, encoders, and decoders that were not updated in the targeted fix.

In this PR, we update all access and assignment in the generated code dealing with schemas to use explicit-self.

Modifications

  • Use self for member access and assignment in all initializers, decoders, and encoders.
  • Update the reference tests and snippet tests.

Result

Removed some cases where valid OpenAPI docs would produce non-compiling code.

Test Plan

Snippet and reference tests. The latter is actually compiled during the CI.

@simonjbeaumont simonjbeaumont added the 🔨 semver/patch No public API change. label Dec 11, 2024
@simonjbeaumont simonjbeaumont changed the title Use self for member access and assignment in all initializers, decoders, and encoders Use self for member access/assignment in all initializers, decoders, and encoders Dec 11, 2024
@simonjbeaumont simonjbeaumont marked this pull request as ready for review December 11, 2024 14:12
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! 🙏

@simonjbeaumont simonjbeaumont merged commit 5dd8d18 into apple:main Dec 11, 2024
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants