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

Jump to bottom

Switch to swift-format #1655

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Switch to swift-format #1655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb96350
Switch to swift-format
glbrntt Sep 26, 2023
a48edbc
Re-format code
glbrntt Sep 27, 2023
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
4 changes: 0 additions & 4 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ jobs:
- uses: actions/checkout@v3
- name: "Formatting and License Headers check"
run: |
SWIFTFORMAT_VERSION=0.52.0
git clone --depth 1 --branch "$SWIFTFORMAT_VERSION" "https://github.com/nicklockwood/SwiftFormat" "$HOME/SwiftFormat"
swift build -c release --package-path "$HOME/SwiftFormat" --product swiftformat
export PATH=$PATH:"$(swift build -c release --package-path "$HOME/SwiftFormat" --show-bin-path)"
./scripts/sanity.sh
unit-tests:
strategy:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ Examples/EchoWeb/node_modules
Examples/EchoWeb/package-lock.json
dev/codegen-tests/**/generated/*
/scripts/.swiftformat-source/
/scripts/.swift-format-source/
Package.resolved
*.out.*
58 changes: 58 additions & 0 deletions .swift-format
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
{
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 this file is slightly different than the one in swift-certificates. Can we align on one please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aligning on formatting rules will be a significant amount of churn (52,000 lines) that I don't think is worth doing. The aim of this PR is to reduce churn by switching to a tool which produces more stable formatting; in that light the rules chosen follow the existing ones where possible to minimise the changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. On the other hand we are planning to do some significant work in gRPC and that might be a good opportunity to do this one time churn. Having an aligned formatting file across our repos is IMO worth it in the long run. We could take this churn and just tell git to ignore this one commit in blames so it shouldn't even show up.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not convinced: we still need to work on and maintain v1 for quite some time so will still bear the cost of the significant churn. Formatters let users avoid having to do the manual work of sticking to style guidelines so the benefit of having a consistent style across projects is very low, IMO.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure we have to maintain the v1 but this is a one time churn right? If we decide that swift-format is the future for our project then IMO we should take that one time churn once and then have everything aligned.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, it's one time but it's a very large change. As I said above, I think the benefit of having consistent style across projects is low, especially as we have tools which enforce the style for you.

What we're effectively debating is which style to use. I would much prefer we stick to the style used in this PR because it minimises the changes made, so much so that they can be reviewed with relative ease.

"fileScopedDeclarationPrivacy" : {
"accessLevel" : "private"
},
"indentation" : {
"spaces" : 2
},
"indentConditionalCompilationBlocks" : false,
"indentSwitchCaseLabels" : false,
"lineBreakAroundMultilineExpressionChainComponents" : false,
"lineBreakBeforeControlFlowKeywords" : false,
"lineBreakBeforeEachArgument" : true,
"lineBreakBeforeEachGenericRequirement" : false,
"lineLength" : 100,
"maximumBlankLines" : 1,
"prioritizeKeepingFunctionOutputTogether" : true,
"respectsExistingLineBreaks" : true,
"rules" : {
"AllPublicDeclarationsHaveDocumentation" : false,
"AlwaysUseLowerCamelCase" : false,
"AmbiguousTrailingClosureOverload" : true,
"BeginDocumentationCommentWithOneLineSummary" : false,
"DoNotUseSemicolons" : true,
"DontRepeatTypeInStaticProperties" : true,
"FileScopedDeclarationPrivacy" : true,
"FullyIndirectEnum" : true,
"GroupNumericLiterals" : true,
"IdentifiersMustBeASCII" : true,
"NeverForceUnwrap" : false,
"NeverUseForceTry" : false,
"NeverUseImplicitlyUnwrappedOptionals" : false,
"NoAccessLevelOnExtensionDeclaration" : true,
"NoAssignmentInExpressions" : true,
"NoBlockComments" : false,
"NoCasesWithOnlyFallthrough" : true,
"NoEmptyTrailingClosureParentheses" : true,
"NoLabelsInCasePatterns" : false,
"NoLeadingUnderscores" : false,
"NoParensAroundConditions" : true,
"NoVoidReturnOnFunctionSignature" : true,
"OneCasePerLine" : true,
"OneVariableDeclarationPerLine" : true,
"OnlyOneTrailingClosureArgument" : true,
"OrderedImports" : true,
"ReturnVoidInsteadOfEmptyTuple" : true,
"UseEarlyExits" : false,
"UseLetInEveryBoundCaseVariable" : false,
"UseShorthandTypeNames" : true,
"UseSingleLinePropertyGetter" : false,
"UseSynthesizedInitializer" : false,
"UseTripleSlashForDocumentationComments" : true,
"UseWhereClausesInForLoops" : false,
"ValidateDocumentationComments" : false
},
"spacesAroundRangeFormationOperators" : true,
"tabWidth" : 2,
"version" : 1
}
60 changes: 0 additions & 60 deletions .swiftformat
View file
Open in desktop

This file was deleted.

7 changes: 4 additions & 3 deletions Sources/Examples/Echo/Implementation/HPACKHeaders+Prettify.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
import NIOHPACK

func prettify(_ headers: HPACKHeaders) -> String {
return "[" + headers.map { name, value, _ in
"'\(name)': '\(value)'"
}.joined(separator: ", ") + "]"
return "["
+ headers.map { name, value, _ in
"'\(name)': '\(value)'"
}.joined(separator: ", ") + "]"
}
3 changes: 2 additions & 1 deletion Sources/Examples/Echo/Implementation/Interceptors.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ public final class ExampleClientInterceptorFactory: Echo_EchoClientInterceptorFa

// Returns an array of interceptors to use for the 'Collect' RPC.
public func makeCollectInterceptors()
-> [ClientInterceptor<Echo_EchoRequest, Echo_EchoResponse>] {
-> [ClientInterceptor<Echo_EchoRequest, Echo_EchoResponse>]
{
return [LoggingEchoClientInterceptor()]
}

Expand Down
5 changes: 3 additions & 2 deletions Sources/Examples/Echo/Runtime/Echo.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import GRPC
import GRPCSampleData
import NIOCore
import NIOPosix

#if canImport(NIOSSL)
import NIOSSL
#endif
Expand Down Expand Up @@ -136,7 +137,7 @@ func startEchoServer(group: EventLoopGroup, port: Int, useTLS: Bool) async throw
print("starting secure server")
#else
fatalError("'useTLS: true' passed to \(#function) but NIOSSL is not available")
#endif // canImport(NIOSSL)
#endif // canImport(NIOSSL)
} else {
print("starting insecure server")
builder = Server.insecure(group: group)
Expand Down Expand Up @@ -180,7 +181,7 @@ func makeClient(
.withTLS(trustRoots: .certificates([caCert.certificate]))
#else
fatalError("'useTLS: true' passed to \(#function) but NIOSSL is not available")
#endif // canImport(NIOSSL)
#endif // canImport(NIOSSL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a bug in swift-format. There should only be one space.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure it is; it's quite common to include two spaces before an inline comment.

} else {
builder = ClientConnection.insecure(group: group)
}
Expand Down
8 changes: 5 additions & 3 deletions Sources/Examples/PacketCapture/PacketCapture.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ struct PCAP: AsyncParsableCommand {
// used TLS we would likely want to place the handler in a different position in the
// pipeline so that the captured packets in the trace would not be encrypted.
let writePCAPHandler = NIOWritePCAPHandler(mode: .client, fileSink: fileSink.write(buffer:))
return channel.eventLoop.makeCompletedFuture(Result {
try channel.pipeline.syncOperations.addHandler(writePCAPHandler, position: .first)
})
return channel.eventLoop.makeCompletedFuture(
Result {
try channel.pipeline.syncOperations.addHandler(writePCAPHandler, position: .first)
}
)
}
}

Expand Down
8 changes: 4 additions & 4 deletions Sources/Examples/RouteGuide/Client/RouteGuideClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import RouteGuideModel
/// Loads the features from `route_guide_db.json`, assumed to be in the directory above this file.
func loadFeatures() throws -> [Routeguide_Feature] {
let url = URL(fileURLWithPath: #filePath)
.deletingLastPathComponent() // main.swift
.deletingLastPathComponent() // Client/
.deletingLastPathComponent() // main.swift
.deletingLastPathComponent() // Client/
.appendingPathComponent("route_guide_db.json")

let data = try Data(contentsOf: url)
Expand Down Expand Up @@ -146,8 +146,8 @@ extension RouteGuideExample {
let summary = try await recordRoute.response

print(
"Finished trip with \(summary.pointCount) points. Passed \(summary.featureCount) features. " +
"Travelled \(summary.distance) meters. It took \(summary.elapsedTime) seconds."
"Finished trip with \(summary.pointCount) points. Passed \(summary.featureCount) features. "
+ "Travelled \(summary.distance) meters. It took \(summary.elapsedTime) seconds."
)
} catch {
print("RecordRoute Failed: \(error)")
Expand Down
3 changes: 2 additions & 1 deletion Sources/Examples/RouteGuide/Server/RouteGuideProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ extension Routeguide_Point {
let deltaLat = lat2 - lat1
let deltaLon = lon2 - lon1

let a = sin(deltaLat / 2) * sin(deltaLat / 2)
let a =
sin(deltaLat / 2) * sin(deltaLat / 2)
+ cos(lat1) * cos(lat2) * sin(deltaLon / 2) * sin(deltaLon / 2)
let c = 2 * atan2(sqrt(a), sqrt(1 - a))

Expand Down
9 changes: 5 additions & 4 deletions Sources/Examples/RouteGuide/Server/RouteGuideServer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,19 @@
* limitations under the License.
*/
import ArgumentParser
import struct Foundation.Data
import struct Foundation.URL
import GRPC
import NIOCore
import NIOPosix
import RouteGuideModel

