Create tests cases to reproduce the module aliasing problem

Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Package.swift

@@ -0,0 +1,29 @@
+// swift-tools-version:5.7
+import PackageDescription
+
+let package = Package(
+    name: "AppPkg",
+    dependencies: [
+        .package(path: "../UtilsPkg"),
+    ],
+    targets: [
+        .executableTarget(
+            name: "App",
+            dependencies: [
+                "Utils",
+                .product(name: "Lib",
+                         package: "UtilsPkg",
+                         moduleAliases: [
+                            "Utils": "GameUtils",
+                            "Lib": "GameLib",
+                        ]),
+            ],
+            path: "./Sources/App"),
+        .target(
+                name: "Utils",
+                dependencies: [],
+                path: "./Sources/Utils"
+            )
+        ]
+    )
+

Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/App/main.swift

@@ -0,0 +1,20 @@
+/*
+ This source file is part of the Swift.org open source project
+
+ Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See http://swift.org/LICENSE.txt for license information
+ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
+*/
+
+import Utils
+import GameUtils
+import GameLib
+
+Utils.echoModule()
+
+GameLib.userHelp()
+
+let level = LevelDetector.detect(for: "TestUser")
+print(level)

Fixtures/ModuleAliasing/DirectDeps3/AppPkg/Sources/Utils/FileUtils.swift

@@ -0,0 +1,13 @@
+/*
+ This source file is part of the Swift.org open source project
+
+ Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See http://swift.org/LICENSE.txt for license information
+ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
+*/
+
+public func echoModule() {
+    print("Utils")
+}

Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Package.swift

@@ -0,0 +1,13 @@
+// swift-tools-version:5.5
+import PackageDescription
+
+let package = Package(
+    name: "UtilsPkg",
+    products: [
+        .library(name: "Lib", targets: ["Lib", "Utils"])
+    ],
+    targets: [
+        .target(name: "Utils", dependencies: []),
+        .target(name: "Lib", dependencies: ["Utils"])
+    ]
+)

Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Lib/Library.swift

@@ -0,0 +1,5 @@
+import Utils
+
+public func userHelp() {
+    _ = Utils.LevelDetector.detect(for: "someUser")
+}

Fixtures/ModuleAliasing/DirectDeps3/UtilsPkg/Sources/Utils/FileUtils.swift

@@ -0,0 +1,15 @@
+/*
+ This source file is part of the Swift.org open source project
+
+ Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+ Licensed under Apache License v2.0 with Runtime Library Exception
+
+ See http://swift.org/LICENSE.txt for license information
+ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
+*/
+
+public struct LevelDetector: Equatable {
+    public static func detect(for user: String) -> Int {
+        return user.count
+    }
+}

Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Package.swift

@@ -0,0 +1,20 @@
+// swift-tools-version:5.7
+import PackageDescription
+
+let package = Package(
+    name: "AppPkg",
+    dependencies: [
+        .package(path: "../XPkg"),
+    ],
+    targets: [
+        .executableTarget(
+            name: "App",
+            dependencies: [
+                .product(name: "X",
+                         package: "XPkg",
+                         moduleAliases: ["Utils": "XUtils", "X": "XNew"]
+                        )
+            ]),
+    ]
+)
+

Fixtures/ModuleAliasing/NestedDeps3/AppPkg/Sources/App/main.swift

@@ -0,0 +1,5 @@
+import XNew
+import XUtils
+
+utilsInX()
+XNew.funcInX()

Fixtures/ModuleAliasing/NestedDeps3/XPkg/Package.swift

@@ -0,0 +1,18 @@
+// swift-tools-version:5.7
+// The swift-tools-version declares the minimum version of Swift required to build this package.
+
+import PackageDescription
+
+let package = Package(
+    name: "XPkg",
+    products: [
+        .library(name: "X", targets: ["X"]),
+    ],
+    targets: [
+        .target(name: "X",
+                dependencies: [
+                    "Utils",
+                ]),
+        .target(name: "Utils", dependencies: [])
+    ]
+)

Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/Utils/File.swift

@@ -0,0 +1,3 @@
+public func utilsInX() {
+    print("utils in X")
+}

Fixtures/ModuleAliasing/NestedDeps3/XPkg/Sources/X/File.swift

@@ -0,0 +1,6 @@
+import Utils
+
+public func funcInX() {
+    print("func in X")
+    utilsInX()
+}

Sources/PackageGraph/ModuleAliasTracker.swift

