Fix FCS type-checking - AutoOpen modules were being handled wrong.

Add a test that was failing before the change.
safesparrow · Nov 13, 2022 · 3e4cd99 · 3e4cd99
1 parent 6a482d6
commit 3e4cd99
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 16 deletions.
diff --git a/tests/ParallelTypeCheckingTests/Code/ASTVisit.fs b/tests/ParallelTypeCheckingTests/Code/ASTVisit.fs
@@ -1254,18 +1254,20 @@ module TopModulesExtraction =
                                 synAccessOption,
                                 range,
                                 synModuleOrNamespaceTrivia) ->
-            if mightHaveAutoOpen synAttributeLists then
-                // Contents of a module that's potentially AutoOpen are available from its parent without a prefix.
-                // Stay safe and as soon as the parent module is reachable, consider this module reachable as well
-                [| LongIdent.Empty |]
-            else
-            // 'module A.B' is equivalent to 'namespace A; module B', meaning that 'A' is opened implicitly
             if
-                synModuleOrNamespaceKind.IsModule && longId.Length > 1
+                mightHaveAutoOpen synAttributeLists && synModuleOrNamespaceKind.IsModule
             then
+                // Contents of a module that's potentially AutoOpen are available from its parent without a prefix.
+                // Stay safe and as soon as the parent module is reachable, consider this module reachable as well
                 [| longId.GetSlice(None, Some <| longId.Length - 2); longId |]
             else
-                [| longId |]
+                // 'module A.B' is equivalent to 'namespace A; module B', meaning that 'A' is opened implicitly
+                if
+                    synModuleOrNamespaceKind.IsModule && longId.Length > 1
+                then
+                    [| longId.GetSlice(None, Some <| longId.Length - 2); longId |]
+                else
+                    [| longId |]
     // TODO Temporarily disabled digging into the file's structure to avoid edge cases where another file depends on this file's namespace existing (but nothing else)
     // synModuleDecls
     // |> moduleDecls
@@ -1307,6 +1309,7 @@ module TopModulesExtraction =
                                 synAccessOption,
                                 range) ->
                 let idents =
+                    // TODO Fix this by making it similar to what happens in other places where we detect AutoOpen modules
                     if mightHaveAutoOpen synAttributeLists then
                         // Contents of a module that's potentially AutoOpen are available everywhere, so treat it as if it had no name ('root' module).
                         [| LongIdent.Empty |]
@@ -1326,18 +1329,20 @@ module TopModulesExtraction =
                                    synAccessOption,
                                    range,
                                    synModuleOrNamespaceTrivia) ->
-            if mightHaveAutoOpen synAttributeLists then
-                // Contents of a module that's potentially AutoOpen are available from its parent without a prefix.
-                // Stay safe and as soon as the parent module is reachable, consider this module reachable as well
-                [| LongIdent.Empty |]
-            else
-            // 'module A.B' is equivalent to 'namespace A; module B', meaning that 'A' is opened implicitly
             if
-                synModuleOrNamespaceKind.IsModule && longId.Length > 1
+                mightHaveAutoOpen synAttributeLists && synModuleOrNamespaceKind.IsModule
             then
+                // Contents of a module that's potentially AutoOpen are available from its parent without a prefix.
+                // Stay safe and as soon as the parent module is reachable, consider this module reachable as well
                 [| longId.GetSlice(None, Some <| longId.Length - 2); longId |]
             else
-                [| longId |]
+                // 'module A.B' is equivalent to 'namespace A; module B', meaning that 'A' is opened implicitly
+                if
+                    synModuleOrNamespaceKind.IsModule && longId.Length > 1
+                then
+                    [| longId.GetSlice(None, Some <| longId.Length - 2); longId |]
+                else
+                    [| longId |]
 
     and moduleSigDecls (x: SynModuleSigDecl list) : Eit =
         let emptyState = Eit.Nested [||]

diff --git a/tests/ParallelTypeCheckingTests/Tests/TestDependencyResolution.fs b/tests/ParallelTypeCheckingTests/Tests/TestDependencyResolution.fs
@@ -49,6 +49,29 @@ open A
     let expectedEdges = [ "B.fs", [ "A.fs" ] ]
     assertGraphEqual deps expectedEdges
 
+
+[<Test>]
+let ``Another failing FCS test`` () =
+    let files =
+        [|
+            "A.fsi", """
+namespace FSharp.Compiler.CodeAnalysis
+type LegacyReferenceResolver = X of int  
+"""
+            "B.fsi", """
+[<System.Obsolete("This module is not for external use and may be removed in a future release of FSharp.Compiler.Service")>]  
+module public FSharp.Compiler.CodeAnalysis.LegacyMSBuildReferenceResolver  
+  
+val getResolver: unit -> LegacyReferenceResolver
+"""
+        |] |> buildFiles
+
+    let deps = DependencyResolution.detectFileDependencies files
+
+    let expectedEdges = ["B.fsi", ["A.fsi"]]
+    assertGraphEqual deps expectedEdges
+
+
 [<Test>]
 let ``When defining a top-level module, the implicit parent namespace is taken into account when considering references to the file - .fsi pair`` () =
     let files =