updates from feedback

Miguel Perez · Miguel Perez · commit f668f6a5f86b · 2021-06-30T22:41:35.000-04:00
diff --git a/Fixtures/Templates/AddTemplate/GoodTemplate/Sources/___NAME___/___NAME___.swift b/Fixtures/Templates/AddTemplate/GoodTemplate/Sources/___NAME___/___NAME___.swift
@@ -1,4 +1,4 @@
-public struct ___NAME___ {
+public struct ___NAME_AS_C99___ {
     public private(set) var text = "Hello, World!"
 
     public init() {
diff --git a/Fixtures/Templates/AddTemplate/GoodTemplate/Tests/___NAME___Tests/___NAME___Tests.swift b/Fixtures/Templates/AddTemplate/GoodTemplate/Tests/___NAME___Tests/___NAME___Tests.swift
@@ -1,11 +1,11 @@
 import XCTest
-@testable import ___NAME___
+@testable import ___NAME_AS_C99___
 
-final class ___NAME___Tests: XCTestCase {
+final class ___NAME_AS_C99___Tests: XCTestCase {
     func testExample() throws {
         // This is an example of a functional test case.
         // Use XCTAssert and related functions to verify your tests produce the correct
         // results.
-        XCTAssertEqual(___NAME___().text, "Hello, World!")
+        XCTAssertEqual(___NAME_AS_C99___().text, "Hello, World!")
     }
 }
diff --git a/Fixtures/Templates/AddTemplateErrors/MissingManifest/Sources/___NAME___/___NAME___.swift b/Fixtures/Templates/AddTemplateErrors/MissingManifest/Sources/___NAME___/___NAME___.swift
@@ -1,4 +1,4 @@
-public struct ___NAME___ {
+public struct ___NAME_AS_C99___ {
     public private(set) var text = "Hello, World!"
 
     public init() {
diff --git a/Fixtures/Templates/AddTemplateErrors/MissingManifest/Tests/___NAME___Tests/___NAME___Tests.swift b/Fixtures/Templates/AddTemplateErrors/MissingManifest/Tests/___NAME___Tests/___NAME___Tests.swift
@@ -1,11 +1,11 @@
 import XCTest
-@testable import ___NAME___
+@testable import ___NAME_AS_C99___
 
-final class ___NAME___Tests: XCTestCase {
+final class ___NAME_AS_C99___Tests: XCTestCase {
     func testExample() throws {
         // This is an example of a functional test case.
         // Use XCTAssert and related functions to verify your tests produce the correct
         // results.
-        XCTAssertEqual(___NAME___().text, "Hello, World!")
+        XCTAssertEqual(___NAME_AS_C99___().text, "Hello, World!")
     }
 }
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/.gitignore b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/.gitignore
@@ -0,0 +1,7 @@
+.DS_Store
+/.build
+/Packages
+/*.xcodeproj
+xcuserdata/
+DerivedData/
+.swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/Package.swift b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/Package.swift
@@ -0,0 +1,20 @@
+// swift-tools-version:5.5
+// The swift-tools-version declares the minimum version of Swift required to build this package.
+
+import PackageDescription
+
+let package = Package(
+    name: "___NAME___",
+    dependencies: [
+        // Dependencies declare other packages that this package depends on.
+        // .package(url: /* package url */, from: "1.0.0"),
+    ],
+    targets: [
+        // Targets are the basic building blocks of a package. A target can define a module or a test suite.
+        // Targets can depend on other targets in this package, and on products in packages this package depends on.
+        .executableTarget(
+            name: "___NAME___",
+            dependencies: [],
+            path: "src"),
+    ]
+)
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/README.md b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/README.md
@@ -0,0 +1,3 @@
+# ___NAME___
+
+A description of this package.
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/src/main.swift b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/ExeTemplate/src/main.swift
@@ -0,0 +1 @@
+print("Hello, world!")
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/TestTemplate/Package.swift b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/TestTemplate/Package.swift
@@ -23,4 +23,4 @@ let package = Package(
             dependencies: [],
             path: "Sources"),
     ]
-)
+)
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/TestTemplate/README.md b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/TestTemplate/README.md
@@ -1,3 +1,3 @@
 # ___NAME___
 
-A description of this package.
+A description of this package.
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/TestTemplate/Sources/___NAME___.swift b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/TestTemplate/Sources/___NAME___.swift
@@ -3,4 +3,4 @@ public struct ___NAME___ {
 
     public init() {
     }
-}
+}
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/default/Package.swift b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/default/Package.swift
@@ -0,0 +1,20 @@
+// swift-tools-version:5.5
+// The swift-tools-version declares the minimum version of Swift required to build this package.
+
+import PackageDescription
+
+let package = Package(
+    name: "___NAME___",
+    dependencies: [
+        // Dependencies declare other packages that this package depends on.
+        // .package(url: /* package url */, from: "1.0.0"),
+    ],
+    targets: [
+        // Targets are the basic building blocks of a package. A target can define a module or a test suite.
+        // Targets can depend on other targets in this package, and on products in packages this package depends on.
+        .executableTarget(
+            name: "___NAME___",
+            dependencies: [],
+            path: "src"),
+    ]
+)
diff --git a/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/default/src/main.swift b/Fixtures/Templates/MakePackageWithTemplate/templates/new-package/default/src/main.swift
@@ -0,0 +1 @@
+print("Hello, world!")
diff --git a/Sources/Commands/SwiftPackageTool.swift b/Sources/Commands/SwiftPackageTool.swift
@@ -31,7 +31,7 @@ public struct SwiftPackageTool: ParsableCommand {
         abstract: "Perform operations on Swift packages",
         discussion: "SEE ALSO: swift build, swift run, swift test",
         version: SwiftVersion.currentVersion.completeDisplayString,
-        subcommands: getSubCommands(),
+        subcommands: subCommands,
         helpNames: [.short, .long, .customLong("help", withSingleDash: true)])
 
     @OptionGroup()
@@ -43,45 +43,47 @@ public struct SwiftPackageTool: ParsableCommand {
 extension SwiftPackageTool {
     // This way the feature flag can control if the new subcommads
     // are visible
-    static func getSubCommands() -> [ParsableCommand.Type] {
-        var subCommands: [ParsableCommand.Type] = [
-            Clean.self,
-            PurgeCache.self,
-            Reset.self,
-            Update.self,
-            Describe.self,
-            Init.self,
-            Format.self,
-            
-            APIDiff.self,
-            DumpSymbolGraph.self,
-            DumpPIF.self,
-            DumpPackage.self,
-            
-            Edit.self,
-            Unedit.self,
-            
-            Config.self,
-            Resolve.self,
-            Fetch.self,
+    static var subCommands: [ParsableCommand.Type] {
+        get {
+            var commands: [ParsableCommand.Type] = [
+                Clean.self,
+                PurgeCache.self,
+                Reset.self,
+                Update.self,
+                Describe.self,
+                Init.self,
+                Format.self,
+                
+                APIDiff.self,
+                DumpSymbolGraph.self,
+                DumpPIF.self,
+                DumpPackage.self,
+                
+                Edit.self,
+                Unedit.self,
+                
+                Config.self,
+                Resolve.self,
+                Fetch.self,
+                
+                ShowDependencies.self,
+                ToolsVersionCommand.self,
+                GenerateXcodeProject.self,
+                ComputeChecksum.self,
+                ArchiveSource.self,
+                CompletionTool.self,
+            ]
             
-            ShowDependencies.self,
-            ToolsVersionCommand.self,
-            GenerateXcodeProject.self,
-            ComputeChecksum.self,
-            ArchiveSource.self,
-            CompletionTool.self,
-        ]
-        
-        if InitPackage.createPackageMode == .new {
-            if let index = subCommands.firstIndex(where: { $0 == Init.self }) {
-                subCommands.insert(Create.self, at: index + 1)
-                subCommands.insert(AddTemplate.self, at: index + 2)
-                subCommands.insert(UpdateTemplate.self, at: index + 3)
+            if InitPackage.createPackageMode == .new {
+                if let index = commands.firstIndex(where: { $0 == Init.self }) {
+                    commands.insert(Create.self, at: index + 1)
+                    commands.insert(AddTemplate.self, at: index + 2)
+                    commands.insert(UpdateTemplate.self, at: index + 3)
+                }
             }
+            
+            return commands
         }
-        
-        return subCommands
     }
 }
 
@@ -224,15 +226,15 @@ extension SwiftPackageTool {
         
         func run(_ swiftTool: SwiftTool) throws {
             guard let cwd = localFileSystem.currentWorkingDirectory else {
-                throw InternalError("Could not find the current working directory.")
+                throw MakePackageErrors.currentWorkingDirectoryNotFound
             }
             
             guard let configPath = try swiftTool.getConfigPath() else {
-                throw InternalError("Could not find config path")
+                throw MakePackageErrors.configPathNotFound
             }
             
-            guard !(packageType != nil && template != nil) else {
-                throw InternalError("Can't use --type in conjunction with --template")
+            guard packageType == nil || template == nil else {
+                throw MakePackageErrors.cannotUseTypeWithTemplate
             }
             
             let initPackage = try InitPackage(fileSystem: localFileSystem,
@@ -268,15 +270,15 @@ extension SwiftPackageTool {
 
         func run(_ swiftTool: SwiftTool) throws {
             guard let cwd = localFileSystem.currentWorkingDirectory else {
-                throw InternalError("Could not find the current working directory.")
+                throw MakePackageErrors.currentWorkingDirectoryNotFound
             }
             
             guard let configPath = try swiftTool.getConfigPath() else {
-                throw InternalError("Could not find config path")
+                throw MakePackageErrors.configPathNotFound
             }
             
-            guard !(packageType != nil && template != nil) else {
-                throw InternalError("Can't use --type in conjunction with --template")
+            guard packageType == nil || template == nil else {
+                throw MakePackageErrors.cannotUseTypeWithTemplate
             }
           
             let initPackage = try InitPackage(fileSystem: localFileSystem,
@@ -315,22 +317,22 @@ extension SwiftPackageTool {
             let templatePath: AbsolutePath
             if let path = try? AbsolutePath(validating: url) {
                 guard localFileSystem.exists(path) else {
-                    throw StringError("Could not find template: \(path)")
+                    throw MakePackageErrors.templateNotFound(path)
                 }
                 
                 guard localFileSystem.isDirectory(path) else {
                     throw StringError("\(path) is not a valid directory")
                 }
                 
-                templatePath = configPath.appending(components: "templates", "new-package", name ?? path.basename)
+                templatePath = configPath.withTemplate(template: [name ?? path.basename])
                 try localFileSystem.copy(from: path, to: templatePath)
             } else {
                 let provider = GitRepositoryProvider()
                 
                 if let templateName = name {
-                    templatePath = configPath.appending(components: "templates", "new-package", templateName)
+                    templatePath = configPath.withTemplate(template: [templateName])
                 } else if let templateName = url.split(separator: "/").last?.replacingOccurrences(of: ".git", with: "").flatMap({ String($0) }) {
-                    templatePath = configPath.appending(components: "templates", "new-package", String(templateName))
+                    templatePath = configPath.withTemplate(template: [String(templateName)])
                 } else {
                     throw InternalError("Could not determine template name")
                 }
@@ -360,12 +362,12 @@ extension SwiftPackageTool {
                 throw InternalError("Could not find config path")
             }
             
-            guard localFileSystem.exists(configPath.appending(components: "templates", "new-package", templateName)) else {
+            guard localFileSystem.exists(configPath.withTemplate(template: [templateName])) else {
                 throw StringError("Could not find template: \(templateName)")
             }
             
             let provider = GitRepositoryProvider()
-            let templatePath = configPath.appending(components: "templates", "new-package", templateName)
+            let templatePath = configPath.withTemplate(template: [templateName])
             
             guard provider.isValidDirectory(templatePath.pathString) else {
                 throw StringError("Template: \(templateName) is not a git repo, and therefore could not be updated")
@@ -893,6 +895,26 @@ extension SwiftPackageTool {
     }
 }
 
+private enum MakePackageErrors: Swift.Error, CustomStringConvertible {
+    case currentWorkingDirectoryNotFound
+    case configPathNotFound
+    case templateNotFound(AbsolutePath)
+    case cannotUseTypeWithTemplate
+    
+    var description: String {
+        switch self {
+        case .currentWorkingDirectoryNotFound:
+            return "Could not find the current working directory."
+        case .configPathNotFound:
+            return "Could not find the config path."
+        case .templateNotFound(let path):
+            return "Could not find template: \(path)."
+        case .cannotUseTypeWithTemplate:
+            return "When using templates the package's type is derived from the template."
+        }
+    }
+}
+
 extension SwiftPackageTool {
     struct GenerateXcodeProject: SwiftCommand {
         static let configuration = CommandConfiguration(
diff --git a/Sources/Workspace/InitPackage.swift b/Sources/Workspace/InitPackage.swift
@@ -80,11 +80,15 @@ public final class InitPackage {
     let suppliedPackageType: Bool
     
     private var moduleName: String {
-        get { packageName.spm_mangledToC99ExtendedIdentifier() }
+        get {
+            packageName.spm_mangledToC99ExtendedIdentifier()
+        }
     }
     
     private var typeName: String {
-        get { moduleName }
+        get {
+            moduleName
+        }
     }
 
     /// Create an instance that can create a package with given arguments.
@@ -129,33 +133,32 @@ public final class InitPackage {
     }
     
     private func makePackageNew() throws {
-        let templateHomeDirectory = configPath.appending(components: "templates", "new-package")
         var templateName: String?
         
         if let template = packageTemplateName {
-            guard fileSystem.exists(templateHomeDirectory.appending(component: template)) else {
-                throw InternalError("Could not find template folder: \(templateHomeDirectory.appending(component: template))")
+            guard fileSystem.exists(configPath.withTemplate(template: [template])) else {
+                throw InternalError("Could not find template folder: \(configPath.withTemplate(template: [template]))")
             }
             
             templateName = template
         } else {
             // Checking if a default template is present
-            if fileSystem.exists(templateHomeDirectory.appending(component: "default")) {
+            if fileSystem.exists(configPath.withTemplate(template: ["default"])) {
                 templateName = "default"
                 // There is a guard preventing '--type' to be used in conjunction with '--template'
                 // In the event that the defualt template is present and '--type' was used we'll infrom
                 // the user that the package type is coming from the template and not their supplied type.
-                if suppliedPackageType {
-                    progressReporter?("Package type is defined by the template 'default'.")
+                guard !suppliedPackageType else {
+                    throw StringError("Can't use '--type' when the 'default' template is defined")
                 }
             }
         }
         
         if let template = templateName {
-            try fileSystem.getDirectoryContents(templateHomeDirectory.appending(component: template)).forEach {
+            try fileSystem.getDirectoryContents(configPath.withTemplate(template: [template])).forEach {
                 progressReporter?("Copying \($0)")
                 try copyTemplate(fileSystem: fileSystem,
-                                 sourcePath: templateHomeDirectory.appending(components: template, $0),
+                                 sourcePath: configPath.withTemplate(template: [template, $0]),
                                  destinationPath: destinationPath,
                                  name: packageName)
             }
@@ -588,6 +591,14 @@ public final class InitPackage {
     }
 }
 
+extension AbsolutePath {
+    public func withTemplate(template: [String]) -> AbsolutePath {
+        var components = ["templates", "new-package"]
+        components += template
+        return appending(components: components)
+    }
+}
+
 // Private helpers
 
 private enum InitError: Swift.Error {
diff --git a/Tests/WorkspaceTests/TemplateTests.swift b/Tests/WorkspaceTests/TemplateTests.swift

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-public struct ___NAME___ {`
	`1`	`+public struct ___NAME_AS_C99___ {`
`2`	`2`	`public private(set) var text = "Hello, World!"`
`3`	`3`
`4`	`4`	`public init() {`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,11 @@`
`1`	`1`	`import XCTest`
`2`		`-@testable import ___NAME___`
	`2`	`+@testable import ___NAME_AS_C99___`
`3`	`3`
`4`		`-final class ___NAME___Tests: XCTestCase {`
	`4`	`+final class ___NAME_AS_C99___Tests: XCTestCase {`
`5`	`5`	`func testExample() throws {`
`6`	`6`	`// This is an example of a functional test case.`
`7`	`7`	`// Use XCTAssert and related functions to verify your tests produce the correct`
`8`	`8`	`// results.`
`9`		`- XCTAssertEqual(___NAME___().text, "Hello, World!")`
	`9`	`+ XCTAssertEqual(___NAME_AS_C99___().text, "Hello, World!")`
`10`	`10`	`}`
`11`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+# ___NAME___`
	`2`	`+`
	`3`	`+A description of this package.`
Original file line number	Diff line number	Diff line change
`@@ -23,4 +23,4 @@ let package = Package(`
`23`	`23`	`dependencies: [],`
`24`	`24`	`path: "Sources"),`
`25`	`25`	`]`
`26`		`-)`
	`26`	`+)`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,3 @@`
`1`	`1`	`# ___NAME___`
`2`	`2`
`3`		`-A description of this package.`
	`3`	`+A description of this package.`
Original file line number	Diff line number	Diff line change
`@@ -3,4 +3,4 @@ public struct ___NAME___ {`
`3`	`3`
`4`	`4`	`public init() {`
`5`	`5`	`}`
`6`		`-}`
	`6`	`+}`