Skip to content

Commit

Permalink
Graph typechecking issue fix when using global namespace. (dotnet#1…
Browse files Browse the repository at this point in the history
…7553)

* Added test

* Skip the global namespace when transforming longident to path

* Fixed the case

* Fixed the case

* Make it recursive call

* Format

* Fixed case

* Release notes

* Update test

* Update logic

* Added one more test
  • Loading branch information
vzarytovskii authored Aug 19, 2024
1 parent d90885c commit 53ff8ee
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 12 deletions.
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 @@ -10,6 +10,7 @@
* Fix missing message for type error (FS0001). ([Issue #17373](https://github.com/dotnet/fsharp/issues/17373), [PR #17516](https://github.com/dotnet/fsharp/pull/17516))
* Nullness export - make sure option<> and other UseNullAsTrueValue types are properly annotated as nullable for C# and reflection consumers [PR #17528](https://github.com/dotnet/fsharp/pull/17528)
* MethodAccessException on equality comparison of a type private to module. ([Issue #17541](https://github.com/dotnet/fsharp/issues/17541), [PR #17548](https://github.com/dotnet/fsharp/pull/17548))
* Fixed checking failure when `global` namespace is involved with enabled GraphBasedChecking ([PR #17553](https://github.com/dotnet/fsharp/pull/17553))

### Added

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Driver/GraphChecking/DependencyResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ let rec processStateEntry (trie: TrieNode) (state: FileContentQueryState) (entry
FoundDependencies = foundDependencies
}

| ModuleName name ->
| FileContentEntry.ModuleName name ->
// We need to check if the module name is a hit in the Trie.
let state' =
let queryResult = queryTrie trie [ name ]
Expand Down
7 changes: 7 additions & 0 deletions src/Compiler/Driver/GraphChecking/FileContentMapping.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ type Continuations = ((FileContentEntry list -> FileContentEntry list) -> FileCo
let collectFromOption (mapping: 'T -> 'U list) (t: 'T option) : 'U list = List.collect mapping (Option.toList t)

let longIdentToPath (skipLast: bool) (longId: LongIdent) : LongIdentifier =

// We always skip the "special" `global` identifier.
let longId =
match longId with
| h :: t when h.idText = "`global`" -> t
| _ -> longId

match skipLast, longId with
| true, _ :: _ -> List.take (longId.Length - 1) longId
| _ -> longId
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/Driver/GraphChecking/Types.fs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type internal TrieNode =

/// A significant construct found in the syntax tree of a file.
/// This construct needs to be processed in order to deduce potential links to other files in the project.
[<RequireQualifiedAccess; NoComparison; NoEquality>]
type internal FileContentEntry =
/// Any toplevel namespace a file might have.
/// In case a file has `module X.Y.Z`, then `X.Y` is considered to be the toplevel namespace
Expand Down
1 change: 1 addition & 0 deletions src/Compiler/Driver/GraphChecking/Types.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ type internal TrieNode =

/// A significant construct found in the syntax tree of a file.
/// This construct needs to be processed in order to deduce potential links to other files in the project.
[<RequireQualifiedAccess; NoComparison; NoEquality>]
type internal FileContentEntry =
/// Any toplevel namespace a file might have.
/// In case a file has `module X.Y.Z`, then `X.Y` is considered to be the toplevel namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

open FSharp.Test
open FSharp.Test.Compiler
open NUnit.Framework
open Xunit;
open Scenarios

[<Struct>]
Expand Down Expand Up @@ -44,12 +44,14 @@ let compileAValidScenario (scenario: Scenario) (method: Method) =
|> shouldSucceed
|> ignore

let scenarios = codebases
let scenarios = codebases |> List.map (fun c -> [| box c |]) |> Array.ofList

[<TestCaseSource(nameof scenarios)>]
let ``Compile a valid scenario using graph-based type-checking`` (scenario: Scenario) =
[<Theory>]
[<MemberData(nameof scenarios)>]
let ``Compile a valid scenario using graph-based type-checking`` (scenario) =
compileAValidScenario scenario Method.Graph

[<TestCaseSource(nameof scenarios)>]
let ``Compile a valid scenario using sequential type-checking`` (scenario: Scenario) =
[<Theory>]
[<MemberData(nameof scenarios)>]
let ``Compile a valid scenario using sequential type-checking`` (scenario) =
compileAValidScenario scenario Method.Sequential
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ let private nestedModule name content =

let private prefIdent (lid: string) =
let parts = lid.Split(".")
Array.take (parts.Length - 1) parts |> List.ofArray |> PrefixedIdentifier
Array.take (parts.Length - 1) parts |> List.ofArray |> FileContentEntry.PrefixedIdentifier

// Some hardcoded files that reflect the file content of the first files in the Fantomas.Core project.
// See https://github.com/fsprojects/fantomas/tree/0938a3daabec80a22d2e17f82aba38456bb793df/src/Fantomas.Core
Expand Down
56 changes: 52 additions & 4 deletions tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ module Y.C
// This open statement does not do anything.
// It can safely be removed, but because of its presence we need to link it to something that exposes the namespace X.
// We try and pick the file with the lowest index
// We try and pick the file with the lowest index
open X
let c = 0
Expand Down Expand Up @@ -572,7 +572,7 @@ type Bar() =
static member Foo () : unit =
failwith ""
let Foo () : unit =
let Foo () : unit =
Bar.Foo ()
"""
(set [| 0 |])
Expand Down Expand Up @@ -792,7 +792,7 @@ open My.Great.Namespace
type Foo = class end
"""
(set [| 0 |])

sourceFile
"Program"
"""
Expand Down Expand Up @@ -897,7 +897,7 @@ do
"""
(set [| 0 |])
]

scenario
"parentheses around module name in nameof expression"
[
Expand Down Expand Up @@ -1042,4 +1042,52 @@ type Bar(foo: MyRootNamespace.A.Foo, s: string) = class end
"""
(set [| 0 |])
]
scenario
"Library with using global namespace"
[
sourceFile
"Library.fs"
"""
namespace Lib
module File1 =
let mutable discState = System.DateTime.Now
module File2 =
[<Struct>]
type DiscState(rep : int) =
member this.Rep = rep
let mutable discState = DiscState(0)
"""
Set.empty
sourceFile
"App.fs"
"""
module X
let v = global.Lib.File1.discState.Second
let v2 = global.Lib.File2.discState.Rep
"""
(set [| 0 |])
]
scenario
"Library with using global namespace as module alias"
[
sourceFile
"Library.fs"
"""
namespace Z
module N =
let mutable discState = System.DateTime.Now
"""
Set.empty
sourceFile
"App.fs"
"""
module X
module Y = global.Z.N
"""
(set [| 0 |])
]
]

0 comments on commit 53ff8ee

Please sign in to comment.