@@ -16,22 +16,39 @@ import Basics
 // This is a helper class that tracks module aliases in a package dependency graph
 // and handles overriding upstream aliases where aliases themselves conflict.
 struct ModuleAliasTracker {
+    /// Map product identifier to a list of aliases that are associated through product dependencies on it.
     fileprivate var aliasMap = [String: [ModuleAliasModel]]()
+
+    /// Map a package and its product identifier to a list of aliases that are associated through product dependencies on it.
     fileprivate var idToAliasMap = [PackageIdentity: [String: [ModuleAliasModel]]]()
+
+    /// Map from a package + product identifier to the modules that are included in a product + ones that are one level dependency away
     var idToProductToAllModules = [PackageIdentity: [String: [Module]]]()
+
+    /// Map from a product identifier to the modules that are directly included in the product declaration.
     var productToDirectModules = [String: [Module]]()
+
+    /// Map from a product identifier to the modules that are included in a product + ones that are one level dependency way
     var productToAllModules = [String: [Module]]()
+
+    /// Map parent product identifier to any product identifier that it depends on transitively.
     var parentToChildProducts = [String: [String]]()
+
+    // Parent/child relationship DAG through product dependencies
     var parentToChildIDs = [PackageIdentity: [PackageIdentity]]()
     var childToParentID = [PackageIdentity: PackageIdentity]()
+
     var appliedAliases = Set<String>()
 
     init() {}
+
+    /// * Analyze the dependencies of the modules from origin package and build a DAG of packages.
+    /// * Record the module aliases
     mutating func addModuleAliases(modules: [Module], package: PackageIdentity) throws {
         let moduleDependencies = modules.flatMap(\.dependencies)
         for dep in moduleDependencies {
-            if case let .product(productRef, _) = dep,
-               let productPkg = productRef.package {
+            if case let .product(productRef, _) = dep {
+               if let productPkg = productRef.package {
                 let productPkgID = PackageIdentity.plain(productPkg)
                 // Track dependency package ID chain
                 addPackageIDChain(parent: package, child: productPkgID)
@@ -43,6 +60,7 @@ struct ModuleAliasTracker {
                                    originPackage: productPkgID,
                                    consumingPackage: package)
                 }
+               }
             }
         }
     }
@@ -93,27 +111,15 @@ struct ModuleAliasTracker {
             }
         }
 
+        // FIXME this isn't all modules in a product. This is just the dependencies that are one level away from the modules that compose the product and the composing modules themselves.
         var allModulesInProduct = moduleDeps.compactMap(\.module)
         allModulesInProduct.append(contentsOf: product.modules)
         idToProductToAllModules[package, default: [:]][product.identity] = allModulesInProduct
+
         productToDirectModules[product.identity] = product.modules
         productToAllModules[product.identity] = allModulesInProduct
     }
 
