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

Don't emit deprecation warnings (coming from openapi-generated code) #715

Open
MahdiBM opened this issue Jan 15, 2025 · 7 comments
Open
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.

Comments

@MahdiBM
Copy link
Contributor

MahdiBM commented Jan 15, 2025

Motivation

/__w/penny-bot/penny-bot/Lambdas/GitHubAPI/GeneratedSources/Types.swift:2886:22: warning: 'hasDownloads' is deprecated
 2884 |                 self.hasWiki = hasWiki
 2885 |                 self.hasPages = hasPages
 2886 |                 self.hasDownloads = hasDownloads
      |                      `- warning: 'hasDownloads' is deprecated
 2887 |                 self.hasDiscussions = hasDiscussions
 2888 |                 self.archived = archived
/__w/penny-bot/penny-bot/Lambdas/GitHubAPI/GeneratedSources/Types.swift:2900:22: warning: 'useSquashPrTitleAsDefault' is deprecated
 2898 |                 self.deleteBranchOnMerge = deleteBranchOnMerge
 2899 |                 self.allowUpdateBranch = allowUpdateBranch
 2900 |                 self.useSquashPrTitleAsDefault = useSquashPrTitleAsDefault
      |                      `- warning: 'useSquashPrTitleAsDefault' is deprecated
 2901 |                 self.squashMergeCommitTitle = squashMergeCommitTitle
 2902 |                 self.squashMergeCommitMessage = squashMergeCommitMessage
/__w/penny-bot/penny-bot/Lambdas/GitHubAPI/GeneratedSources/Types.swift:2886:22: warning: 'hasDownloads' is deprecated
 2884 |                 self.hasWiki = hasWiki
 2885 |                 self.hasPages = hasPages
 2886 |                 self.hasDownloads = hasDownloads
      |                      `- warning: 'hasDownloads' is deprecated
 2887 |                 self.hasDiscussions = hasDiscussions
 2888 |                 self.archived = archived
/__w/penny-bot/penny-bot/Lambdas/GitHubAPI/GeneratedSources/Types.swift:2900:22: warning: 'useSquashPrTitleAsDefault' is deprecated
 2898 |                 self.deleteBranchOnMerge = deleteBranchOnMerge
 2899 |                 self.allowUpdateBranch = allowUpdateBranch
 2900 |                 self.useSquashPrTitleAsDefault = useSquashPrTitleAsDefault
      |                      `- warning: 'useSquashPrTitleAsDefault' is deprecated
 2901 |                 self.squashMergeCommitTitle = squashMergeCommitTitle
 2902 |                 self.squashMergeCommitMessage = squashMergeCommitMessage
/__w/penny-bot/penny-bot/Lambdas/GitHubAPI/GeneratedSources/Types.swift:2886:22: warning: 'hasDownloads' is deprecated
 2884 |                 self.hasWiki = hasWiki
 2885 |                 self.hasPages = hasPages
 2886 |                 self.hasDownloads = hasDownloads
      |                      `- warning: 'hasDownloads' is deprecated
 2887 |                 self.hasDiscussions = hasDiscussions
 2888 |                 self.archived = archived
/__w/penny-bot/penny-bot/Lambdas/GitHubAPI/GeneratedSources/Types.swift:2900:22: warning: 'useSquashPrTitleAsDefault' is deprecated
 2898 |                 self.deleteBranchOnMerge = deleteBranchOnMerge
 2899 |                 self.allowUpdateBranch = allowUpdateBranch
 2900 |                 self.useSquashPrTitleAsDefault = useSquashPrTitleAsDefault
      |                      `- warning: 'useSquashPrTitleAsDefault' is deprecated
 2901 |                 self.squashMergeCommitTitle = squashMergeCommitTitle
 2902 |                 self.squashMergeCommitMessage = squashMergeCommitMessage

Proposed solution

I think we could have settings like "upgrade_deprecations_to_unavailabilities" or "don't_mark_as_deprecated_at_all".
Or maybe if we annotate the initializer with the deprecations as well, we can somehow manage not breaking users while also not emitting warnings? we might need 2 inits, one with all properties including deprecateds, and one with no deprecated parameters in it.

Alternatives considered

Live with the warnings?

Additional information

No response

@MahdiBM MahdiBM added kind/feature New feature. status/triage Collecting information required to triage the issue. labels Jan 15, 2025
@czechboy0
Copy link
Contributor

Hi @MahdiBM,

