From 07592d4b327143f44ba8a8625dc9aab8311f61a1 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 8 Aug 2024 17:32:09 +0200 Subject: [PATCH 1/6] WIP some unit tests --- .../Signatures/TypeTests.fs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs b/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs index e3be13782fd..3085eda7d35 100644 --- a/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs @@ -263,3 +263,52 @@ type Foo = member Bar: a: int -> int with get member Bar: a: int -> int with set""" + +[] +let ``IsUnionCaseTester tests`` () = + FSharp """ +module Lib + +type Foo = + | Bar of int + | Baz of string + member this.IsP + with get () = 42 + +let bar = Bar 5 + +let f = bar.get_IsBar +""" + |> withLangVersionPreview + |> typecheckResults + |> fun results -> + let isBarSymbolUse = results.GetSymbolUseAtLocation(12, 21, "let f = bar.get_IsBar", [ "get_IsP" ]).Value + match isBarSymbolUse.Symbol with + | :? FSharpMemberOrFunctionOrValue as mfv -> + Assert.False(mfv.IsUnionCaseTester, "IsUnionCaseTester returned true") + Assert.False(mfv.IsMethod, "IsMethod returned true") + Assert.False(mfv.IsProperty, "IsProptery returned true") + Assert.True(mfv.IsPropertyGetterMethod, "IsPropertyGetterMethod returned false") + | _ -> failwith "Expected FSharpMemberOrFunctionOrValue" + +[] +let ``IsUnionCaseTester tests 2`` () = + FSharp """ +module Lib + +type Foo() = + member _.Bar x = x + +let foo = Foo() +""" + |> withLangVersionPreview + |> typecheckResults + |> fun results -> + let isBarSymbolUse = results.GetSymbolUseAtLocation(7, 13, "let foo = Foo()", [ "Foo" ]).Value + match isBarSymbolUse.Symbol with + | :? FSharpMemberOrFunctionOrValue as mfv -> + Assert.False(mfv.IsUnionCaseTester) + //Assert.False(mfv.IsMethod) + //Assert.False(mfv.IsProperty) + Assert.True(mfv.IsConstructor) + | _ -> failwith "Expected FSharpMemberOrFunctionOrValue" \ No newline at end of file From ac00bff1381e88c217deae075bda9c381bdb67a7 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Wed, 14 Aug 2024 18:27:43 +0200 Subject: [PATCH 2/6] Return false instead of throwing --- src/Compiler/Symbols/Symbols.fs | 2 +- tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compiler/Symbols/Symbols.fs b/src/Compiler/Symbols/Symbols.fs index 29489ee68ee..83b7ec3a4c3 100644 --- a/src/Compiler/Symbols/Symbols.fs +++ b/src/Compiler/Symbols/Symbols.fs @@ -1788,7 +1788,7 @@ type FSharpMemberOrFunctionOrValue(cenv, d:FSharpMemberOrValData, item) = match d with | P p -> p.IsUnionCaseTester | M m -> m.IsUnionCaseTester - | E _ | C _ | V _ -> invalidOp "the value or member is not a property" + | E _ | C _ | V _ -> false member _.EventAddMethod = checkIsResolved() diff --git a/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs b/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs index 3085eda7d35..eae04f9febf 100644 --- a/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs @@ -282,7 +282,7 @@ let f = bar.get_IsBar |> withLangVersionPreview |> typecheckResults |> fun results -> - let isBarSymbolUse = results.GetSymbolUseAtLocation(12, 21, "let f = bar.get_IsBar", [ "get_IsP" ]).Value + let isBarSymbolUse = results.GetSymbolUseAtLocation(12, 21, "let f = bar.get_IsBar", [ "get_IsBar" ]).Value match isBarSymbolUse.Symbol with | :? FSharpMemberOrFunctionOrValue as mfv -> Assert.False(mfv.IsUnionCaseTester, "IsUnionCaseTester returned true") From 687df7c5f0f63c7058c73585e62bbef9865892c8 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 29 Aug 2024 18:03:24 +0200 Subject: [PATCH 3/6] Fix IsUnionCaseTester for generated code Seems like methods/properties that are generated in IL, like `get_Is*` in this case, have `IsMethod` (or `IsProperty`) false for some reason, even when `IsPropertyGetterMethod` is true. This would result in `IsUnionCaseTester` giving incorrect answers. This fixes that at `IsUnionCaseTester`, though it might be worth it to see if it can be fixed at the root of the issue --- src/Compiler/Symbols/Symbols.fs | 6 +- .../Signatures/TypeTests.fs | 12 +- .../FSharp.Compiler.Service.Tests.fsproj | 1 + .../GeneratedCodeSymbolsTests.fs | 136 ++++++++++++++++++ 4 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs diff --git a/src/Compiler/Symbols/Symbols.fs b/src/Compiler/Symbols/Symbols.fs index 83b7ec3a4c3..95427b7914f 100644 --- a/src/Compiler/Symbols/Symbols.fs +++ b/src/Compiler/Symbols/Symbols.fs @@ -1788,7 +1788,11 @@ type FSharpMemberOrFunctionOrValue(cenv, d:FSharpMemberOrValData, item) = match d with | P p -> p.IsUnionCaseTester | M m -> m.IsUnionCaseTester - | E _ | C _ | V _ -> false + | V v -> + v.IsPropertyGetterMethod && + v.LogicalName.StartsWith("get_Is") && + v.IsImplied && v.MemberApparentEntity.IsUnionTycon + | E _ | C _ -> false member _.EventAddMethod = checkIsResolved() diff --git a/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs b/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs index eae04f9febf..1494c465af9 100644 --- a/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs @@ -265,7 +265,7 @@ type Foo = member Bar: a: int -> int with set""" [] -let ``IsUnionCaseTester tests`` () = +let ``get_Is* method has IsUnionCaseTester = true`` () = FSharp """ module Lib @@ -285,14 +285,14 @@ let f = bar.get_IsBar let isBarSymbolUse = results.GetSymbolUseAtLocation(12, 21, "let f = bar.get_IsBar", [ "get_IsBar" ]).Value match isBarSymbolUse.Symbol with | :? FSharpMemberOrFunctionOrValue as mfv -> - Assert.False(mfv.IsUnionCaseTester, "IsUnionCaseTester returned true") - Assert.False(mfv.IsMethod, "IsMethod returned true") + Assert.True(mfv.IsUnionCaseTester, "IsUnionCaseTester returned true") + Assert.True(mfv.IsMethod, "IsMethod returned true") Assert.False(mfv.IsProperty, "IsProptery returned true") Assert.True(mfv.IsPropertyGetterMethod, "IsPropertyGetterMethod returned false") | _ -> failwith "Expected FSharpMemberOrFunctionOrValue" [] -let ``IsUnionCaseTester tests 2`` () = +let ``IsUnionCaseTester does not throw for non-method non-property`` () = FSharp """ module Lib @@ -307,8 +307,6 @@ let foo = Foo() let isBarSymbolUse = results.GetSymbolUseAtLocation(7, 13, "let foo = Foo()", [ "Foo" ]).Value match isBarSymbolUse.Symbol with | :? FSharpMemberOrFunctionOrValue as mfv -> - Assert.False(mfv.IsUnionCaseTester) - //Assert.False(mfv.IsMethod) - //Assert.False(mfv.IsProperty) + Assert.False(mfv.IsUnionCaseTester, "IsUnionCaseTester returned true") Assert.True(mfv.IsConstructor) | _ -> failwith "Expected FSharpMemberOrFunctionOrValue" \ No newline at end of file diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj index bcaa3c0b991..9f0c7f230b9 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj @@ -25,6 +25,7 @@ FsUnit.fs + diff --git a/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs b/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs new file mode 100644 index 00000000000..0b03855faaa --- /dev/null +++ b/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs @@ -0,0 +1,136 @@ +module FSharp.Compiler.Service.Tests.GeneratedCodeSymbolsTests + +open Xunit +open System +open System.Diagnostics +open System.IO +open System.Threading +open FSharp.Compiler.CodeAnalysis +open FSharp.Compiler.IO +open FSharp.Compiler.Service.Tests.Common +open FSharp.Compiler.Symbols +open TestFramework + + +[] +module internal Utils = + let getTempPath dir = + Path.Combine(Path.GetTempPath(), dir) + + /// Returns the file name part of a temp file name created with tryCreateTemporaryFileName () + /// and an added process id and thread id to ensure uniqueness between threads. + let getTempFileName() = + let tempFileName = tryCreateTemporaryFileName () + try + let tempFile, tempExt = Path.GetFileNameWithoutExtension tempFileName, Path.GetExtension tempFileName + let procId, threadId = Process.GetCurrentProcess().Id, Thread.CurrentThread.ManagedThreadId + String.concat "" [tempFile; "_"; string procId; "_"; string threadId; tempExt] // ext includes dot + finally + try + FileSystem.FileDeleteShim tempFileName + with _ -> () + + /// Given just a file name, returns it with changed extension located in %TEMP%\ExprTests + let getTempFilePathChangeExt dir tmp ext = + Path.Combine(getTempPath dir, Path.ChangeExtension(tmp, ext)) + + /// If it doesn't exists, create a folder 'ExprTests' in local user's %TEMP% folder + let createTempDir dirName = + let tempPath = getTempPath dirName + do + if Directory.Exists tempPath then () + else Directory.CreateDirectory tempPath |> ignore + + /// Clean up after a test is run. If you need to inspect the create *.fs files, change this function to do nothing, or just break here. + let cleanupTempFiles dirName files = + { new IDisposable with + member _.Dispose() = + for fileName in files do + try + // cleanup: only the source file is written to the temp dir. + FileSystem.FileDeleteShim fileName + with _ -> () + + try + // remove the dir when empty + let tempPath = getTempPath dirName + if Directory.GetFiles tempPath |> Array.isEmpty then + Directory.Delete tempPath + with _ -> () } + + let createOptionsAux fileSources extraArgs = + let dirName = "GeneratedCodeSymbolsTests" + let fileNames = fileSources |> List.map (fun _ -> getTempFileName()) + let temp2 = getTempFileName() + let fileNames = fileNames |> List.map (fun temp1 -> getTempFilePathChangeExt dirName temp1 ".fs") + let dllName = getTempFilePathChangeExt dirName temp2 ".dll" + let projFileName = getTempFilePathChangeExt dirName temp2 ".fsproj" + + createTempDir dirName + for fileSource: string, fileName in List.zip fileSources fileNames do + FileSystem.OpenFileForWriteShim(fileName).Write(fileSource) + let args = [| yield! extraArgs; yield! mkProjectCommandLineArgs (dllName, []) |] + let options = { checker.GetProjectOptionsFromCommandLineArgs (projFileName, args) with SourceFiles = fileNames |> List.toArray } + + cleanupTempFiles dirName (fileNames @ [dllName; projFileName]), options + +[] +let ``IsUnionCaseTester in generated file`` () = + let source = """ +module Lib + +type T () = + member x.IsM = 1 +""" + let cleanup, options = Utils.createOptionsAux [ source ] [ "--langversion:preview" ] + use _holder = cleanup + let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=false) + let wholeProjectResults = exprChecker.ParseAndCheckProject(options) |> Async.RunImmediate + + let mfvs = + seq { + for implFile in wholeProjectResults.AssemblyContents.ImplementationFiles do + for decl in implFile.Declarations do + match decl with + | FSharpImplementationFileDeclaration.Entity(e,ds) -> + for d in ds do + match d with + | FSharpImplementationFileDeclaration.MemberOrFunctionOrValue (mfv, args, body) -> + yield mfv + | _ -> () + | _ -> () + } + + Assert.Contains(mfvs, fun x -> x.LogicalName = "get_IsM") + let mfv = mfvs |> Seq.filter (fun x -> x.LogicalName = "get_IsM") |> Seq.exactlyOne + Assert.False(mfv.IsUnionCaseTester, $"get_IsM has IsUnionCaseTester = {mfv.IsUnionCaseTester}") + +[] +let ``IsUnionCaseTester in generated file 2`` () = + let source = """ +module Lib + +type T = A | B +""" + let cleanup, options = Utils.createOptionsAux [ source ] [ "--langversion:preview" ] + use _holder = cleanup + let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=false) + let wholeProjectResults = exprChecker.ParseAndCheckProject(options) |> Async.RunImmediate + + let mfvs = + seq { + for implFile in wholeProjectResults.AssemblyContents.ImplementationFiles do + for decl in implFile.Declarations do + match decl with + | FSharpImplementationFileDeclaration.Entity(e,ds) -> + for d in ds do + match d with + | FSharpImplementationFileDeclaration.MemberOrFunctionOrValue (mfv, args, body) -> + yield mfv + | _ -> () + | _ -> () + } + + Assert.Contains(mfvs, fun x -> x.LogicalName = "get_IsA") + let mfv = mfvs |> Seq.filter (fun x -> x.LogicalName = "get_IsA") |> Seq.exactlyOne + Assert.True(mfv.IsUnionCaseTester, $"get_IsA has IsUnionCaseTester = {mfv.IsUnionCaseTester}") \ No newline at end of file From 06a62df2fe1f900ca7423423b50d249a16147525 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Thu, 29 Aug 2024 18:46:23 +0200 Subject: [PATCH 4/6] Add a release note --- docs/release-notes/.FSharp.Compiler.Service/9.0.100.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md index b23aae2e37a..1194b01e662 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/9.0.100.md @@ -13,6 +13,7 @@ * Fixed checking failure when `global` namespace is involved with enabled GraphBasedChecking ([PR #17553](https://github.com/dotnet/fsharp/pull/17553)) * Add missing byte chars notations, enforce limits in decimal notation in byte char & string (Issues [#15867](https://github.com/dotnet/fsharp/issues/15867), [#15868](https://github.com/dotnet/fsharp/issues/15868), [#15869](https://github.com/dotnet/fsharp/issues/15869), [PR #15898](https://github.com/dotnet/fsharp/pull/15898)) * Parentheses analysis: keep extra parentheses around unit & tuples in method definitions. ([PR #17618](https://github.com/dotnet/fsharp/pull/17618)) +* Fix IsUnionCaseTester throwing for non-methods/properties [#17301](https://github.com/dotnet/fsharp/pull/17634) ### Added From 918637b2bae12c0d8b4ef19518a829029c85e1e1 Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 30 Aug 2024 13:06:50 +0200 Subject: [PATCH 5/6] Move helpers around in unit tests for more reuse --- tests/FSharp.Compiler.Service.Tests/Common.fs | 62 ++++++++++++- .../ExprTests.fs | 93 ++++--------------- .../GeneratedCodeSymbolsTests.fs | 74 +-------------- 3 files changed, 81 insertions(+), 148 deletions(-) diff --git a/tests/FSharp.Compiler.Service.Tests/Common.fs b/tests/FSharp.Compiler.Service.Tests/Common.fs index 68e5c68598a..ad9195d25c1 100644 --- a/tests/FSharp.Compiler.Service.Tests/Common.fs +++ b/tests/FSharp.Compiler.Service.Tests/Common.fs @@ -5,10 +5,10 @@ open System open System.Diagnostics open System.IO open System.Collections.Generic +open System.Threading open System.Threading.Tasks open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.IO -open FSharp.Compiler.Diagnostics open FSharp.Compiler.Symbols open FSharp.Compiler.Syntax open FSharp.Compiler.Text @@ -473,3 +473,63 @@ let assertRange Assert.Equal(Position.mkPos expectedStartLine expectedStartColumn, actualRange.Start) Assert.Equal(Position.mkPos expectedEndLine expectedEndColumn, actualRange.End) +[] +module TempDirUtils = + let getTempPath dir = + Path.Combine(Path.GetTempPath(), dir) + + /// Returns the file name part of a temp file name created with tryCreateTemporaryFileName () + /// and an added process id and thread id to ensure uniqueness between threads. + let getTempFileName() = + let tempFileName = tryCreateTemporaryFileName () + try + let tempFile, tempExt = Path.GetFileNameWithoutExtension tempFileName, Path.GetExtension tempFileName + let procId, threadId = Process.GetCurrentProcess().Id, Thread.CurrentThread.ManagedThreadId + String.concat "" [tempFile; "_"; string procId; "_"; string threadId; tempExt] // ext includes dot + finally + try + FileSystem.FileDeleteShim tempFileName + with _ -> () + + /// Given just a file name, returns it with changed extension located in %TEMP%\ExprTests + let getTempFilePathChangeExt dir tmp ext = + Path.Combine(getTempPath dir, Path.ChangeExtension(tmp, ext)) + + /// If it doesn't exists, create a folder 'ExprTests' in local user's %TEMP% folder + let createTempDir dirName = + let tempPath = getTempPath dirName + do + if Directory.Exists tempPath then () + else Directory.CreateDirectory tempPath |> ignore + + /// Clean up after a test is run. If you need to inspect the create *.fs files, change this function to do nothing, or just break here. + let cleanupTempFiles dirName files = + { new IDisposable with + member _.Dispose() = + for fileName in files do + try + // cleanup: only the source file is written to the temp dir. + FileSystem.FileDeleteShim fileName + with _ -> () + + try + // remove the dir when empty + let tempPath = getTempPath dirName + if Directory.GetFiles tempPath |> Array.isEmpty then + Directory.Delete tempPath + with _ -> () } + + let createProjectOptions dirName fileSources extraArgs = + let fileNames = fileSources |> List.map (fun _ -> getTempFileName()) + let temp2 = getTempFileName() + let fileNames = fileNames |> List.map (fun temp1 -> getTempFilePathChangeExt dirName temp1 ".fs") + let dllName = getTempFilePathChangeExt dirName temp2 ".dll" + let projFileName = getTempFilePathChangeExt dirName temp2 ".fsproj" + + createTempDir dirName + for fileSource: string, fileName in List.zip fileSources fileNames do + FileSystem.OpenFileForWriteShim(fileName).Write(fileSource) + let args = [| yield! extraArgs; yield! mkProjectCommandLineArgs (dllName, []) |] + let options = { checker.GetProjectOptionsFromCommandLineArgs (projFileName, args) with SourceFiles = fileNames |> List.toArray } + + cleanupTempFiles dirName (fileNames @ [dllName; projFileName]), options diff --git a/tests/FSharp.Compiler.Service.Tests/ExprTests.fs b/tests/FSharp.Compiler.Service.Tests/ExprTests.fs index 6c7c755b0ce..523a70cbea7 100644 --- a/tests/FSharp.Compiler.Service.Tests/ExprTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/ExprTests.fs @@ -3,18 +3,14 @@ open Xunit open FsUnit open System -open System.IO open System.Text open System.Collections.Generic -open System.Diagnostics -open System.Threading open FSharp.Compiler.CodeAnalysis open FSharp.Compiler.Diagnostics open FSharp.Compiler.IO open FSharp.Compiler.Service.Tests.Common open FSharp.Compiler.Symbols open FSharp.Compiler.Symbols.FSharpExprPatterns -open TestFramework type FSharpCore = | FC45 @@ -29,52 +25,11 @@ type FSharpCore = | FC47 -> "FSharp.Core 4.7" | FC50 -> "FSharp.Core 5.0" +[] +let dirName = "ExprTests" [] module internal Utils = - let getTempPath() = - Path.Combine(Path.GetTempPath(), "ExprTests") - - /// If it doesn't exists, create a folder 'ExprTests' in local user's %TEMP% folder - let createTempDir() = - let tempPath = getTempPath() - do - if Directory.Exists tempPath then () - else Directory.CreateDirectory tempPath |> ignore - - /// Returns the file name part of a temp file name created with tryCreateTemporaryFileName () - /// and an added process id and thread id to ensure uniqueness between threads. - let getTempFileName() = - let tempFileName = tryCreateTemporaryFileName () - try - let tempFile, tempExt = Path.GetFileNameWithoutExtension tempFileName, Path.GetExtension tempFileName - let procId, threadId = Process.GetCurrentProcess().Id, Thread.CurrentThread.ManagedThreadId - String.concat "" [tempFile; "_"; string procId; "_"; string threadId; tempExt] // ext includes dot - finally - try - FileSystem.FileDeleteShim tempFileName - with _ -> () - - /// Clean up after a test is run. If you need to inspect the create *.fs files, change this function to do nothing, or just break here. - let cleanupTempFiles files = - { new IDisposable with - member _.Dispose() = - for fileName in files do - try - // cleanup: only the source file is written to the temp dir. - FileSystem.FileDeleteShim fileName - with _ -> () - - try - // remove the dir when empty - let tempPath = getTempPath() - if Directory.GetFiles tempPath |> Array.isEmpty then - Directory.Delete tempPath - with _ -> () } - - /// Given just a file name, returns it with changed extension located in %TEMP%\ExprTests - let getTempFilePathChangeExt tmp ext = - Path.Combine(getTempPath(), Path.ChangeExtension(tmp, ext)) // This behaves slightly differently on Mono versions, 'null' is printed sometimes, 'None' other times // Presumably this is very small differences in Mono reflection causing F# printing to change behaviour @@ -345,22 +300,6 @@ module internal Utils = yield! collectMembers e |> Seq.map printMemberSignature } - -let createOptionsAux fileSources extraArgs = - let fileNames = fileSources |> List.map (fun _ -> Utils.getTempFileName()) - let temp2 = Utils.getTempFileName() - let fileNames = fileNames |> List.map (fun temp1 -> Utils.getTempFilePathChangeExt temp1 ".fs") - let dllName = Utils.getTempFilePathChangeExt temp2 ".dll" - let projFileName = Utils.getTempFilePathChangeExt temp2 ".fsproj" - - Utils.createTempDir() - for fileSource: string, fileName in List.zip fileSources fileNames do - FileSystem.OpenFileForWriteShim(fileName).Write(fileSource) - let args = [| yield! extraArgs; yield! mkProjectCommandLineArgs (dllName, []) |] - let options = { checker.GetProjectOptionsFromCommandLineArgs (projFileName, args) with SourceFiles = fileNames |> List.toArray } - - Utils.cleanupTempFiles (fileNames @ [dllName; projFileName]), options - //--------------------------------------------------------------------------------------------------------- // This project is a smoke test for a whole range of standard and obscure expressions @@ -653,7 +592,7 @@ let testMutableVar = mutableVar 1 let testMutableConst = mutableConst () """ - let createOptionsWithArgs args = createOptionsAux [ fileSource1; fileSource2 ] args + let createOptionsWithArgs args = createProjectOptions dirName [ fileSource1; fileSource2 ] args let createOptions() = createOptionsWithArgs [] @@ -1002,15 +941,15 @@ let ``Test Optimized Declarations Project1`` useTransparentCompiler = let testOperators dnName fsName excludedTests expectedUnoptimized expectedOptimized = - let tempFileName = Utils.getTempFileName() - let filePath = Utils.getTempFilePathChangeExt tempFileName ".fs" - let dllPath =Utils.getTempFilePathChangeExt tempFileName ".dll" - let projFilePath = Utils.getTempFilePathChangeExt tempFileName ".fsproj" + let tempFileName = getTempFileName() + let filePath = getTempFilePathChangeExt dirName tempFileName ".fs" + let dllPath =getTempFilePathChangeExt dirName tempFileName ".dll" + let projFilePath = getTempFilePathChangeExt dirName tempFileName ".fsproj" let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=true) begin - use _cleanup = Utils.cleanupTempFiles [filePath; dllPath; projFilePath] - createTempDir() + use _cleanup = cleanupTempFiles dirName [filePath; dllPath; projFilePath] + createTempDir dirName let source = String.Format(Project1.operatorTests, dnName, fsName) let replace (s:string) r = s.Replace("let " + r, "// let " + r) let fileSource = excludedTests |> List.fold replace source @@ -3192,7 +3131,7 @@ let BigSequenceExpression(outFileOpt,docFileOpt,baseAddressOpt) = """ - let createOptions() = createOptionsAux [fileSource1] [] + let createOptions() = createProjectOptions dirName [fileSource1] [] #if !NETFRAMEWORK && DEBUG [] @@ -3279,7 +3218,7 @@ let f7() = callXY (C()) (D()) let f8() = callXY (D()) (C()) """ - let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"] + let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"] [] [] @@ -3407,7 +3346,7 @@ type MyNumberWrapper = { MyNumber: MyNumber } """ - let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"] + let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"] [] [] @@ -3465,13 +3404,13 @@ let s2 = sign p1 """ - let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"] + let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"] [] [] [] let ``Test ProjectForWitnesses3`` useTransparentCompiler = - let cleanup, options = createOptionsAux [ ProjectForWitnesses3.fileSource1 ] ["--langversion:7.0"] + let cleanup, options = createProjectOptions dirName [ ProjectForWitnesses3.fileSource1 ] ["--langversion:7.0"] use _holder = cleanup let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=useTransparentCompiler) let wholeProjectResults = exprChecker.ParseAndCheckProject(options) |> Async.RunImmediate @@ -3563,7 +3502,7 @@ let isNullQuoted (ts : 't[]) = """ - let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"] + let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"] [] [] @@ -3603,7 +3542,7 @@ module N.M let rec f = new System.EventHandler(fun _ _ -> f.Invoke(null,null)) """ - let createOptions() = createOptionsAux [fileSource1] [] + let createOptions() = createProjectOptions dirName [fileSource1] [] [] [] diff --git a/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs b/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs index 0b03855faaa..235bc40ec52 100644 --- a/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs @@ -1,78 +1,12 @@ module FSharp.Compiler.Service.Tests.GeneratedCodeSymbolsTests open Xunit -open System -open System.Diagnostics -open System.IO -open System.Threading open FSharp.Compiler.CodeAnalysis -open FSharp.Compiler.IO open FSharp.Compiler.Service.Tests.Common open FSharp.Compiler.Symbols -open TestFramework - -[] -module internal Utils = - let getTempPath dir = - Path.Combine(Path.GetTempPath(), dir) - - /// Returns the file name part of a temp file name created with tryCreateTemporaryFileName () - /// and an added process id and thread id to ensure uniqueness between threads. - let getTempFileName() = - let tempFileName = tryCreateTemporaryFileName () - try - let tempFile, tempExt = Path.GetFileNameWithoutExtension tempFileName, Path.GetExtension tempFileName - let procId, threadId = Process.GetCurrentProcess().Id, Thread.CurrentThread.ManagedThreadId - String.concat "" [tempFile; "_"; string procId; "_"; string threadId; tempExt] // ext includes dot - finally - try - FileSystem.FileDeleteShim tempFileName - with _ -> () - - /// Given just a file name, returns it with changed extension located in %TEMP%\ExprTests - let getTempFilePathChangeExt dir tmp ext = - Path.Combine(getTempPath dir, Path.ChangeExtension(tmp, ext)) - - /// If it doesn't exists, create a folder 'ExprTests' in local user's %TEMP% folder - let createTempDir dirName = - let tempPath = getTempPath dirName - do - if Directory.Exists tempPath then () - else Directory.CreateDirectory tempPath |> ignore - - /// Clean up after a test is run. If you need to inspect the create *.fs files, change this function to do nothing, or just break here. - let cleanupTempFiles dirName files = - { new IDisposable with - member _.Dispose() = - for fileName in files do - try - // cleanup: only the source file is written to the temp dir. - FileSystem.FileDeleteShim fileName - with _ -> () - - try - // remove the dir when empty - let tempPath = getTempPath dirName - if Directory.GetFiles tempPath |> Array.isEmpty then - Directory.Delete tempPath - with _ -> () } - - let createOptionsAux fileSources extraArgs = - let dirName = "GeneratedCodeSymbolsTests" - let fileNames = fileSources |> List.map (fun _ -> getTempFileName()) - let temp2 = getTempFileName() - let fileNames = fileNames |> List.map (fun temp1 -> getTempFilePathChangeExt dirName temp1 ".fs") - let dllName = getTempFilePathChangeExt dirName temp2 ".dll" - let projFileName = getTempFilePathChangeExt dirName temp2 ".fsproj" - - createTempDir dirName - for fileSource: string, fileName in List.zip fileSources fileNames do - FileSystem.OpenFileForWriteShim(fileName).Write(fileSource) - let args = [| yield! extraArgs; yield! mkProjectCommandLineArgs (dllName, []) |] - let options = { checker.GetProjectOptionsFromCommandLineArgs (projFileName, args) with SourceFiles = fileNames |> List.toArray } - - cleanupTempFiles dirName (fileNames @ [dllName; projFileName]), options +[] +let dirName = "GeneratedCodeSymbolsTests" [] let ``IsUnionCaseTester in generated file`` () = @@ -82,7 +16,7 @@ module Lib type T () = member x.IsM = 1 """ - let cleanup, options = Utils.createOptionsAux [ source ] [ "--langversion:preview" ] + let cleanup, options = createProjectOptions dirName [ source ] [ "--langversion:preview" ] use _holder = cleanup let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=false) let wholeProjectResults = exprChecker.ParseAndCheckProject(options) |> Async.RunImmediate @@ -112,7 +46,7 @@ module Lib type T = A | B """ - let cleanup, options = Utils.createOptionsAux [ source ] [ "--langversion:preview" ] + let cleanup, options = createProjectOptions dirName [ source ] [ "--langversion:preview" ] use _holder = cleanup let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=false) let wholeProjectResults = exprChecker.ParseAndCheckProject(options) |> Async.RunImmediate From e5160a82a6e509300d61581cf07ced9e180b0d4f Mon Sep 17 00:00:00 2001 From: Adam Boniecki Date: Fri, 30 Aug 2024 15:29:50 +0200 Subject: [PATCH 6/6] Add negative unit test --- .../GeneratedCodeSymbolsTests.fs | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs b/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs index 235bc40ec52..66b9782fb29 100644 --- a/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs @@ -9,7 +9,7 @@ open FSharp.Compiler.Symbols let dirName = "GeneratedCodeSymbolsTests" [] -let ``IsUnionCaseTester in generated file`` () = +let ``IsUnionCaseTester for Is* member in a class`` () = let source = """ module Lib @@ -40,7 +40,7 @@ type T () = Assert.False(mfv.IsUnionCaseTester, $"get_IsM has IsUnionCaseTester = {mfv.IsUnionCaseTester}") [] -let ``IsUnionCaseTester in generated file 2`` () = +let ``IsUnionCaseTester for Is* generated property in DU`` () = let source = """ module Lib @@ -67,4 +67,38 @@ type T = A | B Assert.Contains(mfvs, fun x -> x.LogicalName = "get_IsA") let mfv = mfvs |> Seq.filter (fun x -> x.LogicalName = "get_IsA") |> Seq.exactlyOne - Assert.True(mfv.IsUnionCaseTester, $"get_IsA has IsUnionCaseTester = {mfv.IsUnionCaseTester}") \ No newline at end of file + Assert.True(mfv.IsUnionCaseTester, $"get_IsA has IsUnionCaseTester = {mfv.IsUnionCaseTester}") + +[] +let ``IsUnionCaseTester for Is* user property in DU`` () = + let source = """ +module Lib + +type T = + | A + | B + member x.IsC + with get () = false +""" + let cleanup, options = createProjectOptions dirName [ source ] [ "--langversion:preview" ] + use _holder = cleanup + let exprChecker = FSharpChecker.Create(keepAssemblyContents=true, useTransparentCompiler=false) + let wholeProjectResults = exprChecker.ParseAndCheckProject(options) |> Async.RunImmediate + + let mfvs = + seq { + for implFile in wholeProjectResults.AssemblyContents.ImplementationFiles do + for decl in implFile.Declarations do + match decl with + | FSharpImplementationFileDeclaration.Entity(e,ds) -> + for d in ds do + match d with + | FSharpImplementationFileDeclaration.MemberOrFunctionOrValue (mfv, args, body) -> + yield mfv + | _ -> () + | _ -> () + } + + Assert.Contains(mfvs, fun x -> x.LogicalName = "get_IsC") + let mfv = mfvs |> Seq.filter (fun x -> x.LogicalName = "get_IsC") |> Seq.exactlyOne + Assert.False(mfv.IsUnionCaseTester, $"get_IsC has IsUnionCaseTester = {mfv.IsUnionCaseTester}") \ No newline at end of file