Skip to content

Commit

Permalink
Fix IsUnionCaseTester for generated code
Browse files Browse the repository at this point in the history
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
  • Loading branch information
abonie committed Aug 29, 2024
1 parent ac00bff commit e828e54
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 8 deletions.
6 changes: 5 additions & 1 deletion src/Compiler/Symbols/Symbols.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 5 additions & 7 deletions tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ type Foo =
member Bar: a: int -> int with set"""

[<Fact>]
let ``IsUnionCaseTester tests`` () =
let ``get_Is* method has IsUnionCaseTester = true`` () =
FSharp """
module Lib
Expand All @@ -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"

[<Fact>]
let ``IsUnionCaseTester tests 2`` () =
let ``IsUnionCaseTester does not throw for non-method non-property`` () =
FSharp """
module Lib
Expand All @@ -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"
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<Link>FsUnit.fs</Link>
</Compile>
<Compile Include="Common.fs" />
<Compile Include="GeneratedCodeSymbolsTests.fs" />
<Compile Include="AssemblyReaderShim.fs" />
<Compile Include="ModuleReaderCancellationTests.fs" />
<Compile Include="EditorTests.fs" />
Expand Down
137 changes: 137 additions & 0 deletions tests/FSharp.Compiler.Service.Tests/GeneratedCodeSymbolsTests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
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

let checker = FSharpChecker.Create(useTransparentCompiler=FSharp.Compiler.CompilerConfig.FSharpExperimentalFeaturesEnabledAutomatically)

[<AutoOpen>]
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

[<Fact>]
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}")

[<Fact>]
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}")

0 comments on commit e828e54

Please sign in to comment.