Skip to content

Commit 4b38ab0

Browse files
authored
Add a mode flags optionset to FileHandle. (#1597)
This PR adds an `OptionSet`-conforming type to `FileHandle` so that we don't have to hard-code mode strings we pass to `fopen()`. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent f682170 commit 4b38ab0

File tree

7 files changed

+123
-46
lines changed

7 files changed

+123
-46
lines changed

Sources/Testing/Attachments/Attachment.swift

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -432,10 +432,9 @@ extension Attachable where Self: ~Copyable {
432432
#endif
433433

434434
try withUnsafeBytes(for: attachment) { buffer in
435-
// Note "x" in the mode string which indicates that the file should be
436-
// created and opened exclusively. The underlying `fopen()` call will thus
437-
// fail with `EEXIST` if a file exists at `filePath`.
438-
let file = try FileHandle(atPath: filePath, mode: "wxeb")
435+
// The underlying `fopen()` call should fail with `EEXIST` if a file
436+
// exists at `filePath`.
437+
let file = try FileHandle(atPath: filePath, options: [.writeAccess, .creationRequired])
439438
try file.write(buffer)
440439
}
441440
}

Sources/Testing/ExitTests/ExitTest.swift

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -674,21 +674,20 @@ extension ExitTest {
674674
///
675675
/// The effect of calling this function more than once for the same
676676
/// environment variable is undefined.
677-
private static func _makeFileHandle(forEnvironmentVariableNamed name: String, mode: String) -> FileHandle? {
677+
private static func _makeFileHandle(forEnvironmentVariableNamed name: String, options: FileHandle.OpenOptions) -> FileHandle? {
678678
guard let environmentVariable = Environment.variable(named: name) else {
679679
return nil
680680
}
681681

682682
// Erase the environment variable so that it cannot accidentally be opened
683683
// twice (nor, in theory, affect the code of the exit test.)
684684
Environment.setVariable(nil, named: name)
685-
686685
var fd: CInt?
687686
#if SWT_TARGET_OS_APPLE || os(Linux) || os(FreeBSD) || os(OpenBSD)
688687
fd = CInt(environmentVariable)
689688
#elseif os(Windows)
690689
if let handle = UInt(environmentVariable).flatMap(HANDLE.init(bitPattern:)) {
691-
var flags: CInt = switch (mode.contains("r"), mode.contains("w")) {
690+
var flags: CInt = switch (options.contains(.readAccess), options.contains(.writeAccess)) {
692691
case (true, true):
693692
_O_RDWR
694693
case (true, false):
@@ -698,7 +697,7 @@ extension ExitTest {
698697
case (false, false):
699698
0
700699
}
701-
flags |= _O_BINARY
700+
flags |= _O_BINARY | _O_NOINHERIT
702701
fd = _open_osfhandle(Int(bitPattern: handle), flags)
703702
}
704703
#else
@@ -708,7 +707,7 @@ extension ExitTest {
708707
return nil
709708
}
710709

711-
return try? FileHandle(unsafePOSIXFileDescriptor: fd, mode: mode)
710+
return try? FileHandle(unsafePOSIXFileDescriptor: fd, options: options)
712711
}
713712

714713
/// Make a string suitable for use as the value of an environment variable
@@ -768,7 +767,7 @@ extension ExitTest {
768767
// If an exit test was found, inject back channel handling into its body.
769768
// External tools authors should set up their own back channel mechanisms
770769
// and ensure they're installed before calling ExitTest.callAsFunction().
771-
guard let backChannel = _makeFileHandle(forEnvironmentVariableNamed: "SWT_BACKCHANNEL", mode: "wb") else {
770+
guard let backChannel = _makeFileHandle(forEnvironmentVariableNamed: "SWT_BACKCHANNEL", options: [.writeAccess]) else {
772771
return result
773772
}
774773

@@ -1105,7 +1104,7 @@ extension ExitTest {
11051104
private mutating func _decodeCapturedValuesForEntryPoint() throws {
11061105
// Read the content of the captured values stream provided by the parent
11071106
// process above.
1108-
guard let fileHandle = Self._makeFileHandle(forEnvironmentVariableNamed: "SWT_CAPTURED_VALUES", mode: "rb") else {
1107+
guard let fileHandle = Self._makeFileHandle(forEnvironmentVariableNamed: "SWT_CAPTURED_VALUES", options: [.readAccess]) else {
11091108
return
11101109
}
11111110
let capturedValuesJSON = try fileHandle.readToEnd()

Sources/Testing/Support/FileHandle.swift

Lines changed: 102 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,97 @@ struct FileHandle: ~Copyable, Sendable {
4848
Self(unsafeCFILEHandle: swt_stderr(), closeWhenDone: false)
4949
}
5050

51+
/// Options to apply when opening a file.
52+
///
53+
/// An instance of this type does _not_ represent a value of type `mode_t`.
54+
/// Use its ``stringValue`` property to get a string you can pass to `fopen()`
55+
/// and related functions.
56+
struct OpenOptions: RawRepresentable, OptionSet {
57+
var rawValue: Int
58+
59+
/// The file should be opened for reading.
60+
///
61+
/// This option is equivalent to the `"r"` character or `O_RDONLY`. If you
62+
/// also specify ``writeAccess``, the two options combined are equivalent to
63+
/// the `"w+"` characters or `O_RDWR`.
64+
static var readAccess: Self { .init(rawValue: 1 << 0) }
65+
66+
/// The file should be opened for writing.
67+
///
68+
/// This option is equivalent to the `"w"` character or `O_WRONLY`. If you
69+
/// also specify ``readAccess``, the two options combined are equivalent to
70+
/// the `"w+"` characters or `O_RDWR`.
71+
static var writeAccess: Self { .init(rawValue: 1 << 1) }
72+
73+
/// The underlying file descriptor or handle should be inherited by any
74+
/// child processes created by the current process.
75+
///
76+
/// By default, the resulting file handle is not inherited by any child
77+
/// processes (that is, `FD_CLOEXEC` is set on POSIX-like systems and
78+
/// `HANDLE_FLAG_INHERIT` is cleared on Windows.).
79+
///
80+
/// This option is equivalent to the `"e"` character (`"N"` on Windows) or
81+
/// `O_CLOEXEC` (`_O_NOINHERIT` on Windows).
82+
static var inheritedByChild: Self { .init(rawValue: 1 << 2) }
83+
84+
/// The file must be created when opened.
85+
///
86+
/// When you use this option and a file already exists at the given path
87+
/// when you try to open it, the testing library throws an error equivalent
88+
/// to `EEXIST`.
89+
///
90+
/// This option is equivalent to the `"x"` character or `O_CREAT | O_EXCL`.
91+
static var creationRequired: Self { .init(rawValue: 1 << 3) }
92+
93+
/// The string representation of this instance, suitable for passing to
94+
/// `fopen()` and related functions.
95+
var stringValue: String {
96+
var result = ""
97+
result.reserveCapacity(8)
98+
99+
if contains(.writeAccess) {
100+
result.append("w")
101+
} else if contains(.readAccess) {
102+
result.append("r")
103+
}
104+
// Always open files in binary mode, not text mode.
105+
result.append("b")
106+
107+
if contains(.readAccess) && contains(.writeAccess) {
108+
result.append("+")
109+
}
110+
111+
if contains(.creationRequired) {
112+
result.append("x")
113+
}
114+
115+
if !contains(.inheritedByChild) {
116+
#if os(Windows)
117+
// On Windows, "N" is used rather than "e" to signify that a file
118+
// handle is not inherited.
119+
result.append("N")
120+
#else
121+
result.append("e")
122+
#endif
123+
}
124+
125+
#if DEBUG
126+
assert(result.isContiguousUTF8, "Constructed a file mode string '\(result)' that isn't UTF-8.")
127+
#endif
128+
129+
return result
130+
}
131+
}
132+
51133
/// Initialize an instance of this type by opening a file at the given path
52-
/// with the given mode.
134+
/// with the given options.
53135
///
54136
/// - Parameters:
55137
/// - path: The path to open.
56-
/// - mode: The mode to open `path` with, such as `"wb"`.
138+
/// - options: The options to open `path` with.
57139
///
58140
/// - Throws: Any error preventing the stream from being opened.
59-
init(atPath path: String, mode: String) throws {
141+
init(atPath path: String, options: OpenOptions) throws {
60142
#if os(Windows)
61143
// Special-case CONOUT$ to map to stdout. This way, if somebody specifies
62144
// CONOUT$ as the target path for XML or JSON output from `swift test`,
@@ -68,21 +150,14 @@ struct FileHandle: ~Copyable, Sendable {
68150
// To our knowledge, this sort of special-casing is not required on
69151
// POSIX-like platforms (i.e. when opening "/dev/stdout"), but it can be
70152
// adapted for use there if some POSIX-like platform does need it.
71-
if path == "CONOUT$" && mode.contains("w") {
153+
if path == "CONOUT$" && options.contains(.writeAccess) {
72154
self = .stdout
73155
return
74156
}
75157

76-
// On Windows, "N" is used rather than "e" to signify that a file handle is
77-
// not inherited.
78-
var mode = mode
79-
if let eIndex = mode.firstIndex(of: "e") {
80-
mode.replaceSubrange(eIndex ... eIndex, with: "N")
81-
}
82-
83158
// Windows deprecates fopen() as insecure, so call _wfopen_s() instead.
84159
let fileHandle = try path.withCString(encodedAs: UTF16.self) { path in
85-
try mode.withCString(encodedAs: UTF16.self) { mode in
160+
try options.stringValue.withCString(encodedAs: UTF16.self) { mode in
86161
var file: SWT_FILEHandle?
87162
let result = _wfopen_s(&file, path, mode)
88163
guard let file, result == 0 else {
@@ -92,7 +167,7 @@ struct FileHandle: ~Copyable, Sendable {
92167
}
93168
}
94169
#else
95-
guard let fileHandle = fopen(path, mode) else {
170+
guard let fileHandle = fopen(path, options.stringValue) else {
96171
throw CError(rawValue: swt_errno())
97172
}
98173
#endif
@@ -103,28 +178,32 @@ struct FileHandle: ~Copyable, Sendable {
103178
///
104179
/// - Parameters:
105180
/// - path: The path to read from.
181+
/// - options: The options to open the file in. ``OpenOptions/readAccess``
182+
/// is added to this value automatically.
106183
///
107184
/// - Throws: Any error preventing the stream from being opened.
108185
///
109186
/// By default, the resulting file handle is not inherited by any child
110187
/// processes (that is, `FD_CLOEXEC` is set on POSIX-like systems and
111188
/// `HANDLE_FLAG_INHERIT` is cleared on Windows.).
112-
init(forReadingAtPath path: String) throws {
113-
try self.init(atPath: path, mode: "reb")
189+
init(forReadingAtPath path: String, options: OpenOptions = []) throws {
190+
try self.init(atPath: path, options: options.union(.readAccess))
114191
}
115192

116193
/// Initialize an instance of this type to write to the given path.
117194
///
118195
/// - Parameters:
119196
/// - path: The path to write to.
197+
/// - options: The options to open the file in. ``OpenOptions/writeAccess``
198+
/// is added to this value automatically.
120199
///
121200
/// - Throws: Any error preventing the stream from being opened.
122201
///
123202
/// By default, the resulting file handle is not inherited by any child
124203
/// processes (that is, `FD_CLOEXEC` is set on POSIX-like systems and
125204
/// `HANDLE_FLAG_INHERIT` is cleared on Windows.).
126-
init(forWritingAtPath path: String) throws {
127-
try self.init(atPath: path, mode: "web")
205+
init(forWritingAtPath path: String, options: OpenOptions = []) throws {
206+
try self.init(atPath: path, options: options.union(.writeAccess))
128207
}
129208

130209
/// Initialize an instance of this type with an existing C file handle.
@@ -148,17 +227,17 @@ struct FileHandle: ~Copyable, Sendable {
148227
/// - fd: The POSIX file descriptor to wrap. The caller is responsible for
149228
/// ensuring that this file handle is open in the expected mode and that
150229
/// another part of the system won't close it.
151-
/// - mode: The mode `fd` was opened with, such as `"wb"`.
230+
/// - options: The options `fd` was opened with.
152231
///
153232
/// - Throws: Any error preventing the stream from being opened.
154233
///
155234
/// The resulting file handle takes ownership of `fd` and closes it when it is
156235
/// deinitialized or if an error is thrown from this initializer.
157-
init(unsafePOSIXFileDescriptor fd: CInt, mode: String) throws {
236+
init(unsafePOSIXFileDescriptor fd: CInt, options: OpenOptions) throws {
158237
#if os(Windows)
159-
let fileHandle = _fdopen(fd, mode)
238+
let fileHandle = _fdopen(fd, options.stringValue)
160239
#else
161-
let fileHandle = fdopen(fd, mode)
240+
let fileHandle = fdopen(fd, options.stringValue)
162241
#endif
163242
guard let fileHandle else {
164243
let errorCode = swt_errno()
@@ -566,11 +645,11 @@ extension FileHandle {
566645
defer {
567646
fdReadEnd = -1
568647
}
569-
try readEnd = FileHandle(unsafePOSIXFileDescriptor: fdReadEnd, mode: "rb")
648+
try readEnd = FileHandle(unsafePOSIXFileDescriptor: fdReadEnd, options: .readAccess)
570649
defer {
571650
fdWriteEnd = -1
572651
}
573-
try writeEnd = FileHandle(unsafePOSIXFileDescriptor: fdWriteEnd, mode: "wb")
652+
try writeEnd = FileHandle(unsafePOSIXFileDescriptor: fdWriteEnd, options: .writeAccess)
574653
} catch {
575654
// Don't leak file handles! Ensure we've cleared both pointers before
576655
// returning so the state is consistent in the caller.

Tests/TestingTests/AttachmentTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ struct AttachmentTests {
7272

7373
#if !SWT_NO_FILE_IO
7474
func compare(_ attachableValue: borrowing MySendableAttachable, toContentsOfFileAtPath filePath: String) throws {
75-
let file = try FileHandle(forReadingAtPath: filePath)
75+
let file = try Testing.FileHandle(forReadingAtPath: filePath)
7676
let bytes = try file.readToEnd()
7777

7878
let decodedValue = if #available(macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0, *) {

Tests/TestingTests/Support/FileHandleTests.swift

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ struct FileHandleTests {
4848
@Test("Init from invalid file descriptor")
4949
func invalidFileDescriptor() throws {
5050
#expect(throws: CError.self) {
51-
_ = try FileHandle(unsafePOSIXFileDescriptor: -1, mode: "")
51+
_ = try FileHandle(unsafePOSIXFileDescriptor: -1, options: [])
5252
}
5353
}
5454
#endif
@@ -86,7 +86,7 @@ struct FileHandleTests {
8686
@Test("Writing requires contiguous storage")
8787
func writeIsContiguous() async {
8888
await #expect(processExitsWith: .failure) {
89-
let fileHandle = try FileHandle.null(mode: "wb")
89+
let fileHandle = try FileHandle.null(options: [.writeAccess])
9090
try fileHandle.write([1, 2, 3, 4, 5].lazy.filter { $0 == 1 })
9191
}
9292
}
@@ -110,15 +110,15 @@ struct FileHandleTests {
110110

111111
@Test("Cannot write bytes to a read-only file")
112112
func cannotWriteBytesToReadOnlyFile() throws {
113-
let fileHandle = try FileHandle.null(mode: "rb")
113+
let fileHandle = try FileHandle.null(options: [.readAccess])
114114
#expect(throws: CError.self) {
115115
try fileHandle.write([0, 1, 2, 3, 4, 5])
116116
}
117117
}
118118

119119
@Test("Cannot write string to a read-only file")
120120
func cannotWriteStringToReadOnlyFile() throws {
121-
let fileHandle = try FileHandle.null(mode: "rb")
121+
let fileHandle = try FileHandle.null(options: [.readAccess])
122122
#expect(throws: CError.self) {
123123
try fileHandle.write("Impossible!")
124124
}
@@ -181,7 +181,7 @@ struct FileHandleTests {
181181

182182
@Test("/dev/null is not a TTY or pipe")
183183
func devNull() throws {
184-
let fileHandle = try FileHandle.null(mode: "wb")
184+
let fileHandle = try FileHandle.null(options: [.writeAccess])
185185
#expect(!Bool(fileHandle.isTTY))
186186
#if !SWT_NO_PIPES
187187
#expect(!Bool(fileHandle.isPipe))
@@ -230,11 +230,11 @@ extension FileHandle {
230230
return FileHandle(unsafeCFILEHandle: tmpFile, closeWhenDone: true)
231231
}
232232

233-
static func null(mode: String) throws -> FileHandle {
233+
static func null(options: FileHandle.OpenOptions) throws -> FileHandle {
234234
#if os(Windows)
235-
try FileHandle(atPath: "NUL", mode: mode)
235+
try FileHandle(atPath: "NUL", options: options)
236236
#else
237-
try FileHandle(atPath: "/dev/null", mode: mode)
237+
try FileHandle(atPath: "/dev/null", options: options)
238238
#endif
239239
}
240240
}

Tests/TestingTests/SwiftPMTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ struct SwiftPMTests {
223223
configuration.handleEvent(Event(.runEnded, testID: nil, testCaseID: nil), in: eventContext)
224224
}
225225

226-
let fileHandle = try FileHandle(forReadingAtPath: temporaryFilePath)
226+
let fileHandle = try Testing.FileHandle(forReadingAtPath: temporaryFilePath)
227227
let fileContents = try fileHandle.readToEnd()
228228
#expect(!fileContents.isEmpty)
229229
#expect(fileContents.contains(UInt8(ascii: "<")))
@@ -241,7 +241,7 @@ struct SwiftPMTests {
241241
_ = remove(temporaryFilePath)
242242
}
243243
do {
244-
let fileHandle = try FileHandle(forWritingAtPath: temporaryFilePath)
244+
let fileHandle = try Testing.FileHandle(forWritingAtPath: temporaryFilePath)
245245
try fileHandle.write(
246246
"""
247247
{

Tests/TestingTests/Traits/TagListTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ struct TagListTests {
143143
}
144144
"""
145145
try jsonContent.withUTF8 { jsonContent in
146-
let fileHandle = try FileHandle(forWritingAtPath: jsonPath)
146+
let fileHandle = try Testing.FileHandle(forWritingAtPath: jsonPath)
147147
try fileHandle.write(jsonContent)
148148
}
149149
defer {

0 commit comments

Comments
 (0)