Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not throw from IsUnionCaseTester and handle a case where symbol is not considered a method/property #17634

Merged
merged 7 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.100.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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)
* Consider `open type` used when the type is an enum and any of the enum cases is used unqualified. ([PR #17628](https://github.com/dotnet/fsharp/pull/17628))

### Added
Expand Down
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 _ -> invalidOp "the value or member is not a property"
| V v ->
v.IsPropertyGetterMethod &&
v.LogicalName.StartsWith("get_Is") &&
v.IsImplied && v.MemberApparentEntity.IsUnionTycon
| E _ | C _ -> false

member _.EventAddMethod =
checkIsResolved()
Expand Down
47 changes: 47 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/Signatures/TypeTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,50 @@ type Foo =
member Bar: a: int -> int with get

member Bar: a: int -> int with set"""

[<Fact>]
let ``get_Is* method has IsUnionCaseTester = true`` () =
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_IsBar" ]).Value
match isBarSymbolUse.Symbol with
| :? FSharpMemberOrFunctionOrValue as mfv ->
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 does not throw for non-method non-property`` () =
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, "IsUnionCaseTester returned true")
Assert.True(mfv.IsConstructor)
| _ -> failwith "Expected FSharpMemberOrFunctionOrValue"
62 changes: 61 additions & 1 deletion tests/FSharp.Compiler.Service.Tests/Common.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -473,3 +473,63 @@ let assertRange
Assert.Equal(Position.mkPos expectedStartLine expectedStartColumn, actualRange.Start)
Assert.Equal(Position.mkPos expectedEndLine expectedEndColumn, actualRange.End)

[<AutoOpen>]
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
93 changes: 16 additions & 77 deletions tests/FSharp.Compiler.Service.Tests/ExprTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -29,52 +25,11 @@ type FSharpCore =
| FC47 -> "FSharp.Core 4.7"
| FC50 -> "FSharp.Core 5.0"

[<Literal>]
let dirName = "ExprTests"

[<AutoOpen>]
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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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 []

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -3192,7 +3131,7 @@ let BigSequenceExpression(outFileOpt,docFileOpt,baseAddressOpt) =
"""


let createOptions() = createOptionsAux [fileSource1] []
let createOptions() = createProjectOptions dirName [fileSource1] []

#if !NETFRAMEWORK && DEBUG
[<Fact(Skip = "Test is known to fail in DEBUG when not using NetFramework. Use RELEASE configuration or NetFramework to run it.")>]
Expand Down Expand Up @@ -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"]

[<Theory>]
[<InlineData(false)>]
Expand Down Expand Up @@ -3407,7 +3346,7 @@ type MyNumberWrapper =
{ MyNumber: MyNumber }
"""

let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"]
let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"]

[<Theory>]
[<InlineData(false)>]
Expand Down Expand Up @@ -3465,13 +3404,13 @@ let s2 = sign p1

"""

let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"]
let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"]

[<Theory>]
[<InlineData(false)>]
[<InlineData(true)>]
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
Expand Down Expand Up @@ -3563,7 +3502,7 @@ let isNullQuoted (ts : 't[]) =

"""

let createOptions() = createOptionsAux [fileSource1] ["--langversion:7.0"]
let createOptions() = createProjectOptions dirName [fileSource1] ["--langversion:7.0"]

[<Theory>]
[<InlineData(false)>]
Expand Down Expand Up @@ -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] []

[<Theory>]
[<InlineData(false)>]
Expand Down
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
Loading
Loading