Code cleanup; FilePath separator normalization

Sources/System/FilePath.swift → Sources/System/FilePath/FilePath.swift

@@ -7,31 +7,22 @@
  See https://swift.org/LICENSE.txt for license information
 */
 
-extension UnsafePointer where Pointee == UInt8 {
-  internal var _asCChar: UnsafePointer<CChar> {
-    UnsafeRawPointer(self).assumingMemoryBound(to: CChar.self)
-  }
-}
-extension UnsafePointer where Pointee == CChar {
-  internal var _asUInt8: UnsafePointer<UInt8> {
-    UnsafeRawPointer(self).assumingMemoryBound(to: UInt8.self)
-  }
-}
-extension UnsafeBufferPointer where Element == UInt8 {
-  internal var _asCChar: UnsafeBufferPointer<CChar> {
-    let base = baseAddress?._asCChar
-    return UnsafeBufferPointer<CChar>(start: base, count: self.count)
-  }
-}
-extension UnsafeBufferPointer where Element == CChar {
-  internal var _asUInt8: UnsafeBufferPointer<UInt8> {
-    let base = baseAddress?._asUInt8
-    return UnsafeBufferPointer<UInt8>(start: base, count: self.count)
-  }
-}
-
 // NOTE: FilePath not frozen for ABI flexibility
 
+// A platform-native string representation, for file paths
+#if os(Windows)
+  internal typealias SystemString = [UInt16]
+#else
+  internal typealias SystemString = [CChar]
+#endif
+
+// TODO: Adjust comment header to de-emphasize the null-terminated
+// bytes part. This is a type that represents a location in the file
+// system, and can vend null-terminated bytes. Also, we will be
+// normalizing the separator representation, so it needs to pretty
+// much be re-written.
+
+
 /// A null-terminated sequence of bytes
 /// that represents a location in the file system.
 ///
