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.

Sign up for GitHub

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

Add deprecation annotations for RPCs marked "deprecated=true" #316

Merged
merged 3 commits into from
Oct 28, 2024
Merged

Add deprecation annotations for RPCs marked "deprecated=true" #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ee7dcb
305: Acknowledge deprecated option
eseay Oct 24, 2024
24ae23d
305: Apply deprecation annotations for generated mocks
eseay Oct 24, 2024
dd40511
305: PR feedback
eseay Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion Plugins/ConnectMocksPlugin/ConnectMockGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ final class ConnectMockGenerator: Generator {

private func printCallbackMethodMockImplementation(for method: MethodDescriptor) {
self.printLine()
if let availabilityAnnotation = method.callbackAvailabilityAnnotation() {
self.printLine(availabilityAnnotation)
}
if !method.serverStreaming && !method.clientStreaming {
self.printLine("@discardableResult")
}
Expand Down Expand Up @@ -150,7 +153,9 @@ final class ConnectMockGenerator: Generator {

private func printAsyncAwaitMethodMockImplementation(for method: MethodDescriptor) {
self.printLine()

if let availabilityAnnotation = method.asyncAwaitAvailabilityAnnotation() {
self.printLine(availabilityAnnotation)
}
self.printLine(
"\(self.typeVisibility) "
+ method.asyncAwaitSignature(
Expand Down Expand Up @@ -219,4 +224,28 @@ private extension MethodDescriptor {
"""
}
}

func callbackAvailabilityAnnotation() -> String? {
if self.options.deprecated {
// swiftlint:disable line_length
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rebello95 @jhump Let me know if this is acceptable. I use SwiftLint in my own repos as well, so I know the rules exist for a purpose; however, I may propose disabling this specific rule in this specific context to improve legibility in the editor and to more closely mimic what will actually be output at the time of generation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems fine to me, doing additional gymnastics would be a bit excessive 😉

return """
@available(iOS, introduced: 12, deprecated: 12, message: "This RPC has been marked as deprecated in its `.proto` file.")
@available(macOS, introduced: 10.15, deprecated: 10.15, message: "This RPC has been marked as deprecated in its `.proto` file.")
@available(tvOS, introduced: 13, deprecated: 13, message: "This RPC has been marked as deprecated in its `.proto` file.")
@available(watchOS, introduced: 6, deprecated: 6, message: "This RPC has been marked as deprecated in its `.proto` file.")
"""
// swiftlint:enable line_length
} else {
return nil
}
}

func asyncAwaitAvailabilityAnnotation() -> String? {
if self.options.deprecated {
// swiftlint:disable:next line_length
return "@available(iOS, introduced: 13, deprecated: 13, message: \"This RPC has been marked as deprecated in its `.proto` file.\")"
} else {
return nil
}
}
}
34 changes: 32 additions & 2 deletions Plugins/ConnectSwiftPlugin/ConnectClientGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ final class ConnectClientGenerator: Generator {
private func printCallbackMethodInterface(for method: MethodDescriptor) {
self.printLine()
self.printCommentsIfNeeded(for: method)
if let availabilityAnnotation = method.callbackAvailabilityAnnotation() {
self.printLine(availabilityAnnotation)
}
if !method.serverStreaming && !method.clientStreaming {
self.printLine("@discardableResult")
}
Expand All @@ -133,7 +136,7 @@ final class ConnectClientGenerator: Generator {
private func printAsyncAwaitMethodInterface(for method: MethodDescriptor) {
self.printLine()
self.printCommentsIfNeeded(for: method)
self.printLine("@available(iOS 13, *)")
self.printLine(method.asyncAwaitAvailabilityAnnotation())
self.printLine(
method.asyncAwaitSignature(
using: self.namer, includeDefaults: false, options: self.options
Expand All @@ -143,6 +146,9 @@ final class ConnectClientGenerator: Generator {

private func printCallbackMethodImplementation(for method: MethodDescriptor) {
self.printLine()
if let availabilityAnnotation = method.callbackAvailabilityAnnotation() {
self.printLine(availabilityAnnotation)
}
if !method.serverStreaming && !method.clientStreaming {
self.printLine("@discardableResult")
}
Expand All @@ -162,7 +168,7 @@ final class ConnectClientGenerator: Generator {

private func printAsyncAwaitMethodImplementation(for method: MethodDescriptor) {
self.printLine()
self.printLine("@available(iOS 13, *)")
self.printLine(method.asyncAwaitAvailabilityAnnotation())
self.printLine(
"\(self.visibility) "
+ method.asyncAwaitSignature(
Expand Down Expand Up @@ -201,6 +207,30 @@ private extension MethodDescriptor {
}
}

func callbackAvailabilityAnnotation() -> String? {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think all these helper functions can be private

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I left them without explicit access to align with the functions that were already there; however, I think we're good on not needing to specify access level since the enclosing extension private extension MethodDescriptor is already marked.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah didn't realize these were inside a private extension 👍🏽

if self.options.deprecated {
// swiftlint:disable line_length
return """
@available(iOS, introduced: 12, deprecated: 12, message: "This RPC has been marked as deprecated in its `.proto` file.")
@available(macOS, introduced: 10.15, deprecated: 10.15, message: "This RPC has been marked as deprecated in its `.proto` file.")
@available(tvOS, introduced: 13, deprecated: 13, message: "This RPC has been marked as deprecated in its `.proto` file.")
@available(watchOS, introduced: 6, deprecated: 6, message: "This RPC has been marked as deprecated in its `.proto` file.")
"""
// swiftlint:enable line_length
} else {
return nil
}
}

func asyncAwaitAvailabilityAnnotation() -> String {
if self.options.deprecated {
// swiftlint:disable:next line_length
return "@available(iOS, introduced: 13, deprecated: 13, message: \"This RPC has been marked as deprecated in its `.proto` file.\")"
} else {
return "@available(iOS 13, *)"
}
}

func callbackReturnValue() -> String {
if self.clientStreaming && self.serverStreaming {
return """
Expand Down
Loading