diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index adac6199697..b4238426bff 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -949,3 +949,11 @@ stages: commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj -c Release ;; dotnet build tests/OpenTK.Tests.Integration/OpenTK.Tests.Integration.fsproj -c Release displayName: OpenTK_FSharp_Build + - repo: bryanedds/Prime + commit: 8d55f4e8e1d76e42f5fb3c9ba69eb79fe695e9fa + buildScript: dotnet build Prime.sln --configuration Release + displayName: Prime_Build + - repo: bryanedds/Nu + commit: b35cbe02029e0e33d72a4846816cf22714eb3aac + buildScript: dotnet build Nu.sln --configuration Release + displayName: Nu_Build diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md index 085f73b09d0..9db28a547c3 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md @@ -3,6 +3,7 @@ * Fix false FS1182 (unused variable) warning for query expression variables used in where, let, join, and select clauses. ([Issue #422](https://github.com/dotnet/fsharp/issues/422)) * Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0, introduced by [#17706](https://github.com/dotnet/fsharp/pull/17706). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) * Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) +* Fix graph-based type checking incorrectly resolving dependencies when the same module name is defined across multiple files in the same namespace. ([PR #19280](https://github.com/dotnet/fsharp/pull/19280)) ### Added diff --git a/src/Compiler/Driver/GraphChecking/TrieMapping.fs b/src/Compiler/Driver/GraphChecking/TrieMapping.fs index 215f8a2dae6..e8b027d8d5b 100644 --- a/src/Compiler/Driver/GraphChecking/TrieMapping.fs +++ b/src/Compiler/Driver/GraphChecking/TrieMapping.fs @@ -53,6 +53,10 @@ let rec mergeTrieNodes (accumulatorTrie: TrieNode) (currentTrie: TrieNode) : Tri // Replace the module in favour of the namespace (which can hold nested children). | TrieNodeInfo.Module(_name, file), TrieNodeInfo.Namespace(name, currentFilesThatExposeTypes, filesDefiningNamespaceWithoutTypes) -> TrieNodeInfo.Namespace(name, currentFilesThatExposeTypes.Add file, filesDefiningNamespaceWithoutTypes) + // Multiple files define the same module name in the same namespace. + // Promote to a Namespace node so both file indices are preserved as dependencies. + | TrieNodeInfo.Module(name, file1), TrieNodeInfo.Module(_, file2) -> + TrieNodeInfo.Namespace(name, ImmutableHashSet.singleton file1 |> _.Add(file2), ImmutableHashSet.empty ()) | _ -> accumulatorTrie.Current let nextChildren = diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs index 424fc6d471e..2d73ff13130 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -1150,6 +1150,163 @@ let value = Script.ScriptModule.compute "hi" """ (set [| 2 |]) ] + scenario + "Sub-namespace opens parent namespace with types and modules" + [ + sourceFile + "Types.fs" + """ +namespace Nu + +type GameTime = + { Value: float } + static member ofSeconds (s: float) = { Value = s } +""" + Set.empty + sourceFile + "Assets.fs" + """ +namespace Nu + +module Assets = + module Default = + let Black = "black" +""" + (set [| 0 |]) + sourceFile + "Constants.fs" + """ +namespace Nu.Constants +open Nu + +module Dissolve = + let x = Assets.Default.Black +""" + (set [| 0; 1 |]) + ] + scenario + "Sub-namespace opens parent namespace and uses type constructor" + [ + sourceFile + "Types.fs" + """ +namespace Nu + +type GameTime = + { Value: float } + static member ofSeconds (s: float) = { Value = s } +""" + Set.empty + sourceFile + "Constants.fs" + """ +namespace Nu.Constants +open Nu + +module Defaults = + let IncomingTime = GameTime.ofSeconds 0.5 +""" + (set [| 0 |]) + ] + scenario + "Sub-namespace opens parent namespace with only modules" + [ + sourceFile + "Modules.fs" + """ +namespace Nu + +module Assets = + module Default = + let Black = "black" +""" + Set.empty + sourceFile + "Consumer.fs" + """ +namespace Nu.Sub +open Nu + +module M = + let x = Assets.Default.Black +""" + (set [| 0 |]) + ] + scenario + "Multiple namespace declarations in one file with AutoOpen" + [ + sourceFile + "Entity.fs" + """ +namespace Nu + +type Entity = { Name: string } +""" + Set.empty + sourceFile + "BlockMap.fs" + """ +namespace Nu.BlockMap + +module BlockMapCore = + let defaultSize = 32 + +namespace Nu + +[] +module BlockMapExtensions = + let getBlockMapSize () = 42 +""" + (set [| 0 |]) + sourceFile + "Consumer.fs" + """ +namespace Nu.Game +open Nu + +module GameLogic = + let size = getBlockMapSize () + let e : Entity = { Name = "test" } +""" + (set [| 0; 1 |]) + ] + // CompilationRepresentation(ModuleSuffix) on Assets2 avoids FS0248 at the CLR level. + scenario + "Same module name defined in multiple files of the same namespace" + [ + sourceFile + "Assets1.fs" + """ +namespace Nu + +[] +module Assets = + module Default = + let PackageName = "Default" +""" + Set.empty + sourceFile + "Assets2.fs" + """ +namespace Nu + +[] +module Assets = + module Default = + let Black = "black" +""" + Set.empty + sourceFile + "Constants.fs" + """ +namespace Nu.Constants +open Nu + +module Dissolve = + let x = Assets.Default.Black +""" + (set [| 0; 1 |]) + ] ] diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs index 2a3d7b608ea..02b8d998fe1 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/TrieMappingTests.fs @@ -329,6 +329,93 @@ let _ = () let aNode = trie.Children.["A"] Assert.Equal>(set [| 0 |], aNode.Files) +let private assertModuleMergePromotesToNamespace (fileCount: int) = + let sourceFiles = + [| for i in 0 .. fileCount - 1 do + { + Idx = i + FileName = $"M{i + 1}.fs" + ParsedInput = parseSourceCode ($"M{i + 1}.fs", $"module N.M\n\nlet v{i} = {i}") + } + { + Idx = fileCount + FileName = "Dummy.fs" + ParsedInput = Unchecked.defaultof + } |] + + let trie = getLastTrie sourceFiles + let nNode = trie.Children.["N"] + let mNode = nNode.Children.["M"] + let expectedIndices = set [| 0 .. fileCount - 1 |] + Assert.Equal>(expectedIndices, mNode.Files) + + match mNode.Current with + | TrieNodeInfo.Namespace(_, filesThatExposeTypes, filesDefiningNamespaceWithoutTypes) -> + Assert.Equal>(expectedIndices, set filesThatExposeTypes) + Assert.True(filesDefiningNamespaceWithoutTypes.Count = 0, "filesDefiningNamespaceWithoutTypes should be empty after Module+Module merge") + | other -> Assert.Fail($"Expected Namespace after Module+Module merge of {fileCount} files, got {other}") + +[] +[] +[] +let ``Module+Module merge promotes to Namespace and preserves all file indices`` (fileCount: int) = + assertModuleMergePromotesToNamespace fileCount + +[] +let ``Module+Module merge preserves children from both sides`` () = + let trie = + getLastTrie + [| + { + Idx = 0 + FileName = "M1.fs" + ParsedInput = + parseSourceCode ( + "M1.fs", + """ +namespace N + +module M = + module Child1 = + let x = 1 +""" + ) + } + { + Idx = 1 + FileName = "M2.fs" + ParsedInput = + parseSourceCode ( + "M2.fs", + """ +namespace N + +module M = + module Child2 = + let y = 2 +""" + ) + } + { + Idx = 2 + FileName = "Dummy.fs" + ParsedInput = Unchecked.defaultof + } + |] + + let nNode = trie.Children.["N"] + let mNode = nNode.Children.["M"] + Assert.Equal>(set [| 0; 1 |], mNode.Files) + + match mNode.Current with + | TrieNodeInfo.Namespace(_, filesThatExposeTypes, _) -> + Assert.Equal>(set [| 0; 1 |], set filesThatExposeTypes) + | other -> Assert.Fail($"Expected Namespace after Module+Module merge, got {other}") + + Assert.True(mNode.Children.ContainsKey("Child1"), "Child1 from file 0 should survive merge") + Assert.True(mNode.Children.ContainsKey("Child2"), "Child2 from file 1 should survive merge") + Assert.Equal(2, mNode.Children.Count) + [] let ``Two nested modules with the same name, in named namespace`` () = let trie =