can you take a step back and clarify what the issue is? Are you compiling with warnings-as-errors, so deprecation warnings broke your CI? Or do you just not want to see the deprecation warnings?

I suspect in both cases I'll first point to https://github.com/swiftlang/swift-evolution/blob/main/proposals/0443-warning-control-flags.md, which gives you first class control over diagnostics.

Would that cover your use case?

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Jan 15, 2025

Are you compiling with warnings-as-errors

Well I can't since we're using the openapi generator since a long time ago, but I would if I could.

Or do you just not want to see the deprecation warnings?

That's also correct.

I suspect in both cases I'll first point to https://github.com/swiftlang/swift-evolution/blob/main/proposals/0443-warning-control-flags.md, which gives you first class control over diagnostics.

That's proposal is nice, but it is not quite "fine grained". I wouldn't want to disable basically all of deprecation warnings of a project just to not get deprecation warnings from the OpenAPI Generator generated-code.

Also if there is such an option as "upgrade_deprecations_to_unavailabilities", it would prompt me to not use the deprecated stuff instead of helping me ignore them by silencing the warnings.

@simonjbeaumont
Copy link
Collaborator

I think I'd be open to a config to disable emitting deprecation annotations. I have examples where they cause warnings even when you don't use the deprecated properties in the schema, and even when I'm filtering to just what's used.

@czechboy0
Copy link
Contributor

I wouldn't want to disable basically all of deprecation warnings of a project just to not get deprecation warnings from the OpenAPI Generator generated-code.

What if you put the generated code into a separate module, and disabled deprecation warnings on just that module? Would that achieve what you need?

@czechboy0
Copy link
Contributor

Yeah, I think it'd be worth pursuing in SwiftPM, rather than to expect every package plugin that generates Swift code to reimplement it: https://github.com/swiftlang/swift-evolution/blob/main/proposals/0443-warning-control-flags.md#support-in-swiftpm

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Jan 15, 2025

I have examples where they cause warnings even when you don't use the deprecated properties in the schema, and even when I'm filtering to just what's used.

That's exactly what is happening in Penny.

What if you put the generated code into a separate module, and disabled deprecation warnings on just that module

We have the generated code in its own GitHubAPI module, but the module does contain some more stuff related to the GitHub API. We could do another module like GitHubAPIGenerated then export it in GitHubAPI but I'd consider that a workaround, not a solution.

I think it'd be worth pursuing in SwiftPM

SwiftPM is not the problem there though. The compiler stuff themselves are insufficient.

@czechboy0
Copy link
Contributor

SwiftPM is not the problem there though. The compiler stuff themselves are insufficient.

Is it? The compiler allows you to customize deprecation warnings for a target. Isn't that also what this issue asks for, just implemented in the generator? Since all the generated files always end up in one target.

This is analogous to the feature request of generating all code into an explicit Swift namespace. We considered it, but since Swift modules solve this problem at the lower level, we chose not to. Customizing the handling of warnings rings similar, especially with the recent SE proposal, which explicitly calls our in future work doing this easily per target at the SwiftPM level.

philippzagar added a commit to StanfordSpezi/SpeziLLM that referenced this issue Feb 24, 2025
# OpenAPI spec preprocessing

## ♻️ Current situation & Problem
The OpenAI OpenAPI specification contains the following issues that
require preprocessing before code generation:

- **Incorrect `required` Property**: A non-existent property is
incorrectly marked as `required` (see
[`openai-openapi#421`](openai/openai-openapi#421)).
- **Unsupported `oneOf` Syntax**: The `swift-openapi-generator` does not
fully support `oneOf` with `required` properties (see
[`swift-openapi-generator#739`](apple/swift-openapi-generator#739)).
- **Deprecation Warnings**: `deprecated` markings in the OpenAPI spec
trigger warnings in the generated Swift code (see
[`swift-openapi-generator#106`](apple/swift-openapi-generator#106)
and
[`swift-openapi-generator#715`](apple/swift-openapi-generator#715)).

Without preprocessing, these issues result in unnecessary warnings
during the Swift code generation and in the resulting Swift client code.

## ⚙️ Release Notes 
- Add OpenAPI spec preprocessing script
- Preprocess OpenAPI OpenAI spec 


## 📚 Documentation
README for proper motivation for the need of preprocessing and usage
instructions.


## ✅ Testing
Local testing


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature. status/triage Collecting information required to triage the issue.
Projects
None yet
Development

No branches or pull requests

3 participants