Apply review feedback

Author: milseman

Sources/System/FilePath/FilePathComponents.swift

@@ -199,8 +199,6 @@ extension FilePath: _PlatformStringable {
 
 }
 
-// TODO(root): Re-think these initializers. We want them to be conv
-
 extension FilePath.Component {
   // The index of the `.` denoting an extension
   internal func _extensionIndex() -> SystemString.Index? {

Sources/System/FilePath/FilePathParsing.swift

@@ -158,8 +158,6 @@ extension FilePath {
         if writeIdx != relStart {
           let priorComponent = _parseComponent(priorTo: writeIdx)
 
-          // FIXME: it's not up until writeIdx because separator,
-          // parseComponent should give the component...
           if !_isParentDirectory(priorComponent) {
             writeIdx = priorComponent.lowerBound
             readIdx = nextStart
@@ -182,8 +180,6 @@ extension FilePath {
 
     assert(readIdx == _storage.endIndex && readIdx >= writeIdx)
     if readIdx != writeIdx {
-      // TODO: Why was it everything after writeIdx?
-//      storage.removeSubrange(storage.index(after: writeIdx)...)
       _storage.removeSubrange(writeIdx...)
       _removeTrailingSeparator()
     }
@@ -275,24 +271,6 @@ extension FilePath.ComponentView {
   internal var _relativeStart: SystemString.Index {
     _path._relativeStart
   }
-
-  internal func parseComponentStart(
-    endingAt i: SystemString.Index
-  ) -> SystemString.Index {
-    if i == _relativeStart, i != _path._storage.startIndex {
-      return _path._storage.startIndex
-    }
-    var i = i
-    if i != _path._storage.endIndex {
-      assert(isSeparator(_path._storage[i]))
-      i = _path._storage.index(before: i)
-    }
-    var slice = _path._storage[..<i]
-    while let c = slice.last, !isSeparator(c) {
-      slice.removeLast()
-    }
-    return slice.endIndex
-  }
 }
 
 extension SystemString {
@@ -370,7 +348,7 @@ extension FilePath {
   }
 
   // Perform an append, inseting a separator if needed.
-  // Note tha this will not check whether `content` is a root
+  // Note that this will not check whether `content` is a root
   internal mutating func _append(unchecked content: Slice<SystemString>) {
     assert(FilePath(SystemString(content)).root == nil)
     if content.isEmpty { return }

Sources/System/FilePath/FilePathSyntax.swift

@@ -305,13 +305,13 @@ extension FilePath {
   /// replace the extension.
   ///
   /// Examples:
-  ///   * `/tmp/foo.txt                 => txt`
-  ///   * `/Appliations/Foo.app/        => app`
-  ///   * `/Appliations/Foo.app/bar.txt => txt`
-  ///   * `/tmp/foo.tar.gz              => gz`
-  ///   * `/tmp/.hidden                 => nil`
-  ///   * `/tmp/.hidden.                => ""`
-  ///   * `/tmp/..                      => nil`
+  ///   * `/tmp/foo.txt                  => txt`
+  ///   * `/Applications/Foo.app/        => app`
+  ///   * `/Applications/Foo.app/bar.txt => txt`
+  ///   * `/tmp/foo.tar.gz               => gz`
+  ///   * `/tmp/.hidden                  => nil`
+  ///   * `/tmp/.hidden.                 => ""`
+  ///   * `/tmp/..                       => nil`
   ///
   /// Example:
   ///
@@ -391,12 +391,10 @@ extension FilePath {
   /// Returns a copy of `self` in lexical-normal form, that is `.` and `..`
   /// components have been collapsed lexically (i.e. without following
   /// symlinks). See `lexicallyNormalize`
-  public var lexicallyNormal: FilePath {
-    __consuming get {
-      var copy = self
-      copy.lexicallyNormalize()
-      return copy
-    }
+  public __consuming func lexicallyNormalized() -> FilePath {
+    var copy = self
+    copy.lexicallyNormalize()
+    return copy
   }
 
   /// Create a new `FilePath` by resolving `subpath` relative to `self`,
@@ -428,7 +426,7 @@ extension FilePath {
   public __consuming func lexicallyResolving(
     _ subpath: __owned FilePath
   ) -> FilePath? {
-    let subpath = subpath.removingRoot().lexicallyNormal
+    let subpath = subpath.removingRoot().lexicallyNormalized()
     guard !subpath.isEmpty else { return self }
     guard subpath.components.first?.kind != .parentDirectory else {
       return nil

Tests/SystemTests/FilePathTests/FilePathExtras.swift

@@ -50,7 +50,8 @@ extension FilePath {
     guard !other.isEmpty else { return true }
     guard !isEmpty else { return false }
 
-    let (selfLex, otherLex) = (self.lexicallyNormal, other.lexicallyNormal)
+    let (selfLex, otherLex) =
+      (self.lexicallyNormalized(), other.lexicallyNormalized())
     if otherLex.isAbsolute { return selfLex.starts(with: otherLex) }
 
     // FIXME: Windows semantics with relative roots?

Tests/SystemTests/FilePathTests/FilePathSyntaxTest.swift

@@ -243,7 +243,7 @@ extension SyntaxTestCase {
       copy.lexicallyNormalize()
       expectEqual(copy == path, path.isLexicallyNormal, "isLexicallyNormal")
       expectEqual(lexicallyNormalized, copy.description, "lexically normalized")
-      expectEqual(copy, path.lexicallyNormal, "lexicallyNormal")
+      expectEqual(copy, path.lexicallyNormalized(), "lexicallyNormal")
 
       expectEqual(absolute, path.isAbsolute, "absolute")
       expectEqual(!absolute, path.isRelative, "!absolute")