-    func validateAndApplyAliases(product: Product,
-                                 package: PackageIdentity,
-                                 observabilityScope: ObservabilityScope) throws {
-        guard let modules = idToProductToAllModules[package]?[product.identity] else { return }
-        let modulesWithAliases = modules.filter{ $0.moduleAliases != nil }
-        for moduleWithAlias in modulesWithAliases {
-            if moduleWithAlias.sources.containsNonSwiftFiles {
-                let aliasesMsg = moduleWithAlias.moduleAliases?.map{"'\($0.key)' as '\($0.value)'"}.joined(separator: ", ") ?? ""
-                observabilityScope.emit(warning: "target '\(moduleWithAlias.name)' for product '\(product.name)' from package '\(package.description)' has module aliases: [\(aliasesMsg)] but may contain non-Swift sources; there might be a conflict among non-Swift symbols")
-            }
-            moduleWithAlias.applyAlias()
-        }
-    }
-
     mutating func propagateAliases(observabilityScope: ObservabilityScope) {
         // First get the root package ID
         var pkgID = childToParentID.first?.key
@@ -128,13 +134,14 @@ struct ModuleAliasTracker {
         if let productToAllModules = idToProductToAllModules[rootPkg] {
             // First, propagate aliases upstream
             for productID in productToAllModules.keys {
+                // Alias buffer tracks the most downstream alias for each original module name
                 var aliasBuffer = [String: ModuleAliasModel]()
                 propagate(productID: productID, observabilityScope: observabilityScope, aliasBuffer: &aliasBuffer)
             }
 
             // Then, merge or override upstream aliases downwards
             for productID in productToAllModules.keys {
-                merge(productID: productID, observabilityScope: observabilityScope)
+                merge(productID: productID, observabilityScope: observabilityScope, root: true)
             }
         }
         // Finally, fill in aliases for modules in products that are in the
@@ -198,14 +205,15 @@ struct ModuleAliasTracker {
         }
     }
 
-    // Merge all the upstream aliases and override them if necessary
-    mutating func merge(productID: String, observabilityScope: ObservabilityScope) {
+    // Merge all the upstream aliases and override them if necessary (depth-first).
+    mutating func merge(productID: String, observabilityScope: ObservabilityScope, root: Bool) {
         guard let children = parentToChildProducts[productID] else {
             return
         }
         for childID in children {
             merge(productID: childID,
-                  observabilityScope: observabilityScope)
+                  observabilityScope: observabilityScope,
+                  root: false)
         }
 
         if let curDirectModules = productToDirectModules[productID] {
@@ -232,10 +240,10 @@ struct ModuleAliasTracker {
                 let depProdModuleAliases = depProdModule.moduleAliases ?? [:]
                 for (key, val) in depProdModuleAliases {
                     var shouldAddAliases = false
-                    if depProdModule.name == key {
-                        shouldAddAliases = true
-                    } else if !depProductModules.map({$0.name}).contains(key) {
-                        shouldAddAliases = true
+                    if depProdModule.name == key && !root {
+                            shouldAddAliases = true
+                    } else if !depProductModules.map({$0.name}).contains(key) && !root {
+                            shouldAddAliases = true
                     }
                     if shouldAddAliases {
                         if depProductAliases[key]?.contains(val) ?? false {
@@ -246,6 +254,7 @@ struct ModuleAliasTracker {
                     }
                 }
             }
+
             chainModuleAliases(modules: curDirectModules,
                                checkedModules: relevantModules,
                                moduleAliases: moduleAliases,
@@ -263,14 +272,11 @@ struct ModuleAliasTracker {
     mutating func fillInRest(package: PackageIdentity) {
         if let productToModules = idToProductToAllModules[package] {
             for (_, productModules) in productToModules {
-                let unAliased = productModules.contains { $0.moduleAliases == nil }
-                if unAliased {
-                    for module in productModules {
-                        let depAliases = module.recursiveDependentModules.compactMap{$0.moduleAliases}.flatMap{$0}
-                        for (key, alias) in depAliases {
-                            appliedAliases.insert(key)
-                            module.addModuleAlias(for: key, as: alias)
-                        }
+                for module in productModules {
+                    let depAliases = module.recursiveDependentModules.compactMap{$0.moduleAliases}.flatMap{$0}
+                    for (key, alias) in depAliases {
+                        appliedAliases.insert(key)
+                        module.addModuleAlias(for: key, as: alias)
                     }
                 }
             }
@@ -281,6 +287,20 @@ struct ModuleAliasTracker {
         }
     }
 
+    func validateAndApplyAliases(product: Product,
+                                 package: PackageIdentity,
+                                 observabilityScope: ObservabilityScope) throws {
+        guard let modules = idToProductToAllModules[package]?[product.identity] else { return }
+        let modulesWithAliases = modules.filter{ $0.moduleAliases != nil }
+        for moduleWithAlias in modulesWithAliases {
+            if moduleWithAlias.sources.containsNonSwiftFiles {
+                let aliasesMsg = moduleWithAlias.moduleAliases?.map{"'\($0.key)' as '\($0.value)'"}.joined(separator: ", ") ?? ""
+                observabilityScope.emit(warning: "target '\(moduleWithAlias.name)' for product '\(product.name)' from package '\(package.description)' has module aliases: [\(aliasesMsg)] but may contain non-Swift sources; there might be a conflict among non-Swift symbols")
+            }
+            moduleWithAlias.applyAlias()
+        }
+    }
+
     func diagnoseUnappliedAliases(observabilityScope: ObservabilityScope) {
         for aliasList in aliasMap.values {
             for productAlias in aliasList {
@@ -301,6 +321,7 @@ struct ModuleAliasTracker {
         observabilityScope: ObservabilityScope
     ) {
         guard !modules.isEmpty else { return }
+
         var aliasDict = [String: String]()
         var prechainAliasDict = [String: [String]]()
         var directRefAliasDict = [String: [String]]()

Tests/BuildTests/ModuleAliasingBuildTests.swift

@@ -1806,9 +1806,11 @@ final class ModuleAliasingBuildTests: XCTestCase {
 
         result.checkProductsCount(1)
         result.checkTargetsCount(9)
+        // The root-level package is the most down-stream, so it sets the upstream module's name via the
+        // aliases. It shouldn't have an alias itself since it references the module by the alias name.
         XCTAssertTrue(
             result.targetMap.values
-                .contains { $0.module.name == "A" && $0.module.moduleAliases?["Utils"] == "XUtils" }
+                .contains { $0.module.name == "A" && $0.module.moduleAliases == nil}
         )
         XCTAssertTrue(
             result.targetMap.values
@@ -2021,12 +2023,12 @@ final class ModuleAliasingBuildTests: XCTestCase {
 
         result.checkProductsCount(1)
         result.checkTargetsCount(8)
-
+        // The root-level package is the most down-stream, so it sets the upstream module's name via the
+        // aliases. It shouldn't have an alias itself since it references the module by the alias name.
         XCTAssertTrue(
             result.targetMap.values
                 .contains {
-                    $0.module.name == "A" && $0.module.moduleAliases?["Log"] == "XLog" && $0.module.moduleAliases?
-                        .count == 1
+                    $0.module.name == "A" && $0.module.moduleAliases == nil
                 }
         )
         XCTAssertTrue(
@@ -2240,11 +2242,12 @@ final class ModuleAliasingBuildTests: XCTestCase {
 
         result.checkProductsCount(1)
         result.checkTargetsCount(8)
+        // The root-level package is the most down-stream, so it sets the upstream module's name via the
+        // aliases. It shouldn't have an alias itself since it references the module by the alias name.
         XCTAssertTrue(
             result.targetMap.values
                 .contains {
-                    $0.module.name == "A" && $0.module.moduleAliases?["Utils"] == "XUtils" && $0.module
-                        .moduleAliases?["Log"] == "XLog" && $0.module.moduleAliases?.count == 2
+                    $0.module.name == "A" && $0.module.moduleAliases == nil
                 }
         )
         XCTAssertTrue(