Moved // FIXME: lines that broke doc comments to be above docs

Author: bscothern

Sources/Basic/DiagnosticsEngine.swift

@@ -67,6 +67,11 @@ public func <<< <T>(
     return builder
 }
 
+// FIXME: One thing we should consider is whether we should make the diagnostic
+// a protocol and put these properties on the type, which is conceptually the
+// right modeling, but might be cumbersome of our desired features don't
+// perfectly align with what the language can express.
+//
 /// A unique identifier for a diagnostic.
 ///
 /// Diagnostic identifiers are intended to be a stable representation of a
@@ -76,12 +81,6 @@ public func <<< <T>(
 /// (e.g., a command line tool, an IDE, or a web UI) is expecting to receive
 /// diagnostics and be able to present additional UI affordances or workflows
 /// associated for specific kinds of diagnostics.
-///
-//
-// FIXME: One thing we should consider is whether we should make the diagnostic
-// a protocol and put these properties on the type, which is conceptually the
-// right modeling, but might be cumbersome of our desired features don't
-// perfectly align with what the language can express.
 public class DiagnosticID: ObjectIdentifierProtocol {
 
     /// A piece of a diagnostic description.

Sources/Basic/FileSystem.swift

@@ -108,15 +108,15 @@ public enum FileMode {
     }
 }
 
+// FIXME: Design an asynchronous story?
+//
 /// Abstracted access to file system operations.
 ///
 /// This protocol is used to allow most of the codebase to interact with a
 /// natural filesystem interface, while still allowing clients to transparently
 /// substitute a virtual file system or redirect file system operations.
 ///
 /// NOTE: All of these APIs are synchronous and can block.
-//
-// FIXME: Design an asynchronous story?
 public protocol FileSystem: class {
     /// Check whether the given path exists and is accessible.
     func exists(_ path: AbsolutePath, followSymlink: Bool) -> Bool
@@ -133,10 +133,10 @@ public protocol FileSystem: class {
     /// Check whether the given path is accessible and is a symbolic link.
     func isSymlink(_ path: AbsolutePath) -> Bool
 
-    /// Get the contents of the given directory, in an undefined order.
-    //
     // FIXME: Actual file system interfaces will allow more efficient access to
     // more data than just the name here.
+    //
+    /// Get the contents of the given directory, in an undefined order.
     func getDirectoryContents(_ path: AbsolutePath) throws -> [String]
 
     /// Get the current working directory (similar to `getcwd(3)`), which can be
@@ -157,21 +157,21 @@ public protocol FileSystem: class {
     /// - recursive: If true, create missing parent directories if possible.
     func createDirectory(_ path: AbsolutePath, recursive: Bool) throws
 
+    // FIXME: This is obviously not a very efficient or flexible API.
+    //
     /// Get the contents of a file.
     ///
     /// - Returns: The file contents as bytes, or nil if missing.
-    //
-    // FIXME: This is obviously not a very efficient or flexible API.
     func readFileContents(_ path: AbsolutePath) throws -> ByteString
 
-    /// Write the contents of a file.
-    //
     // FIXME: This is obviously not a very efficient or flexible API.
+    //
+    /// Write the contents of a file.
     func writeFileContents(_ path: AbsolutePath, bytes: ByteString) throws
 
-    /// Write the contents of a file.
-    //
     // FIXME: This is obviously not a very efficient or flexible API.
+    //
+    /// Write the contents of a file.
     func writeFileContents(_ path: AbsolutePath, bytes: ByteString, atomically: Bool) throws
 
     /// Recursively deletes the file system entity at `path`.
@@ -511,9 +511,9 @@ private class LocalFileSystem: FileSystem {
     }
 }
 
-/// Concrete FileSystem implementation which simulates an empty disk.
-//
 // FIXME: This class does not yet support concurrent mutation safely.
+//
+/// Concrete FileSystem implementation which simulates an empty disk.
 public class InMemoryFileSystem: FileSystem {
     private class Node {
         /// The actual node data.

Sources/Basic/LazyCache.swift

@@ -8,6 +8,9 @@
  See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 */
 
+// FIXME: This wrapper could benefit from local static variables, in which case
+// we could embed the cache object inside the accessor.
+//
 /// Thread-safe lazily cached methods.
 ///
 /// The `lazy` annotation in Swift does not result in a thread-safe accessor,
@@ -25,9 +28,6 @@
 /// ```
 ///
 /// See: https://bugs.swift.org/browse/SR-1042
-//
-// FIXME: This wrapper could benefit from local static variables, in which case
-// we could embed the cache object inside the accessor.
 public struct LazyCache<Class, T> {
     // FIXME: It would be nice to avoid a per-instance lock, but this type isn't
     // intended for creating large numbers of instances of. We also really want

Sources/Basic/OutputByteStream.swift

@@ -537,9 +537,10 @@ public struct Format {
 /// Inmemory implementation of OutputByteStream.
 public final class BufferedOutputByteStream: _OutputByteStreamBase {
 
-    /// Contents of the stream.
     // FIXME: For inmemory implementation we should be share this buffer with OutputByteStream.
     // One way to do this is by allowing OuputByteStream to install external buffers.
+    //
+    /// Contents of the stream.
     private var contents = [UInt8]()
 
     override public init() {

Sources/Basic/Path.swift

@@ -205,13 +205,14 @@ public struct AbsolutePath: Hashable {
         return _impl.string
     }
 
+    // FIXME: We should investigate if it would be more efficient to instead
+    // return a path component iterator that does all its work lazily, moving
+    // from one path separator to the next on-demand.
+    //
     /// Returns an array of strings that make up the path components of the
     /// absolute path.  This is the same sequence of strings as the basenames
     /// of each successive path component, starting from the root.  Therefore
     /// the first path component of an absolute path is always `/`.
-    // FIXME: We should investigate if it would be more efficient to instead
-    // return a path component iterator that does all its work lazily, moving
-    // from one path separator to the next on-demand.
     public var components: [String] {
         // FIXME: This isn't particularly efficient; needs optimization, and
         // in fact, it might well be best to return a custom iterator so we
@@ -292,14 +293,15 @@ public struct RelativePath: Hashable {
         return _impl.string
     }
 
+    // FIXME: We should investigate if it would be more efficient to instead
+    // return a path component iterator that does all its work lazily, moving
+    // from one path separator to the next on-demand.
+    //
     /// Returns an array of strings that make up the path components of the
     /// relative path.  This is the same sequence of strings as the basenames
     /// of each successive path component.  Therefore the returned array of
     /// path components is never empty; even an empty path has a single path
     /// component: the `.` string.
-    // FIXME: We should investigate if it would be more efficient to instead
-    // return a path component iterator that does all its work lazily, moving
-    // from one path separator to the next on-demand.
     public var components: [String] {
         // FIXME: This isn't particularly efficient; needs optimization, and
         // in fact, it might well be best to return a custom iterator so we

Sources/Basic/TemporaryFile.swift

@@ -17,8 +17,9 @@ public enum TempFileError: Swift.Error {
     /// Could not create a unique temporary filename.
     case couldNotCreateUniqueName
 
-    /// Some error thrown defined by posix's open().
     // FIXME: This should be factored out into a open error enum.
+    //
+    /// Some error thrown defined by posix's open().
     case other(Int32)
 
     /// Couldn't find a temporary directory.
@@ -131,9 +132,9 @@ extension TemporaryFile: CustomStringConvertible {
     }
 }
 
-/// Contains the error which can be thrown while creating a directory using POSIX's mkdir.
-//
 // FIXME: This isn't right place to declare this, probably POSIX or merge with FileSystemError?
+//
+/// Contains the error which can be thrown while creating a directory using POSIX's mkdir.
 public enum MakeDirectoryError: Swift.Error {
     /// The given path already exists as a directory, file or symbolic link.
     case pathExists

Sources/Build/BuildPlan.swift

@@ -17,8 +17,9 @@ import func POSIX.getenv
 
 public struct BuildParameters {
 
-    /// Path to the module cache directory to use for SwiftPM's own tests.
     // FIXME: Error handling.
+    //
+    /// Path to the module cache directory to use for SwiftPM's own tests.
     public static let swiftpmTestCache = resolveSymlinks(try! determineTempDirectory()).appending(component: "org.swift.swiftpm.tests-3")
 
     /// Returns the directory to be used for module cache.

Sources/PackageDescription4/JSON.swift

@@ -20,8 +20,9 @@ enum JSON {
 }
 
 extension JSON {
-    /// Converts the JSON to string representation.
     // FIXME: No escaping implemented for now.
+    //
+    /// Converts the JSON to string representation.
     func toString() -> String {
         switch self {
         case .null:

Sources/PackageGraph/DependencyResolver.swift

@@ -226,6 +226,9 @@ public protocol PackageContainer {
     /// if the previous one did not satisfy all constraints.
     func versions(filter isIncluded: (Version) -> Bool) -> AnySequence<Version>
 
+    // FIXME: We should perhaps define some particularly useful error codes
+    // here, so the resolver can handle errors more meaningfully.
+    //
     /// Fetch the declared dependencies for a particular version.
     ///
     /// This property is expected to be efficient to access, and cached by the
@@ -234,9 +237,6 @@ public protocol PackageContainer {
     /// - Precondition: `versions.contains(version)`
     /// - Throws: If the version could not be resolved; this will abort
     ///   dependency resolution completely.
-    //
-    // FIXME: We should perhaps define some particularly useful error codes
-    // here, so the resolver can handle errors more meaningfully.
     func getDependencies(at version: Version) throws -> [PackageContainerConstraint<Identifier>]
 
     /// Fetch the declared dependencies for a particular revision.
@@ -330,9 +330,9 @@ public protocol DependencyResolverDelegate {
     associatedtype Identifier: PackageContainerIdentifier
 }
 
-/// A bound version for a package within an assignment.
-//
 // FIXME: This should be nested, but cannot be currently.
+//
+/// A bound version for a package within an assignment.
 public enum BoundVersion: Equatable, CustomStringConvertible {
     /// The assignment should not include the package.
     ///
@@ -364,16 +364,16 @@ public enum BoundVersion: Equatable, CustomStringConvertible {
     }
 }
 
+// FIXME: Maybe each package should just return this, instead of a list of
+// `PackageContainerConstraint`s. That won't work if we decide this should
+// eventually map based on the `Container` rather than the `Identifier`, though,
+// so they are separate for now.
+//
 /// A container for constraints for a set of packages.
 ///
 /// This data structure is only designed to represent satisfiable constraint
 /// sets, it cannot represent sets including containers which have an empty
 /// constraint.
-//
-// FIXME: Maybe each package should just return this, instead of a list of
-// `PackageContainerConstraint`s. That won't work if we decide this should
-// eventually map based on the `Container` rather than the `Identifier`, though,
-// so they are separate for now.
 public struct PackageContainerConstraintSet<C: PackageContainer>: Collection, Hashable {
     public typealias Container = C
     public typealias Identifier = Container.Identifier
@@ -513,6 +513,8 @@ public struct PackageContainerConstraintSet<C: PackageContainer>: Collection, Ha
     }
 }
 
+// FIXME: Actually make efficient.
+//
 /// A container for version assignments for a set of packages, exposed as a
 /// sequence of `Container` to `BoundVersion` bindings.
 ///
@@ -524,16 +526,14 @@ public struct PackageContainerConstraintSet<C: PackageContainer>: Collection, Ha
 /// The set itself is designed to only ever contain a consistent set of
 /// assignments, i.e. each assignment should satisfy the induced
 /// `constraints`, but this invariant is not explicitly enforced.
-//
-// FIXME: Actually make efficient.
 struct VersionAssignmentSet<C: PackageContainer>: Equatable, Sequence {
     typealias Container = C
     typealias Identifier = Container.Identifier
 
-    /// The assignment records.
-    //
     // FIXME: Does it really make sense to key on the identifier here. Should we
     // require referential equality of containers and use that to simplify?
+    //
+    /// The assignment records.
     fileprivate var assignments: OrderedDictionary<Identifier, (container: Container, binding: BoundVersion)>
 
     /// Create an empty assignment.
@@ -596,15 +596,15 @@ struct VersionAssignmentSet<C: PackageContainer>: Equatable, Sequence {
         return result
     }
 
+    // FIXME: We need to cache this.
+    //
     /// The combined version constraints induced by the assignment.
     ///
     /// This consists of the merged constraints which need to be satisfied on
     /// each package as a result of the versions selected in the assignment.
     ///
     /// The resulting constraint set is guaranteed to be non-empty for each
     /// mapping, assuming the invariants on the set are followed.
-    //
-    // FIXME: We need to cache this.
     var constraints: PackageContainerConstraintSet<Container> {
         // Collect all of the constraints.
         var result = PackageContainerConstraintSet<Container>()
@@ -643,9 +643,9 @@ struct VersionAssignmentSet<C: PackageContainer>: Equatable, Sequence {
         return result
     }
 
-    /// Check if the given `binding` for `container` is valid within the assignment.
-    //
     // FIXME: This is currently very inefficient.
+    //
+    /// Check if the given `binding` for `container` is valid within the assignment.
     func isValid(binding: BoundVersion, for container: Container) -> Bool {
         switch binding {
         case .excluded:
@@ -820,8 +820,9 @@ public class DependencyResolver<
     /// Skip updating containers while fetching them.
     private let skipUpdate: Bool
 
-    /// Contains any error encountered during dependency resolution.
     // FIXME: @testable private
+    //
+    /// Contains any error encountered during dependency resolution.
     var error: Swift.Error?
 
     /// Key used to cache a resolved subtree.
@@ -969,6 +970,11 @@ public class DependencyResolver<
         return assignment
     }
 
+    // FIXME: This needs to a way to return information on the failure, or we
+    // will need to have it call the delegate directly.
+    //
+    // FIXME: @testable private
+    //
     /// Resolve an individual container dependency tree.
     ///
     /// This is the primary method in our bottom-up algorithm for resolving
@@ -983,11 +989,6 @@ public class DependencyResolver<
     ///   - constraints: The external constraints which must be honored by the solution.
     ///   - exclusions: The list of individually excluded package versions.
     /// - Returns: A sequence of feasible solutions, starting with the most preferable.
-    //
-    // FIXME: This needs to a way to return information on the failure, or we
-    // will need to have it call the delegate directly.
-    //
-    // FIXME: @testable private
     func resolveSubtree(
         _ container: Container,
         subjectTo allConstraints: ConstraintSet,

Sources/PackageGraph/PackageGraphRoot.swift

@@ -35,8 +35,9 @@ public struct PackageGraphRootInput {
 /// Represents the inputs to the package graph.
 public struct PackageGraphRoot {
 
-    /// Represents a top level package dependencies.
     // FIXME: We can kill this now.
+    //
+    /// Represents a top level package dependencies.
     public struct PackageDependency {
 
         public typealias Requirement = PackageModel.PackageDependencyDescription.Requirement

Sources/PackageLoading/ManifestLoader.swift

@@ -431,8 +431,8 @@ private func sandboxProfile() -> String {
     return stream.bytes.asString!
 }
 
-/// Represents a string error.
 // FIXME: We should probably just remove this and make all Manifest errors Codable.
+/// Represents a string error.
 struct StringError: Equatable, Codable, CustomStringConvertible, Error {
 
     /// The description of the error.
@@ -521,8 +521,8 @@ final class ManifestLoadRule: LLBuildRule {
     }
 }
 
-/// A rule to get file info of a file on disk.
 // FIXME: Find a proper place for this rule.
+/// A rule to get file info of a file on disk.
 final class FileInfoRule: LLBuildRule {
 
     struct RuleKey: LLBuildKey {
@@ -564,10 +564,10 @@ final class FileInfoRule: LLBuildRule {
     }
 }
 
+// FIXME: Find a proper place for this rule.
 /// A rule to compute the current version of the pacakge manager.
 ///
 /// This rule will always run.
-// FIXME: Find a proper place for this rule.
 final class SwiftPMVersionRule: LLBuildRule {
 
     struct RuleKey: LLBuildKey {