Skip to content

Commit 981d23e

Browse files
throw an error for Swift tools version ≤ 5.3 when anything but a single U+0020 is used as the spacing between "//" and "swift-tools-version"
Up to and through Swift 5.3, the format of the swift tools version specification must be prefixed exactly with "// swift-tools-version:", where there is 1 and only 1 space character (U+0020) between "//" and "swift-tools-version". Even though future versions (> 5.3) will support any combination of horizontal whitespace characters for the spacing, if the specified Swift tools version ≤ 5.3, then the specification must use "// swift-tools-version:" to be backward compatible with lower Swift versions. A new capture group is added to the regex pattern, for the continuous sequence of whitespace characters between "//" and "swift-tools-version". The capture group for the version specifier remains unchanged, but becomes the 2nd capture group and the 3rd matching range. If both the whitespace sequence and the version specifier are present, and if the version specifier is valid, then compare the version with 5.3. If Swift tools version ≤ 5.3, then throw an error if the whitespace sequence contains anything but a single U+0020. The error informs the user the whitespace characters used, and informs the user that only a single U+0020 is valid. The test function `testBasics()` is renamed to `testValidVersions` to better reflect what it tests. All valid versions that use anything but a single U+0020 as spacing have their version specifier raised to above 5.3. A new test function `testBackwardCompatibilityError()` is added to verify that if Swift tools version ≤ 5.3, anything but U+0020 is rejected as the spacing between "//" and "swift-tools-version". Version specifications are moved from `testNonMatching()` to a new test function `testDefault()`, because Swift tools version defaults to 3.1 if the specification reaches a newline character before any pre-defined misspelling.
1 parent 2dfd166 commit 981d23e

File tree

2 files changed

+270
-53
lines changed

2 files changed

+270
-53
lines changed

