Misc. unused opens analyzer fixes

docs/release-notes/.FSharp.Compiler.Service/9.0.300.md

@@ -26,6 +26,7 @@
 * Fix confusing type inference error in task expression ([Issue #13789](https://github.com/dotnet/fsharp/issues/13789), [PR #18450](https://github.com/dotnet/fsharp/pull/18450))
 * Fix missing `null` highlighting in tooltips ([PR #18457](https://github.com/dotnet/fsharp/pull/18457))
 * Make `[<CallerMemberName; Struct>]` combination work([PR #18444](https://github.com/dotnet/fsharp/pull/18444/))
+* Miscellaneous unused opens analyzer fixes. ([Issue #17629](https://github.com/dotnet/fsharp/issues/17629), [Issue #17929](https://github.com/dotnet/fsharp/issues/17929), [Issue #16226](https://github.com/dotnet/fsharp/issues/16226), [Issue #18389](https://github.com/dotnet/fsharp/issues/18389), [PR #18510](https://github.com/dotnet/fsharp/pull/18510))
 
 ### Added
 * Added missing type constraints in FCS. ([PR #18241](https://github.com/dotnet/fsharp/pull/18241))

src/Compiler/Service/ServiceAnalysis.fs

@@ -57,6 +57,13 @@ module UnusedOpens =
                                 for field in entity.FSharpFields do
                                     if field.IsStatic && field.IsLiteral then
                                         field
+
+                            if
+                                entity.IsFSharpUnion
+                                && not (entity.HasAttribute<RequireQualifiedAccessAttribute>())
+                            then
+                                for unionCase in entity.UnionCases do
+                                    unionCase
                     |]
 
                 HashSet<_>(symbols, symbolHash)
@@ -75,7 +82,7 @@ module UnusedOpens =
                 [|
                     yield OpenedModule(modul, isNestedAutoOpen)
                     for ent in modul.NestedEntities do
-                        if ent.IsFSharpModule && ent.HasAttribute<AutoOpenAttribute>() then
+                        if ent.HasAttribute<AutoOpenAttribute>() then
                             yield! getModuleAndItsAutoOpens true ent
                 |]
 
@@ -192,7 +199,7 @@ module UnusedOpens =
                         not (
                             usedModules.BagExistsValueForKey(
                                 openedEntity.Entity,
-                                fun scope -> rangeContainsRange scope openStatement.AppliedScope
+                                fun scope -> rangeContainsRange openStatement.AppliedScope scope
                             )
                         ))
 
@@ -271,7 +278,7 @@ module UnusedOpens =
 
     /// Filter out the open statements whose contents are referred to somewhere in the symbol uses.
     /// Async to allow cancellation.
-    let filterOpenStatements (symbolUses1: FSharpSymbolUse[], symbolUses2: FSharpSymbolUse[]) openStatements =
+    let filterOpenStatements (symbolUses1: FSharpSymbolUse[], symbolUses2: FSharpSymbolUse[]) (openStatements: OpenStatement[]) =
         async {
             // the key is a namespace or module, the value is a list of FSharpSymbolUse range of symbols defined in the
             // namespace or module. So, it's just symbol uses ranges grouped by namespace or module where they are _defined_.
@@ -284,18 +291,25 @@ module UnusedOpens =
                     match f.DeclaringEntity with
                     | Some entity when entity.IsNamespace || entity.IsFSharpModule ->
                         symbolUsesRangesByDeclaringEntity.BagAdd(entity, symbolUse.Range)
+
+                        if f.ApparentEnclosingEntity <> entity then
+                            symbolUsesRangesByDeclaringEntity.BagAdd(f.ApparentEnclosingEntity, symbolUse.Range)
                     | _ -> ()
                 | _ -> ()
 
+            // Check the open statements in reverse order to account for shadowing.
+            let reversedOpenStatements = openStatements |> List.ofArray |> List.rev
+
             let! results =
                 filterOpenStatementsIncremental
                     symbolUses2
                     symbolUsesRangesByDeclaringEntity
-                    (List.ofArray openStatements)
+                    reversedOpenStatements
                     (Dictionary(entityHash))
                     []
 
-            return results |> List.map (fun os -> os.Range)
+            // Re-reverse the results.
+            return results |> List.map _.Range |> List.rev
         }
 
     /// Get the open statements whose contents are not referred to anywhere in the symbol uses.

tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs

@@ -5660,6 +5660,119 @@ type UseTheThings(i:int) =
            (((37, 10), (37, 21)), "open type FSharpEnum2 // Unused, should appear.")]
     unusedOpensData |> shouldEqual expected
 
+[<Theory>]
+[<InlineData("""// 1. Auto-open type extension.
+// https://github.com/dotnet/fsharp/issues/17629
+module Module
+
+type T =
+    static member A = 99
+
+[<AutoOpen>]
+module M =
+    type T with
+        static member Lol = 3
+
+// Shows as unused; the unused opens analyzer will suggest removing it…
+open type T
+
+// …Even though it's used.
+let lol = Lol
+""")>]
+[<InlineData("""// 2. Open type on fully-qualified union type.
+// https://github.com/dotnet/fsharp/issues/17629
+module M =
+    type E = A | B
+
+open type M.E // Considered unused, even though it is.
+
+let f x =
+    match x with
+    | A -> ()
+    | B -> ()
+""")>]
+[<InlineData("""// 3. Open namespace with auto-opened type.
+// https://github.com/dotnet/fsharp/issues/17929
+namespace SomeOtherNamespace
+
+[<AutoOpen>]
+type SomeUnusedType =
+    static member y = 1
+
+namespace Test
+
+open SomeOtherNamespace
+
+type UseTheThings(i:int) =
+    member x.UseSomeOtherNamespaceTypeMember() = y
+""")>]
+let ``Unused opens misc`` fileSource1Text =
+    let fileName1 = Path.ChangeExtension(getTemporaryFileName (), ".fs")
+    let base2 = getTemporaryFileName ()
+    let dllName = Path.ChangeExtension(base2, ".dll")
+    let projFileName = Path.ChangeExtension(base2, ".fsproj")
+    let fileSource1 = SourceText.ofString fileSource1Text
+    FileSystem.OpenFileForWriteShim(fileName1).Write(fileSource1Text)
+
+    let fileNames = [|fileName1|]
+    let args = mkProjectCommandLineArgs (dllName, [])
+    let keepAssemblyContentsChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=CompilerAssertHelpers.UseTransparentCompiler)
+    let options = { keepAssemblyContentsChecker.GetProjectOptionsFromCommandLineArgs (projFileName, args) with SourceFiles = fileNames }
+
+    let fileCheckResults =
+        keepAssemblyContentsChecker.ParseAndCheckFileInProject(fileName1, 0, fileSource1, options)  |> Async.RunImmediate
+        |> function
+            | _, FSharpCheckFileAnswer.Succeeded(res) -> res
+            | _ -> failwithf "Parsing aborted unexpectedly..."
+    let lines = FileSystem.OpenFileForReadShim(fileName1).ReadAllLines()
+    let unusedOpens = UnusedOpens.getUnusedOpens (fileCheckResults, (fun i -> lines[i-1])) |> Async.RunImmediate
+    let unusedOpensData = [ for uo in unusedOpens -> tups uo, lines[uo.StartLine-1] ]
+    unusedOpensData |> shouldEqual []
+
+[<Fact>]
+let ``Unused opens shadowing`` () =
+    let fileSource1Text =
+        """
+// https://github.com/dotnet/fsharp/issues/16226
+namespace Case2
+
+module RecordA =
+    type Record = { Foo: string }
+
+module RecordB =
+    type Record = { Bar: string }
+
+module Use =
+    open RecordB // Not required.
+    open RecordA // Required.
+    open RecordB // Required.
+
+    let convertBToA (recordB: Record) =
+        { Foo = recordB.Bar }
+        """
+
+    let fileName1 = Path.ChangeExtension(getTemporaryFileName (), ".fs")
+    let base2 = getTemporaryFileName ()
+    let dllName = Path.ChangeExtension(base2, ".dll")
+    let projFileName = Path.ChangeExtension(base2, ".fsproj")
+    let fileSource1 = SourceText.ofString fileSource1Text
+    FileSystem.OpenFileForWriteShim(fileName1).Write(fileSource1Text)
+
+    let fileNames = [|fileName1|]
+    let args = mkProjectCommandLineArgs (dllName, [])
+    let keepAssemblyContentsChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=CompilerAssertHelpers.UseTransparentCompiler)
+    let options = { keepAssemblyContentsChecker.GetProjectOptionsFromCommandLineArgs (projFileName, args) with SourceFiles = fileNames }
+
+    let fileCheckResults =
+        keepAssemblyContentsChecker.ParseAndCheckFileInProject(fileName1, 0, fileSource1, options)  |> Async.RunImmediate
+        |> function
+            | _, FSharpCheckFileAnswer.Succeeded(res) -> res
+            | _ -> failwithf "Parsing aborted unexpectedly..."
+    let lines = FileSystem.OpenFileForReadShim(fileName1).ReadAllLines()
+    let unusedOpens = UnusedOpens.getUnusedOpens (fileCheckResults, (fun i -> lines[i-1])) |> Async.RunImmediate
+    let unusedOpensData = [ for uo in unusedOpens -> tups uo, lines[uo.StartLine-1] ]
+    unusedOpensData |> shouldEqual [(((12, 9), (12, 16)), "    open RecordB // Not required.")]
+
 [<Fact>]
 let ``Unused opens smoke test auto open`` () =