From 857655affeb263181f9e56c157a4759221fc4756 Mon Sep 17 00:00:00 2001 From: Wowbagger's Liquid Lunch <55120045+WowbaggersLiquidLunch@users.noreply.github.com> Date: Wed, 4 Nov 2020 05:26:22 -0500 Subject: [PATCH] adapt `writeToolsVersion(at:version:fs:)` to using the new `ToolsVersionLoader.split(_:)` This allows the old `ToolsVersionLoader.split(_:)` and `regex` to be fully removed instead of just deprecated. In implementing this change, the logic of the new `ToolsVersionLoader.split(_:)` is updated for the following 2 purposes: 1. Primarily to partially match the old `split(_:)`'s behaviour in deciding if the label of the Swift tools version specification is malformed. 2. Secondarily to provide a special path of diagnosis for when the label is prefixed with "swift-tools-version:" (case-insensitive). The comments left in the source provides much better details. --- CHANGELOG.md | 4 +- .../PackageLoading/ToolsVersionLoader.swift | 159 +++++++++--------- Sources/Workspace/ToolsVersionWriter.swift | 29 ++-- .../ToolsVersionLoaderTests.swift | 5 +- 4 files changed, 104 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f652e87e03..31174b28403 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,9 @@ Swift 5.3.1 * API Removal - `ToolsVersionLoader.Error.malformedToolsVersion(specifier: String, currentToolsVersion: ToolsVersion)` is now obsoleted, and replaced by `ToolsVersionLoader.Error.malformedToolsVersionSpecification(_ malformation: ToolsVersionSpecificationMalformation)`. + `ToolsVersionLoader.Error.malformedToolsVersion(specifier: String, currentToolsVersion: ToolsVersion)` is replaced by `ToolsVersionLoader.Error.malformedToolsVersionSpecification(_ malformation: ToolsVersionSpecificationMalformation)`. + + `ToolsVersionLoader.split(_ bytes: ByteString) -> (versionSpecifier: String?, rest: [UInt8])` and `ToolsVersionLoader.regex` are together replaced by `ToolsVersionLoader.split(_ manifest: String) -> ManifestComponents`. * Source Breakages for Swift Packages diff --git a/Sources/PackageLoading/ToolsVersionLoader.swift b/Sources/PackageLoading/ToolsVersionLoader.swift index 0cc160f43fa..2f0367e2699 100644 --- a/Sources/PackageLoading/ToolsVersionLoader.swift +++ b/Sources/PackageLoading/ToolsVersionLoader.swift @@ -124,6 +124,8 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol { /// The label part of the Swift tools version specification is malformed. case label(_ malformationDetails: MalformationDetails) /// The version specifier is malformed. + /// + /// If the version specifier is diagnosed as missing, it could be a misdiagnosis due to some misspellings in the label. This is because of a compromise made in `ToolsVersionLoader.split(_:)`. case versionSpecifier(_ malformationDetails: MalformationDetails) /// An unidentifiable component of the Swift tools version specification is malformed. case unidentified @@ -201,8 +203,7 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol { case .versionSpecifier(let versionSpecifier): switch versionSpecifier { case .isMissing: - // If the version specifier is missing, then its terminator must be missing as well. So, there is nothing in between the version specifier and everything that should be in front the version specifier. So, appending a valid version specifier will fix this error. - return "the Swift tools version specification is missing a version specifier; consider appending '\(ToolsVersion.currentToolsVersion)' to the line to specify the current Swift toolchain version as the lowest Swift version supported by the project" + return "the Swift tools version specification is possibly missing a version specifier; consider using 'swift-tools-version:\(ToolsVersion.currentToolsVersion)' to specify the current Swift toolchain version as the lowest Swift version supported by the project" case .isMisspelt(let misspeltVersionSpecifier): return "the Swift tools version '\(misspeltVersionSpecifier)' is misspelt or otherwise invalid; consider replacing it with '\(ToolsVersion.currentToolsVersion)' to specify the current Swift toolchain version as the lowest Swift version supported by the project" } @@ -426,45 +427,6 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol { return version } - // FIXME: Remove this function. - // This function is currently preserved because it's used in `writeToolsVersion(at:version:fs:). - /// Splits the bytes to find the Swift tools version specifier and the contents sans the first line of the manifest. - /// - /// - Warning: This function has been deprecated since Swift 5.3.1, please use `split(_ manifestContents: String) -> ManifestComponents` instead. - /// - /// - Note: This function imposes the following limitations that are removed in its replacement: - /// - Leading whitespace, other than a sequence of newline characters (`U+000A`), is not accepted in the given manifest contents. - /// - Only `U+000A` is recognised as a line terminator. - /// - A Swift tools version specification must be prefixed with `// swift-tools-version:` verbatim, where the spacing between `//` and `swift-tools-version` is exactly 1 `U+0020`. - /// - /// - Bug: This function treats only `U+000A` as a line terminator. - /// - Bug: If there is a contiguous sequence of `U+000A` at the very beginning of the manifest file, this function mistakes the first non-empty line of the manifest as its first line. - /// - /// - Parameter bytes: The raw bytes of the content of the manifest. - /// - Returns: The version specifier (if present, or `nil`) and the raw bytes of the contents sans the first line of the manifest. - @available(swift, deprecated: 5.3.1, renamed: "ToolsVersionLoader.split(_:)") - public static func split(_ bytes: ByteString) -> (versionSpecifier: String?, rest: [UInt8]) { - let splitted = bytes.contents.split( - separator: UInt8(ascii: "\n"), - maxSplits: 1, - omittingEmptySubsequences: false) - // Try to match our regex and see if the "first" line is a valid tools version specification line. - guard let firstLine = ByteString(splitted[0]).validDescription, - let match = ToolsVersionLoader.regex.firstMatch( - in: firstLine, options: [], range: NSRange(location: 0, length: firstLine.count)), - // The 2 ranges are: - // 1. The entire matched string. - // 2. Capture group 1: The version specifier. - // Since the version specifier is in the last range, if the number of ranges is less than 2, then no version specifier is captured by the regex. - // FIXME: Should this be `== 2` instead? - match.numberOfRanges >= 2 else { - return (nil, bytes.contents) - } - let versionSpecifier = NSString(string: firstLine).substring(with: match.range(at: 1)) - // FIXME: We can probably optimize here and return array slice. - return (versionSpecifier, splitted.count == 1 ? [] : Array(splitted[1])) - } - /// Splits the given manifest into its constituent components. /// /// A manifest consists of the following parts: @@ -535,42 +497,95 @@ public class ToolsVersionLoader: ToolsVersionLoaderProtocol { /// The position right past the last character of the spacing that immediately follows the comment marker. /// /// Because the spacing consists of only horizontal whitespace characters, this position is the same as the first character that's not a horizontal whitespace after `commentMarker`. If no such character exists, then the position is the `endIndex` of the Swift tools version specification line. - let endIndexOfSpacingAfterCommentMarker = specificationWithIgnoredTrailingContents[endIndexOfCommentMarker...].firstIndex(where: { !$0.isWhitespace } ) ?? specificationWithIgnoredTrailingContents.endIndex - // ☝️ + let startIndexOfLabel = specificationWithIgnoredTrailingContents[endIndexOfCommentMarker...].firstIndex(where: { !$0.isWhitespace } ) ?? specificationWithIgnoredTrailingContents.endIndex + // ☝️ // Technically, this is looking for the position of the first character that's not a whitespace, BOTH HORIZONTAL AND VERTICAL. However, since all vertical horizontal whitespace characters are also line terminators, and because the Swift tools version specification does not contain any line terminator, we can safely use `Character.isWhitespace` to check if a character is a horizontal whitespace. /// The spacing that immediately follows `commentMarker`. /// /// The spacing consists of only horizontal whitespace characters. - let spacingAfterCommentMarker = specificationWithIgnoredTrailingContents[endIndexOfCommentMarker..