Module aliasing: validate source files / only allow Swift files for modules aliased (#4303)

Resolves rdar://90661600

Author: elsh

Sources/PackageGraph/PackageGraph+Loading.swift

@@ -271,8 +271,8 @@ private func createResolvedPackages(
     }
 
     // Gather and resolve module aliases specified for targets in all dependent packages
-    let packageAliases = resolveModuleAliases(packageBuilders: packageBuilders,
-                                              observabilityScope: observabilityScope)
+    let packageAliases = try resolveModuleAliases(packageBuilders: packageBuilders,
+                                                  observabilityScope: observabilityScope)
 
     // Scan and validate the dependencies
     for packageBuilder in packageBuilders {
@@ -621,7 +621,7 @@ private func computePlatforms(
 
 // Track and override module aliases specified for targets in a package graph
 private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder],
-                                  observabilityScope: ObservabilityScope) -> [PackageIdentity: [String: [ModuleAliasModel]]]? {
+                                  observabilityScope: ObservabilityScope) throws -> [PackageIdentity: [String: [ModuleAliasModel]]]? {
 
     // If there are no module aliases specified, return early
     let hasAliases = packageBuilders.contains { $0.package.targets.contains {
@@ -647,17 +647,12 @@ private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder],
 
                     if let aliasList = prodRef.moduleAliases {
                         for (depName, depAlias) in aliasList {
-                            if let existingAlias = aliasTracker.alias(target: depName, originPackage: prodPkgID) {
-                                // Error if there are multiple aliases specified for this product dependency
-                                observabilityScope.emit(PackageGraphError.multipleModuleAliases(target: depName, product: prodRef.name, package: prodPkg, aliases: [existingAlias, depAlias]))
-                                return nil
-                            }
                             // Track aliases for this product
-                            aliasTracker.addAlias(depAlias,
-                                                  target: depName,
-                                                  product: prodRef.name,
-                                                  originPackage: PackageIdentity.plain(prodPkg),
-                                                  consumingPackage: packageBuilder.package.identity)
+                            try aliasTracker.addAlias(depAlias,
+                                                      target: depName,
+                                                      product: prodRef.name,
+                                                      originPackage: PackageIdentity.plain(prodPkg),
+                                                      consumingPackage: packageBuilder.package.identity)
                         }
                     }
                 }
@@ -667,16 +662,31 @@ private func resolveModuleAliases(packageBuilders: [ResolvedPackageBuilder],
 
     // Track targets that need module aliases for each package
     for packageBuilder in packageBuilders {
-        for produdct in packageBuilder.package.products {
-            var list = produdct.targets.map{$0.dependencies}.flatMap{$0}.compactMap{$0.target?.name}
-            list.append(contentsOf: produdct.targets.map{$0.name})
-            aliasTracker.addAliasesForTargets(list, product: produdct.name, package: packageBuilder.package.identity)
+        for product in packageBuilder.package.products {
+            var allTargets = product.targets.map{$0.dependencies}.flatMap{$0}.compactMap{$0.target}
+            allTargets.append(contentsOf: product.targets)
+            aliasTracker.addAliasesForTargets(allTargets,
+                                              product: product.name,
+                                              package: packageBuilder.package.identity)
         }
     }
 
     // Override module aliases upstream if needed
     aliasTracker.propagateAliases()
 
+    // Validate sources (Swift files only) for modules being aliased.
+    // Needs to be done after `propagateAliases` since aliases defined
+    // upstream can be overriden.
+    for packageBuilder in packageBuilders {
+        for product in packageBuilder.package.products {
+            var allTargets = product.targets.map{$0.dependencies}.flatMap{$0}.compactMap{$0.target}
+            allTargets.append(contentsOf: product.targets)
+            try aliasTracker.validateSources(allTargets,
+                                             product: product.name,
+                                             package: packageBuilder.package.identity)
+        }
+    }
+
     return aliasTracker.idTargetToAliases
 }
 
@@ -694,7 +704,15 @@ private class ModuleAliasTracker {
                   target: String,
                   product: String,
                   originPackage: PackageIdentity,
-                  consumingPackage: PackageIdentity) {
+                  consumingPackage: PackageIdentity) throws {
+        if let aliasDict = aliasMap[originPackage] {
+            let models = aliasDict.values.flatMap{$0}.filter { $0.name == target }
+            if !models.isEmpty {
+                // Error if there are multiple aliases specified for this product dependency
+                throw PackageGraphError.multipleModuleAliases(target: target, product: product, package: originPackage.description, aliases: models.map{$0.alias} + [alias])
+            }
+        }
+
         let model = ModuleAliasModel(name: target, alias: alias, originPackage: originPackage, consumingPackage: consumingPackage)
         aliasMap[originPackage, default: [:]][product, default: []].append(model)
     }
@@ -710,31 +728,31 @@ private class ModuleAliasTracker {
         }
     }
 
-    func addAliasesForTargets(_ targets: [String],
+    func addAliasesForTargets(_ targets: [Target],
                               product: String,
                               package: PackageIdentity) {
-        
         let aliases = aliasMap[package]?[product]
-        for targetName in targets {
-            if idTargetToAliases[package]?[targetName] == nil {
-                idTargetToAliases[package, default: [:]][targetName] = []
+        for target in targets {
+            if idTargetToAliases[package]?[target.name] == nil {
+                idTargetToAliases[package, default: [:]][target.name] = []
             }
 
             if let aliases = aliases {
-                idTargetToAliases[package]?[targetName]?.append(contentsOf: aliases)
+                idTargetToAliases[package]?[target.name]?.append(contentsOf: aliases)
             }
         }
     }
 
-    func alias(target: String,
-               originPackage: PackageIdentity) -> String? {
-        if let aliasDict = aliasMap[originPackage] {
-            let models = aliasDict.values.flatMap{$0}.filter { $0.name == target }
-            // this func only checks if there's any existing alias so
-            // just return the first alias value
-            return models.first?.alias
+    func validateSources(_ targets: [Target],
+                         product: String,
+                         package: PackageIdentity) throws {
+        for target in targets {
+            if let aliases = idTargetToAliases[package]?[target.name], !aliases.isEmpty {
+                if target.sources.containsNonSwiftFiles {
+                    throw PackageGraphError.invalidSourcesForModuleAliasing(target: target.name, product: product, package: package.description)
+                }
+            }
         }
-        return nil
     }
 
     func propagateAliases() {

Sources/PackageGraph/PackageGraph.swift

@@ -44,6 +44,12 @@ enum PackageGraphError: Swift.Error {
                                product: String,
                                package: String,
                                aliases: [String])
+
+    /// Invalid sources found (only Swift files allowed) for aliasing a module
+    /// specified by a given target/product/package.
+    case invalidSourcesForModuleAliasing(target: String,
+                                         product: String,
+                                         package: String)
 }
 
 /// A collection of packages.
@@ -227,6 +233,8 @@ extension PackageGraphError: CustomStringConvertible {
             return "multiple products named '\(product)' in: '\(packages.joined(separator: "', '"))'"
         case .multipleModuleAliases(let target, let product, let package, let aliases):
             return "multiple aliases: ['\(aliases.joined(separator: "', '"))'] found for target '\(target)' in product '\(product)' from package '\(package)'"
+        case .invalidSourcesForModuleAliasing(let target, let product, let package):
+            return "module aliasing can only be used for Swift based targets; non-Swift sources found in target '\(target)' for product '\(product)' from package '\(package)'"
         }
     }
 }

Sources/PackageModel/Sources.swift

@@ -50,4 +50,13 @@ public struct Sources: Codable {
             return ext == SupportedLanguageExtension.m.rawValue || ext == SupportedLanguageExtension.mm.rawValue
         })
     }
+
+    public var containsNonSwiftFiles: Bool {
+        return paths.contains(where: {
+            guard let ext = $0.extension else {
+                return false
+            }
+            return !SupportedLanguageExtension.swiftExtensions.contains(ext)
+        })
+    }
 }