@@ -59,13 +50,24 @@ extension UnsafeBufferPointer where Element == CChar {
 /// like case insensitivity, Unicode normalization, and symbolic links.
 // @available(macOS 10.16, iOS 14.0, watchOS 7.0, tvOS 14.0, *)
 public struct FilePath {
-  internal var bytes: [CChar]
+  internal typealias Storage = SystemString
+
+  internal var bytes: Storage
 
   /// Creates an empty, null-terminated path.
   public init() {
     self.bytes = [0]
     _invariantCheck()
   }
+
+  // In addition to the empty init, this init will properly normalize
+  // separators. All other initializers should be implemented by
+  // ultimately deferring to a normalizing init.
+  internal init<C: Collection>(nullTerminatedBytes: C) where C.Element == CChar {
+    self.bytes = Array(nullTerminatedBytes)
+    self._normalizeSeparators()
+    _invariantCheck()
+  }
 }
 
 //
@@ -79,12 +81,7 @@ extension FilePath {
 
 // @available(macOS 10.16, iOS 14.0, watchOS 7.0, tvOS 14.0, *)
 extension FilePath {
-  internal init<C: Collection>(nullTerminatedBytes: C) where C.Element == CChar {
-    self.bytes = Array(nullTerminatedBytes)
-    _invariantCheck()
-  }
-
-  internal init<C: Collection>(byteContents bytes: C) where C.Element == CChar {
+  fileprivate init<C: Collection>(byteContents bytes: C) where C.Element == CChar {
     var nulTermBytes = Array(bytes)
     nulTermBytes.append(0)
     self.init(nullTerminatedBytes: nulTermBytes)
@@ -129,6 +126,11 @@ extension FilePath {
   // byte contents and unterminated byte contents.
 }
 
+#if os(Windows)
+  // TODO: wchar_t* initializers, interfaces, etc.
+  // TODO: Consider eventually doing OSString
+#endif
+
 //
 // MARK: - String interfaces
 //
@@ -186,11 +188,16 @@ extension String {
     }
     self = str
   }
+
+  // TODO: Consider a init?(validating:), keeping the encoding agnostic in API and
+  // dependent on file system.
 }
 
 // @available(macOS 10.16, iOS 14.0, watchOS 7.0, tvOS 14.0, *)
 extension FilePath: CustomStringConvertible, CustomDebugStringConvertible {
-  /// A textual representation of the file path.
+  /// A textual representation of the file path using the platform's preferred separator.
+  ///
+  /// On Unix, the preferred separator is `/`. On Windows, the preferred separator is `\`
   @inline(never)
   public var description: String { String(decoding: self) }
 
@@ -201,10 +208,3 @@ extension FilePath: CustomStringConvertible, CustomDebugStringConvertible {
 // @available(macOS 10.16, iOS 14.0, watchOS 7.0, tvOS 14.0, *)
 extension FilePath: Hashable, Codable {}
 
-// @available(macOS 10.16, iOS 14.0, watchOS 7.0, tvOS 14.0, *)
-extension FilePath {
-  fileprivate func _invariantCheck() {
-    precondition(bytes.last! == 0)
-    _debugPrecondition(bytes.firstIndex(of: 0) == bytes.count - 1)
-  }
-}

Sources/System/FilePath/FilePathParsing.swift

@@ -0,0 +1,108 @@
+/*
+ This source file is part of the Swift System open source project
+
+ Copyright (c) 2020 Apple Inc. and the Swift System project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See https://swift.org/LICENSE.txt for license information
+*/
+
+private typealias SystemChar = SystemString.Element
+
+// The separator we use internally
+private var genericSeparator: SystemChar {
+  numericCast(UInt8(ascii: "/"))
+}
+
+// The platform preferred separator
+//
+// TODO: Make private
+private var platformSeparator: SystemChar {
+#if os(Windows)
+  return numericCast(UInt8(ascii: "\\"))
+#else
+  return genericSeparator
+#endif
+}
+
+private func isSeparator(_ c: SystemChar) -> Bool {
+  c == genericSeparator || c == platformSeparator
+}
+
+extension FilePath {
+  // For invariant enforcing/checking. Should always return `nil` on
+  // a fully-formed path
+  private func _trailingSepIdx() -> Storage.Index? {
+    guard bytes.count > 2 else { return nil }
+    let idx = bytes.index(bytes.endIndex, offsetBy: -2)
+    return isSeparator(bytes[idx]) ? idx : nil
+  }
+
+  // Enforce invariants by removing a trailing separator.
+  //
+  // Precondition: There is exactly zero or one trailing slashes
+  //
+  // Postcondition: Path is root, or has no trailing separator
+  private mutating func _removeTrailingSeparator() {
+    assert(bytes.last == 0, "even empty paths have null termiantor")
+    defer { assert(_trailingSepIdx() == nil) }
+
+    guard let sepIdx = _trailingSepIdx() else { return }
+    bytes.remove(at: sepIdx)
+  }
+
+  // Enforce invariants by normalizing the internal separator representation.
+  //
+  // 1) Normalize all separators to platform-preferred separator
+  // 2) Drop redundant separators
+  // 3) Drop trailing separators
+  //
+  // On Windows, UNC and device paths are allowed to begin with two separators
+  //
+  // The POSIX standard does allow two leading separators to
+  // denote implementation-specific handling, but Darwin and Linux
+  // do not treat these differently.
+  //
+  internal mutating func _normalizeSeparators() {
+    var (writeIdx, readIdx) = (bytes.startIndex, bytes.startIndex)
+    #if os(Windows)
+    // TODO: skip over two leading backslashes
+    #endif
+
+    while readIdx < bytes.endIndex {
+      assert(writeIdx <= readIdx)
+
+      let wasSeparator = isSeparator(bytes[readIdx])
+      if wasSeparator {
+        bytes[readIdx] = platformSeparator
+      }
+      bytes.swapAt(writeIdx, readIdx)
+      bytes.formIndex(after: &writeIdx)
+      bytes.formIndex(after: &readIdx)
+
+      if readIdx == bytes.endIndex { break }
+
+      while wasSeparator && isSeparator(bytes[readIdx]) {
+        bytes.formIndex(after: &readIdx)
+        if readIdx == bytes.endIndex { break }
+      }
+    }
+    bytes.removeLast(bytes.distance(from: writeIdx, to: readIdx))
+    self._removeTrailingSeparator()
+  }
+
+  internal func _invariantCheck() {
+  #if DEBUG
+    precondition(bytes.last! == 0)
+
+    // TODO: Should this be a hard trap unconditionally??
+    precondition(bytes.firstIndex(of: 0) == bytes.count - 1)
+
+    var normal = self
+    normal._normalizeSeparators()
+    precondition(self == normal)
+    precondition(self._trailingSepIdx() == nil)
+
+  #endif // DEBUG
+  }
+}

Sources/System/Util.swift

@@ -94,3 +94,27 @@ extension OptionSet {
     return result
   }
 }
+
+extension UnsafePointer where Pointee == UInt8 {
+  internal var _asCChar: UnsafePointer<CChar> {
+    UnsafeRawPointer(self).assumingMemoryBound(to: CChar.self)
+  }
+}
+extension UnsafePointer where Pointee == CChar {
+  internal var _asUInt8: UnsafePointer<UInt8> {
+    UnsafeRawPointer(self).assumingMemoryBound(to: UInt8.self)
+  }
+}
+extension UnsafeBufferPointer where Element == UInt8 {
+  internal var _asCChar: UnsafeBufferPointer<CChar> {
+    let base = baseAddress?._asCChar
+    return UnsafeBufferPointer<CChar>(start: base, count: self.count)
+  }
+}
+extension UnsafeBufferPointer where Element == CChar {
+  internal var _asUInt8: UnsafeBufferPointer<UInt8> {
+    let base = baseAddress?._asUInt8
+    return UnsafeBufferPointer<UInt8>(start: base, count: self.count)
+  }
+}
+

Tests/SystemTests/TestingInfrastructure.swift

@@ -21,15 +21,15 @@ internal protocol TestCase {
 }
 extension TestCase {
   func expectEqualSequence<S1: Sequence, S2: Sequence>(
-    _ actual: S1, _ expected: S2,
+    _ expected: S1, _ actual: S2,
     _ message: String? = nil
   ) where S1.Element: Equatable, S1.Element == S2.Element {
     if !actual.elementsEqual(expected) {
       fail(message)
     }
   }
   func expectEqual<E: Equatable>(
-    _ actual: E, _ expected: E,
+    _ expected: E, _ actual: E,
     _ message: String? = nil
   ) {
     if actual != expected {
@@ -88,7 +88,7 @@ internal struct MockTestCase: TestCase {
       // Test our API mappings to the lower-level syscall invocation
       do {
         try body(true)
-        self.expectEqual(mocking.trace.dequeue(), self.expected)
+        self.expectEqual(self.expected, mocking.trace.dequeue())
       } catch {
         self.fail()
       }
@@ -103,7 +103,7 @@ internal struct MockTestCase: TestCase {
         self.fail()
       } catch Errno.interrupted {
         // Success!
-        self.expectEqual(mocking.trace.dequeue(), self.expected)
+        self.expectEqual(self.expected, mocking.trace.dequeue())
       } catch {
         self.fail()
       }
@@ -114,13 +114,13 @@ internal struct MockTestCase: TestCase {
         mocking.forceErrno = .counted(errno: EINTR, count: 3)
 
         try body(interruptable)
-        self.expectEqual(mocking.trace.dequeue(), self.expected) // EINTR
-        self.expectEqual(mocking.trace.dequeue(), self.expected) // EINTR
-        self.expectEqual(mocking.trace.dequeue(), self.expected) // EINTR
-        self.expectEqual(mocking.trace.dequeue(), self.expected) // Success
+        self.expectEqual(self.expected, mocking.trace.dequeue()) // EINTR
+        self.expectEqual(self.expected, mocking.trace.dequeue()) // EINTR
+        self.expectEqual(self.expected, mocking.trace.dequeue()) // EINTR
+        self.expectEqual(self.expected, mocking.trace.dequeue()) // Success
       } catch Errno.interrupted {
         self.expectFalse(interruptable)
-        self.expectEqual(mocking.trace.dequeue(), self.expected) // EINTR
+        self.expectEqual(self.expected, mocking.trace.dequeue()) // EINTR
       } catch {
         self.fail()
       }