Sources/PackageLoading/ToolsVersionLoader.swift

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol {
102102
case inaccessibleManifest(path: AbsolutePath, reason: String)
103103
/// Malformed tools version specifier
104104
case malformedToolsVersion(specifier: String, currentToolsVersion: ToolsVersion)
105+
// TODO: Make the case more general, to better adapt to future changes.
106+
/// The spacing between "//" and "swift-tools-version" either is empty or uses whitespace characters unsupported by Swift ≤ 5.3.
107+
case invalidSpacingAfterSlashes(charactersUsed: String, specifiedVersion: ToolsVersion)
105108

106109
public var description: String {
107110
switch self {
@@ -111,6 +114,17 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol {
111114
return "the package manifest at '\(manifestFile)' cannot be accessed (\(reason))"
112115
case .malformedToolsVersion(let versionSpecifier, let currentToolsVersion):
113116
return "the tools version '\(versionSpecifier)' is not valid; consider using '// swift-tools-version:\(currentToolsVersion.major).\(currentToolsVersion.minor)' to specify the current tools version"
117+
case let .invalidSpacingAfterSlashes(charactersUsed, specifiedVersion):
118+
// Tell the user what characters are currently used (invalidly) in the specification in place of U+0020.
119+
let unicodeCodePointsOfCharactersUsed: [UInt32] = charactersUsed.flatMap(\.unicodeScalars).map(\.value)
120+
let unicodeCodePointsOfCharactersUsedPrefixedByUPlus: [String] = unicodeCodePointsOfCharactersUsed.map { codePoint in
121+
var codePointString = String(codePoint, radix: 16).uppercased()
122+
if codePointString.count < 4 {
123+
codePointString = String(repeating: "0", count: 4 - codePointString.count) + codePointString
124+
}
125+
return "U+\(codePointString)"
126+
}
127+
return "\(charactersUsed.isEmpty ? "zero spacing" : "horizontal whitespace sequence [\(unicodeCodePointsOfCharactersUsedPrefixedByUPlus.joined(separator: ", "))]") between \"//\" and \"swift-tools-version\" is supported by only Swift > 5.3; consider using a single space (U+0020) for Swift \(specifiedVersion)"
114128
}
115129
}
116130
}
@@ -132,9 +146,16 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol {
132146
do { contents = try fileSystem.readFileContents(file) } catch {
133147
throw Error.inaccessibleManifest(path: file, reason: String(describing: error))
134148
}
135-
149+
150+
/// The constituent parts of the swift tools version specification found in the comment.
151+
let deconstructedToolsVersionSpecification = ToolsVersionLoader.split(contents)
152+
136153
// Get the version specifier string from tools version file.
137-
guard let versionSpecifier = ToolsVersionLoader.split(contents).versionSpecifier else {
154+
guard
155+
let spacingAfterSlashes = deconstructedToolsVersionSpecification.spacingAfterSlashes,
156+
let versionSpecifier = deconstructedToolsVersionSpecification.versionSpecifier
157+
else {
158+
// TODO: Make the diagnsosis more granular by having the regex capture more groups.
138159
// Try to diagnose if there is a misspelling of the swift-tools-version comment.
139160
let splitted = contents.contents.split(
140161
separator: UInt8(ascii: "\n"),
@@ -153,11 +174,23 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol {
153174
guard let version = ToolsVersion(string: versionSpecifier) else {
154175
throw Error.malformedToolsVersion(specifier: versionSpecifier, currentToolsVersion: currentToolsVersion)
155176
}
177+
178+
// Ensure that for Swift ≤ 5.3, a single U+0020 is used as spacing between "//" and "swift-tools-version".
179+
// Use `ToolsVersion(version:)` instead of `ToolsVeresion(string:)` here to avoid forced unwrappiing.
180+
guard spacingAfterSlashes == " " || version > ToolsVersion(version: "5.3.0") else {
181+
throw Error.invalidSpacingAfterSlashes(charactersUsed: spacingAfterSlashes, specifiedVersion: version)
182+
}
183+
156184
return version
157185
}
158186

159-
/// Splits the bytes to the version specifier (if present) and rest of the contents.
160-
public static func split(_ bytes: ByteString) -> (versionSpecifier: String?, rest: [UInt8]) {
187+
/// Splits the bytes to constituent parts of the swift tools version specification and rest of the contents.
188+
///
189+
/// The constituent parts include the spacing between "//" and "swift-tools-version", and the version specifier, if either is present.
190+
///
191+
/// - Parameter bytes: The raw bytes of the content of `Package.swift`.
192+
/// - Returns: The spacing between "//" and "swift-tools-version" (if present, or `nil`), the version specifier (if present, or `nil`), and of raw bytes of the rest of the content of `Package.swift`.
193+
public static func split(_ bytes: ByteString) -> (spacingAfterSlashes: String?, versionSpecifier: String?, rest: [UInt8]) {
161194
let splitted = bytes.contents.split(
162195
separator: UInt8(ascii: "\n"),
163196
maxSplits: 1,
@@ -166,19 +199,32 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol {
166199
guard let firstLine = ByteString(splitted[0]).validDescription,
167200
let match = ToolsVersionLoader.regex.firstMatch(
168201
in: firstLine, options: [], range: NSRange(location: 0, length: firstLine.count)),
169-
match.numberOfRanges >= 2 else {
170-
return (nil, bytes.contents)
202+
// The 3 ranges are:
203+
// 1. The entire matched string.
204+
// 2. Capture group 1: the comment spacing.
205+
// 3. Capture group 2: The version specifier.
206+
// Since the version specifier is in the last range, if the number of ranges is less than 3, then no version specifier is captured by the regex.
207+
// FIXME: Should this be `== 3` instead?
208+
match.numberOfRanges >= 3 else {
209+
return (nil, nil, bytes.contents)
171210
}
172-
let versionSpecifier = NSString(string: firstLine).substring(with: match.range(at: 1))
211+
let spacingAfterSlashes = NSString(string: firstLine).substring(with: match.range(at: 1))
212+
let versionSpecifier = NSString(string: firstLine).substring(with: match.range(at: 2))
173213
// FIXME: We can probably optimize here and return array slice.
174-
return (versionSpecifier, splitted.count == 1 ? [] : Array(splitted[1]))
214+
return (spacingAfterSlashes, versionSpecifier, splitted.count == 1 ? [] : Array(splitted[1]))
175215
}
176216

177217
/// The regex to match swift tools version specification.
218+
///
219+
/// The specification must have the following format:
178220
/// - It should start with `//` followed by any amount of _horizontal_ whitespace characters.
179221
/// - Following that it should contain the case insensitive string `swift-tools-version:`.
180222
/// - The text between the above string and `;` or string end becomes the tools version specifier.
223+
///
224+
/// There are 2 capture groups in the regex pattern:
225+
/// 1. The continuous sequence of whitespace characters between "//" and "swift-tools-version".
226+
/// 2. The version specifier.
181227
static let regex = try! NSRegularExpression(
182-
pattern: "^//\\h*?swift-tools-version:(.*?)(?:;.*|$)",
228+
pattern: "^//(\\h*?)swift-tools-version:(.*?)(?:;.*|$)",
183229
options: [.caseInsensitive])
184230
}

0 commit comments

Comments
 (0)