import struct Foundation.Data
import struct Foundation.URL

/// Loads the features from `route_guide_db.json`, assumed to be in the directory above this file.
func loadFeatures() throws -> [Routeguide_Feature] {
let url = URL(fileURLWithPath: #filePath)
.deletingLastPathComponent() // main.swift
.deletingLastPathComponent() // Server/
.deletingLastPathComponent() // main.swift
.deletingLastPathComponent() // Server/
.appendingPathComponent("route_guide_db.json")

let data = try Data(contentsOf: url)
Expand Down
6 changes: 4 additions & 2 deletions Sources/GRPC/AsyncAwaitSupport/Call+AsyncRequestStreamWriter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ extension Call where Request: Sendable, Response: Sendable {
GRPCAsyncWriterSinkDelegate<(Request, Compression)>
>
internal func makeRequestStreamWriter()
-> (GRPCAsyncRequestStreamWriter<Request>, AsyncWriter.Sink) {
-> (GRPCAsyncRequestStreamWriter<Request>, AsyncWriter.Sink)
{
let delegate = GRPCAsyncWriterSinkDelegate<(Request, Compression)>(
didYield: { requests in
for (request, compression) in requests {
let compress = compression
let compress =
compression
.isEnabled(callDefault: self.options.messageEncoding.enabledForRequests)

// TODO: be smarter about inserting flushes.
Expand Down
13 changes: 7 additions & 6 deletions Sources/GRPC/AsyncAwaitSupport/GRPCAsyncBidirectionalStreamingCall.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ import NIOHPACK
public struct GRPCAsyncBidirectionalStreamingCall<Request: Sendable, Response: Sendable>: Sendable {
private let call: Call<Request, Response>
private let responseParts: StreamingResponseParts<Response>
private let responseSource: NIOThrowingAsyncSequenceProducer<
Response,
Error,
NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark,
GRPCAsyncSequenceProducerDelegate
>.Source
private let responseSource:
NIOThrowingAsyncSequenceProducer<
Response,
Error,
NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark,
GRPCAsyncSequenceProducerDelegate
>.Source
private let requestSink: AsyncSink<(Request, Compression)>

/// A request stream writer for sending messages to the server.
Expand Down
26 changes: 15 additions & 11 deletions Sources/GRPC/AsyncAwaitSupport/GRPCAsyncServerHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,23 @@ internal final class AsyncServerHandler<
internal private(set) var handlerStateMachine: ServerHandlerStateMachine
/// A bag of components used by the user handler.
@usableFromInline
internal private(set) var handlerComponents: Optional<ServerHandlerComponents<
Request,
Response,
GRPCAsyncWriterSinkDelegate<(Response, Compression)>
>>
internal private(set) var handlerComponents:
Optional<
ServerHandlerComponents<
Request,
Response,
GRPCAsyncWriterSinkDelegate<(Response, Compression)>
>
>

/// The user provided function to execute.
@usableFromInline
internal let userHandler: @Sendable (
GRPCAsyncRequestStream<Request>,
GRPCAsyncResponseStreamWriter<Response>,
GRPCAsyncServerCallContext
) async throws -> Void
internal let userHandler:
@Sendable (
GRPCAsyncRequestStream<Request>,
GRPCAsyncResponseStreamWriter<Response>,
GRPCAsyncServerCallContext
) async throws -> Void

@usableFromInline
internal typealias AsyncSequenceProducer = NIOThrowingAsyncSequenceProducer<
Expand Down Expand Up @@ -415,7 +419,7 @@ internal final class AsyncServerHandler<
internal func receiveInterceptedMetadata(_ headers: HPACKHeaders) {
switch self.interceptorStateMachine.interceptedRequestMetadata() {
case .forward:
() // continue
() // continue
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 wrong too.

case .cancel:
return self.cancel(error: nil)
case .drop:
Expand Down
13 changes: 7 additions & 6 deletions Sources/GRPC/AsyncAwaitSupport/GRPCAsyncServerStreamingCall.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ import NIOHPACK
public struct GRPCAsyncServerStreamingCall<Request: Sendable, Response: Sendable> {
private let call: Call<Request, Response>
private let responseParts: StreamingResponseParts<Response>
private let responseSource: NIOThrowingAsyncSequenceProducer<
Response,
Error,
NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark,
GRPCAsyncSequenceProducerDelegate
>.Source
private let responseSource:
NIOThrowingAsyncSequenceProducer<
Response,
Error,
NIOAsyncSequenceProducerBackPressureStrategies.HighLowWatermark,
GRPCAsyncSequenceProducerDelegate
>.Source

/// The stream of responses from the server.
public let responseStream: GRPCAsyncResponseStream<Response>
Expand Down
4 changes: 2 additions & 2 deletions Sources/GRPC/AsyncAwaitSupport/GRPCClient+AsyncAwaitSupport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ extension GRPCClient {
requestType: Request.Type = Request.self,
responseType: Response.Type = Response.self
) -> GRPCAsyncResponseStream<Response>
where RequestStream.Element == Request {
where RequestStream.Element == Request {
let call = self.channel.makeAsyncBidirectionalStreamingCall(
path: path,
callOptions: callOptions ?? self.defaultCallOptions,
Expand All @@ -350,7 +350,7 @@ extension GRPCClient {
requestType: Request.Type = Request.self,
responseType: Response.Type = Response.self
) -> GRPCAsyncResponseStream<Response>
where RequestStream.Element == Request {
where RequestStream.Element == Request {
let call = self.channel.makeAsyncBidirectionalStreamingCall(
path: path,
callOptions: callOptions ?? self.defaultCallOptions,
Expand Down
Loading