diff --git a/src/Compiler/Service/FSharpCheckerResults.fs b/src/Compiler/Service/FSharpCheckerResults.fs index 0abff6d4d04..060bed1b4cf 100644 --- a/src/Compiler/Service/FSharpCheckerResults.fs +++ b/src/Compiler/Service/FSharpCheckerResults.fs @@ -259,11 +259,25 @@ type FSharpSymbolUse(denv: DisplayEnv, symbol: FSharpSymbol, inst: TyparInstanti member this.IsPrivateToFile = let isPrivate = match this.Symbol with - | :? FSharpMemberOrFunctionOrValue as m -> not m.IsModuleValueOrMember || m.Accessibility.IsPrivate + | :? FSharpMemberOrFunctionOrValue as m -> + let fileSignatureLocation = + m.DeclaringEntity |> Option.bind (fun e -> e.SignatureLocation) + + let fileDeclarationLocation = + m.DeclaringEntity |> Option.map (fun e -> e.DeclarationLocation) + + let fileHasSignatureFile = fileSignatureLocation <> fileDeclarationLocation + + let symbolIsNotInSignatureFile = m.SignatureLocation = Some m.DeclarationLocation + + fileHasSignatureFile && symbolIsNotInSignatureFile + || not m.IsModuleValueOrMember + || m.Accessibility.IsPrivate | :? FSharpEntity as m -> m.Accessibility.IsPrivate | :? FSharpGenericParameter -> true | :? FSharpUnionCase as m -> m.Accessibility.IsPrivate | :? FSharpField as m -> m.Accessibility.IsPrivate + | :? FSharpActivePatternCase as m -> m.Accessibility.IsPrivate | _ -> false let declarationLocation = diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index bd6d518dce2..de14de003a5 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -205,12 +205,13 @@ + + + %(RelativeDir)\TestSource\%(Filename)%(Extension) + - - %(RelativeDir)\TestSource\%(Filename)%(Extension) - %(RelativeDir)\BaseLine\%(Filename)%(Extension) diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs index 4f6c1ae0f05..1afd0d56220 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs +++ b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/CommonWorkflows.fs @@ -7,26 +7,16 @@ open Xunit open FSharp.Test.ProjectGeneration -let projectDir = "test-projects" - let makeTestProject () = - let name = $"testProject{Guid.NewGuid().ToString()[..7]}" - let dir = Path.GetFullPath projectDir - { - Name = name - ProjectDir = dir ++ name - SourceFiles = [ - sourceFile "First" [] - sourceFile "Second" ["First"] - sourceFile "Third" ["First"] - { sourceFile "Last" ["Second"; "Third"] with EntryPoint = true } - ] - DependsOn = [] - } + SyntheticProject.Create( + sourceFile "First" [], + sourceFile "Second" ["First"], + sourceFile "Third" ["First"], + { sourceFile "Last" ["Second"; "Third"] with EntryPoint = true }) [] let ``Edit file, check it, then check dependent file`` () = - projectWorkflow (makeTestProject()) { + makeTestProject().Workflow { updateFile "First" breakDependentFiles checkFile "First" expectSignatureChanged saveFile "First" @@ -35,7 +25,7 @@ let ``Edit file, check it, then check dependent file`` () = [] let ``Edit file, don't check it, check dependent file`` () = - projectWorkflow (makeTestProject()) { + makeTestProject().Workflow { updateFile "First" breakDependentFiles saveFile "First" checkFile "Second" expectErrors @@ -43,7 +33,7 @@ let ``Edit file, don't check it, check dependent file`` () = [] let ``Check transitive dependency`` () = - projectWorkflow (makeTestProject()) { + makeTestProject().Workflow { updateFile "First" breakDependentFiles saveFile "First" checkFile "Last" expectSignatureChanged @@ -51,7 +41,7 @@ let ``Check transitive dependency`` () = [] let ``Change multiple files at once`` () = - projectWorkflow (makeTestProject()) { + makeTestProject().Workflow { updateFile "First" (setPublicVersion 2) updateFile "Second" (setPublicVersion 2) updateFile "Third" (setPublicVersion 2) @@ -62,7 +52,7 @@ let ``Change multiple files at once`` () = [] let ``Files depend on signature file if present`` () = (makeTestProject() - |> updateFile "First" (fun f -> { f with HasSignatureFile = true }) + |> updateFile "First" addSignatureFile |> projectWorkflow) { updateFile "First" breakDependentFiles saveFile "First" @@ -71,7 +61,7 @@ let ``Files depend on signature file if present`` () = [] let ``Adding a file`` () = - projectWorkflow (makeTestProject()) { + makeTestProject().Workflow { addFileAbove "Second" (sourceFile "New" []) updateFile "Second" (addDependency "New") saveAll @@ -80,7 +70,7 @@ let ``Adding a file`` () = [] let ``Removing a file`` () = - projectWorkflow (makeTestProject()) { + makeTestProject().Workflow { removeFile "Second" saveAll checkFile "Last" expectErrors @@ -88,20 +78,13 @@ let ``Removing a file`` () = [] let ``Changes in a referenced project`` () = - let name = $"library{Guid.NewGuid().ToString()[..7]}" - let dir = Path.GetFullPath projectDir - let library = { - Name = name - ProjectDir = dir ++ name - SourceFiles = [ sourceFile "Library" [] ] - DependsOn = [] - } + let library = SyntheticProject.Create("library", sourceFile "Library" []) let project = { makeTestProject() with DependsOn = [library] } |> updateFile "First" (addDependency "Library") - projectWorkflow project { + project.Workflow { updateFile "Library" updatePublicSurface saveFile "Library" checkFile "Last" expectSignatureChanged diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SymbolUse.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SymbolUse.fs new file mode 100644 index 00000000000..8549f8beaa9 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SymbolUse.fs @@ -0,0 +1,79 @@ +module FSharp.Compiler.ComponentTests.FSharpChecker.SymbolUse + +open FSharp.Compiler.CodeAnalysis +open Xunit +open FSharp.Test.ProjectGeneration + + +module IsPrivateToFile = + + [] + let ``Function definition in signature file`` () = + let project = SyntheticProject.Create( + sourceFile "First" [] |> addSignatureFile, + sourceFile "Second" ["First"]) + + project.Workflow { + checkFile "First" (fun (typeCheckResult: FSharpCheckFileResults) -> + let symbolUse = typeCheckResult.GetSymbolUseAtLocation(5, 6, "let f2 x = x + 1", ["f2"]) |> Option.defaultWith (fun () -> failwith "no symbol use found") + Assert.False(symbolUse.IsPrivateToFile)) + } + + [] + let ``Function definition, no signature file`` () = + let project = SyntheticProject.Create( + sourceFile "First" [], + sourceFile "Second" ["First"]) + + project.Workflow { + checkFile "First" (fun (typeCheckResult: FSharpCheckFileResults) -> + let symbolUse = typeCheckResult.GetSymbolUseAtLocation(5, 6, "let f2 x = x + 1", ["f2"]) |> Option.defaultWith (fun () -> failwith "no symbol use found") + Assert.False(symbolUse.IsPrivateToFile)) + } + + [] + let ``Function definition not in signature file`` () = + let projectName = "IsPrivateToFileTest1" + let signature = $""" +module {projectName}.ModuleFirst +type TFirstV_1<'a> = | TFirst of 'a +val f: x: 'a -> TFirstV_1<'a> +// no f2 here +""" + let project = SyntheticProject.Create(projectName, + { sourceFile "First" [] with SignatureFile = Custom signature }, + sourceFile "Second" ["First"]) + + project.Workflow { + checkFile "First" (fun (typeCheckResult: FSharpCheckFileResults) -> + let symbolUse = typeCheckResult.GetSymbolUseAtLocation(5, 6, "let f2 x = x + 1", ["f2"]) |> Option.defaultWith (fun () -> failwith "no symbol use found") + Assert.True(symbolUse.IsPrivateToFile)) + } + + [] + let ``Function parameter, no signature file`` () = + SyntheticProject.Create(sourceFile "First" []).Workflow { + checkFile "First" (fun (typeCheckResult: FSharpCheckFileResults) -> + let symbolUse = typeCheckResult.GetSymbolUseAtLocation(5, 8, "let f2 x = x + 1", ["x"]) |> Option.defaultWith (fun () -> failwith "no symbol use found") + Assert.True(symbolUse.IsPrivateToFile)) + } + + /// This is a bug: https://github.com/dotnet/fsharp/issues/14277 + [] + let ``Function parameter, with signature file`` () = + SyntheticProject.Create(sourceFile "First" [] |> addSignatureFile).Workflow { + checkFile "First" (fun (typeCheckResult: FSharpCheckFileResults) -> + let symbolUse = typeCheckResult.GetSymbolUseAtLocation(5, 8, "let f2 x = x + 1", ["x"]) |> Option.defaultWith (fun () -> failwith "no symbol use found") + // This should be false, because it's also in the signature file + Assert.True(symbolUse.IsPrivateToFile)) + } + + [] + let ``Private function, with signature file`` () = + SyntheticProject.Create( + { sourceFile "First" [] with ExtraSource = "let private f3 x = x + 1" } + |> addSignatureFile).Workflow { + checkFile "First" (fun (typeCheckResult: FSharpCheckFileResults) -> + let symbolUse = typeCheckResult.GetSymbolUseAtLocation(6, 14, "let private f3 x = x + 1", ["f3"]) |> Option.defaultWith (fun () -> failwith "no symbol use found") + Assert.False(symbolUse.IsPrivateToFile)) + } \ No newline at end of file diff --git a/tests/FSharp.Test.Utilities/ProjectGeneration.fs b/tests/FSharp.Test.Utilities/ProjectGeneration.fs index 4f3ad9b4aee..7dd50e28d03 100644 --- a/tests/FSharp.Test.Utilities/ProjectGeneration.fs +++ b/tests/FSharp.Test.Utilities/ProjectGeneration.fs @@ -23,11 +23,14 @@ open FSharp.Compiler.Text open Xunit -let private projectRoot = __SOURCE_DIRECTORY__ +let private projectRoot = "test-projects" let private defaultFunctionName = "f" +type SignatureFile = No | AutoGenerated | Custom of string + + type SyntheticSourceFile = { Id: string @@ -37,29 +40,55 @@ type SyntheticSourceFile = DependsOn: string list /// Changing this makes dependent files' code invalid FunctionName: string - HasSignatureFile: bool + SignatureFile: SignatureFile HasErrors: bool + ExtraSource: string EntryPoint: bool } member this.FileName = $"File{this.Id}.fs" member this.SignatureFileName = $"{this.FileName}i" + member this.HasSignatureFile = + match this.SignatureFile with + | No -> false + | _ -> true + + let sourceFile fileId deps = { Id = fileId PublicVersion = 1 InternalVersion = 1 DependsOn = deps FunctionName = defaultFunctionName - HasSignatureFile = false + SignatureFile = No HasErrors = false + ExtraSource = "" EntryPoint = false } type SyntheticProject = { Name: string ProjectDir: string SourceFiles: SyntheticSourceFile list - DependsOn: SyntheticProject list } + DependsOn: SyntheticProject list + RecursiveNamespace: bool } + + static member Create(?name: string) = + let name = defaultArg name $"TestProject_{Guid.NewGuid().ToString()[..7]}" + let dir = Path.GetFullPath projectRoot + { + Name = name + ProjectDir = dir ++ name + SourceFiles = [] + DependsOn = [] + RecursiveNamespace = false + } + + static member Create([] sourceFiles: SyntheticSourceFile[]) = + { SyntheticProject.Create() with SourceFiles = sourceFiles |> List.ofArray } + + static member Create(name: string, [] sourceFiles: SyntheticSourceFile[]) = + { SyntheticProject.Create(name) with SourceFiles = sourceFiles |> List.ofArray } member this.Find fileId = this.SourceFiles @@ -120,7 +149,11 @@ module Internal = let renderSourceFile (project: SyntheticProject) (f: SyntheticSourceFile) = seq { - $"module %s{project.Name}.Module{f.Id}" + if project.RecursiveNamespace then + $"namespace rec {project.Name}" + $"module Module{f.Id}" + else + $"module %s{project.Name}.Module{f.Id}" for p in project.DependsOn do $"open {p.Name}" @@ -136,6 +169,8 @@ module Internal = $"let f2 x = x + {f.InternalVersion}" + f.ExtraSource + if f.HasErrors then "let wrong = 1 + 'a'" @@ -239,6 +274,8 @@ module ProjectOperations = let addDependency fileId f : SyntheticSourceFile = { f with DependsOn = fileId :: f.DependsOn } + let addSignatureFile f = { f with SignatureFile = AutoGenerated } + let checkFile fileId (project: SyntheticProject) (checker: FSharpChecker) = let file = project.Find fileId let contents = renderSourceFile project file @@ -302,12 +339,17 @@ module ProjectOperations = let file = p.SourceFiles[i] writeFile p file - if file.HasSignatureFile && generateSignatureFiles then + let signatureFileName = p.ProjectDir ++ file.SignatureFileName + + match file.SignatureFile with + | AutoGenerated when generateSignatureFiles -> let project = { p with SourceFiles = p.SourceFiles[0..i] } let! results = checkFile file.Id project checker let signature = getSignature results - let signatureFileName = p.ProjectDir ++ file.SignatureFileName writeFileIfChanged signatureFileName signature + | Custom signature -> + writeFileIfChanged signatureFileName signature + | _ -> () writeFileIfChanged (p.ProjectDir ++ $"{p.Name}.fsproj") (renderFsProj p) } @@ -400,6 +442,20 @@ type ProjectWorkflowBuilder(initialProject: SyntheticProject, ?checker: FSharpCh return { ctx with Signatures = ctx.Signatures.Add(fileId, newSignature) } } + /// Parse and type check given file and process the results using `processResults` function. + [] + member this.CheckFile(workflow: Async, fileId: string, processResults) = + async { + let! ctx = workflow + let! results = checkFile fileId ctx.Project checker + let typeCheckResults = getTypeCheckResult results + + let newSignature = getSignature results + + processResults typeCheckResults + + return { ctx with Signatures = ctx.Signatures.Add(fileId, newSignature) } + } /// Save given file to disk. [] member this.SaveFile(workflow: Async, fileId: string) = @@ -422,3 +478,9 @@ type ProjectWorkflowBuilder(initialProject: SyntheticProject, ?checker: FSharpCh /// Execute a set of operations on a given synthetic project. /// The project is saved to disk and type checked at the start. let projectWorkflow project = ProjectWorkflowBuilder project + + +type SyntheticProject with + /// Execute a set of operations on this project. + /// The project is saved to disk and type checked at the start. + member this.Workflow = projectWorkflow this