Skip to content

Commit

Permalink
Compile with -warnings-as-errors in CI (incl. StrictSendability) (#281)
Browse files Browse the repository at this point in the history
### Motivation

We were not compiling with `-warnings-as-errors` in CI because some of
the reference source code contained deprecation annotations. This meant
that we are not getting CI failures for other warnings that we'd like to
be clean of; specifically strict concurrency. However, in addition to
the file-based reference tests, we now have in-memory snippet-based
reference tests, which contain tests for deprecated OpenAPI schemas and
operations.

### Modifications

- Remove deprecated tests from file-based reference test.
- Clean up warnings under StrictConcurrency.
- Compile with -warnings-as-errors in CI.
- Opt into StrictConcurrency and `-warnings-as-errors` locally with
environment variable.

### Result

- CI will catch sendability warning regressions.
- Developing locally with `SWIFT_OPENAPI_STRICT_CONCURRENCY` will enable
the same behaviour.

### Test Plan

- Added a canary commit to the end of this PR with a sendability issue,
which will be reverted before merging.

---------

Signed-off-by: Si Beaumont <[email protected]>
  • Loading branch information
simonjbeaumont authored Sep 18, 2023
1 parent 445e55b commit a07ebd2
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 59 deletions.
10 changes: 10 additions & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import Foundation
import PackageDescription

// General Swift-settings for all targets.
Expand All @@ -23,6 +24,15 @@ swiftSettings.append(
// Require `any` for existential types.
.enableUpcomingFeature("ExistentialAny")
)

// Strict concurrency is enabled in CI; use this environment variable to enable it locally.
if ProcessInfo.processInfo.environment["SWIFT_OPENAPI_STRICT_CONCURRENCY"].flatMap(Bool.init) ?? false {
#warning("Compiling with Strict Concurrency")
swiftSettings.append(contentsOf: [
.enableExperimentalFeature("StrictConcurrency"),
.unsafeFlags(["-warnings-as-errors"]),
])
}
#endif

let package = Package(
Expand Down
2 changes: 1 addition & 1 deletion Sources/_OpenAPIGeneratorCore/GeneratorMode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//===----------------------------------------------------------------------===//

/// Describes the file to be generated from the specified OpenAPI document.
public enum GeneratorMode: String, Codable, CaseIterable {
public enum GeneratorMode: String, Codable, CaseIterable, Sendable {

/// A file that contains the API protocol, reusable types, and operation
/// namespaces.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,21 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import Foundation
#if os(Linux)
@preconcurrency import struct Foundation.URL
@preconcurrency import struct Foundation.Data
#else
import struct Foundation.URL
import struct Foundation.Data
#endif

/// An in-memory file that contains the generated Swift code.
typealias RenderedSwiftRepresentation = InMemoryOutputFile

/// An in-memory input file that contains the raw data of an OpenAPI document.
///
/// Contents are formatted as either YAML or JSON.
public struct InMemoryInputFile {
public struct InMemoryInputFile: Sendable {

/// The absolute path to the file.
public var absolutePath: URL
Expand All @@ -38,7 +44,7 @@ public struct InMemoryInputFile {
}

/// An in-memory output file that contains the generated Swift source code.
public struct InMemoryOutputFile {
public struct InMemoryOutputFile: Sendable {

/// The base name of the file.
public var baseName: String
Expand Down
3 changes: 1 addition & 2 deletions Tests/OpenAPIGeneratorReferenceTests/CompatabilityTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ final class CompatibilityTest: XCTestCase {
let compatibilityTestNumBuildJobs = getIntEnv("SWIFT_OPENAPI_COMPATIBILITY_TEST_NUM_BUILD_JOBS")

override func setUp() async throws {
setbuf(stdout, nil)
continueAfterFailure = false
try XCTSkipUnless(compatibilityTestEnabled)
if _isDebugAssertConfiguration() {
Expand Down Expand Up @@ -333,7 +332,7 @@ fileprivate extension CompatibilityTest {
}

/// Records diagnostics into an array for testing.
private final class RecordingDiagnosticCollector: DiagnosticCollector {
private final class RecordingDiagnosticCollector: DiagnosticCollector, @unchecked Sendable {
private let lock = NSLock()
private var _diagnostics: [Diagnostic] = []
var diagnostics: [Diagnostic] {
Expand Down
20 changes: 0 additions & 20 deletions Tests/OpenAPIGeneratorReferenceTests/Resources/Docs/petstore.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ paths:
/probe/:
post:
operationId: probe
deprecated: true
responses:
'204':
description: Ack
Expand Down Expand Up @@ -229,13 +228,6 @@ components:
schema:
type: integer
format: int64
header.deprecatedHeader:
name: deprecatedHeader
in: header
deprecated: true
description: A deprecated header parameter
schema:
type: string
schemas:
Pet:
type: object
Expand Down Expand Up @@ -277,7 +269,6 @@ components:
PetKind:
type: string
description: "Kind of pet"
deprecated: true
enum:
- cat
- dog
Expand Down Expand Up @@ -421,17 +412,6 @@ components:
- $ref: '#/components/schemas/MessagedExercise'
discriminator:
propertyName: kind
DeprecatedObject:
deprecated: true
type: object
properties: {}
additionalProperties: false
ObjectWithDeprecatedProperty:
type: object
properties:
message:
type: string
deprecated: true
PetStats:
type: object
properties:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,7 @@ public struct Client: APIProtocol {
}
/// - Remark: HTTP `POST /probe/`.
/// - Remark: Generated from `#/paths//probe//post(probe)`.
@available(*, deprecated) public func probe(_ input: Operations.probe.Input) async throws -> Operations.probe.Output
{
public func probe(_ input: Operations.probe.Input) async throws -> Operations.probe.Output {
try await client.send(
input: input,
forOperation: Operations.probe.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ fileprivate extension UniversalServer where APIHandler: APIProtocol {
}
/// - Remark: HTTP `POST /probe/`.
/// - Remark: Generated from `#/paths//probe//post(probe)`.
@available(*, deprecated) func probe(request: Request, metadata: ServerRequestMetadata) async throws -> Response {
func probe(request: Request, metadata: ServerRequestMetadata) async throws -> Response {
try await handle(
request: request,
with: metadata,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public protocol APIProtocol: Sendable {
func postStats(_ input: Operations.postStats.Input) async throws -> Operations.postStats.Output
/// - Remark: HTTP `POST /probe/`.
/// - Remark: Generated from `#/paths//probe//post(probe)`.
@available(*, deprecated) func probe(_ input: Operations.probe.Input) async throws -> Operations.probe.Output
func probe(_ input: Operations.probe.Input) async throws -> Operations.probe.Output
/// Update just a specific property of an existing pet. Nothing is updated if no request body is provided.
///
/// - Remark: HTTP `PATCH /pets/{petId}`.
Expand Down Expand Up @@ -592,23 +592,6 @@ public enum Components {
}
}
}
/// - Remark: Generated from `#/components/schemas/DeprecatedObject`.
@available(*, deprecated) public struct DeprecatedObject: Codable, Hashable, Sendable {
/// Creates a new `DeprecatedObject`.
public init() {}
public init(from decoder: any Decoder) throws { try decoder.ensureNoAdditionalProperties(knownKeys: []) }
}
/// - Remark: Generated from `#/components/schemas/ObjectWithDeprecatedProperty`.
public struct ObjectWithDeprecatedProperty: Codable, Hashable, Sendable {
/// - Remark: Generated from `#/components/schemas/ObjectWithDeprecatedProperty/message`.
@available(*, deprecated) public var message: Swift.String?
/// Creates a new `ObjectWithDeprecatedProperty`.
///
/// - Parameters:
/// - message:
public init(message: Swift.String? = nil) { self.message = message }
public enum CodingKeys: String, CodingKey { case message }
}
/// - Remark: Generated from `#/components/schemas/PetStats`.
public struct PetStats: Codable, Hashable, Sendable {
/// - Remark: Generated from `#/components/schemas/PetStats/count`.
Expand All @@ -631,10 +614,6 @@ public enum Components {
///
/// - Remark: Generated from `#/components/parameters/path.petId`.
public typealias path_period_petId = Swift.Int64
/// A deprecated header parameter
///
/// - Remark: Generated from `#/components/parameters/header.deprecatedHeader`.
public typealias header_period_deprecatedHeader = Swift.String
}
/// Types generated from the `#/components/requestBodies` section of the OpenAPI document.
public enum RequestBodies {
Expand Down
9 changes: 1 addition & 8 deletions docker/docker-compose.2204.59.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,7 @@ services:
test:
image: *image
environment:
# Because OpenAPI supports deprecation, the generated code could include
# deprecated symbols, which will produce warnings.
#
# We'll disable -warnings-as-errors for now and revisit this when we
# refactor the compilation of the generated code into a separate
# pipeline.
#
# - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors
- WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors
- IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error
- STRICT_CONCURRENCY_ARG=-Xswiftc -strict-concurrency=complete

Expand Down

0 comments on commit a07ebd2

Please sign in to comment.