diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md index 16f5721e22a..b760f7b02b2 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.300.md +++ b/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 `[]` 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)) diff --git a/src/Compiler/Service/ServiceAnalysis.fs b/src/Compiler/Service/ServiceAnalysis.fs index 6455d9f0ff3..2a9af9a8569 100644 --- a/src/Compiler/Service/ServiceAnalysis.fs +++ b/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()) + 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() then + if ent.HasAttribute() 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. diff --git a/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs b/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs index cef9b119c77..686d10c0a83 100644 --- a/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs +++ b/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 +[] +[] +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 +""")>] +[ () + | B -> () +""")>] +[] +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 [] + +[] +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.")] + [] let ``Unused opens smoke test auto open`` () =