Skip to content

Commit 53ff8ee

Browse files
authored
Graph typechecking issue fix when using global namespace. (#17553)
* 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
1 parent d90885c commit 53ff8ee

File tree

8 files changed

+72
-12
lines changed

8 files changed

+72
-12
lines changed

docs/release-notes/.FSharp.Compiler.Service/9.0.100.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
* 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))
1111
* 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)
1212
* 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))
13+
* Fixed checking failure when `global` namespace is involved with enabled GraphBasedChecking ([PR #17553](https://github.com/dotnet/fsharp/pull/17553))
1314

1415
### Added
1516

src/Compiler/Driver/GraphChecking/DependencyResolution.fs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ let rec processStateEntry (trie: TrieNode) (state: FileContentQueryState) (entry
117117
FoundDependencies = foundDependencies
118118
}
119119

120-
| ModuleName name ->
120+
| FileContentEntry.ModuleName name ->
121121
// We need to check if the module name is a hit in the Trie.
122122
let state' =
123123
let queryResult = queryTrie trie [ name ]

src/Compiler/Driver/GraphChecking/FileContentMapping.fs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ type Continuations = ((FileContentEntry list -> FileContentEntry list) -> FileCo
99
let collectFromOption (mapping: 'T -> 'U list) (t: 'T option) : 'U list = List.collect mapping (Option.toList t)
1010

1111
let longIdentToPath (skipLast: bool) (longId: LongIdent) : LongIdentifier =
12+
13+
// We always skip the "special" `global` identifier.
14+
let longId =
15+
match longId with
16+
| h :: t when h.idText = "`global`" -> t
17+
| _ -> longId
18+
1219
match skipLast, longId with
1320
| true, _ :: _ -> List.take (longId.Length - 1) longId
1421
| _ -> longId

src/Compiler/Driver/GraphChecking/Types.fs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ type internal TrieNode =
6161

6262
/// A significant construct found in the syntax tree of a file.
6363
/// This construct needs to be processed in order to deduce potential links to other files in the project.
64+
[<RequireQualifiedAccess; NoComparison; NoEquality>]
6465
type internal FileContentEntry =
6566
/// Any toplevel namespace a file might have.
6667
/// In case a file has `module X.Y.Z`, then `X.Y` is considered to be the toplevel namespace

src/Compiler/Driver/GraphChecking/Types.fsi

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ type internal TrieNode =
5555

5656
/// A significant construct found in the syntax tree of a file.
5757
/// This construct needs to be processed in order to deduce potential links to other files in the project.
58+
[<RequireQualifiedAccess; NoComparison; NoEquality>]
5859
type internal FileContentEntry =
5960
/// Any toplevel namespace a file might have.
6061
/// In case a file has `module X.Y.Z`, then `X.Y` is considered to be the toplevel namespace

tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/CompilationTests.fs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
open FSharp.Test
44
open FSharp.Test.Compiler
5-
open NUnit.Framework
5+
open Xunit;
66
open Scenarios
77

88
[<Struct>]
@@ -44,12 +44,14 @@ let compileAValidScenario (scenario: Scenario) (method: Method) =
4444
|> shouldSucceed
4545
|> ignore
4646

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

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

53-
[<TestCaseSource(nameof scenarios)>]
54-
let ``Compile a valid scenario using sequential type-checking`` (scenario: Scenario) =
54+
[<Theory>]
55+
[<MemberData(nameof scenarios)>]
56+
let ``Compile a valid scenario using sequential type-checking`` (scenario) =
5557
compileAValidScenario scenario Method.Sequential

tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/QueryTrieTests.fs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ let private nestedModule name content =
3636

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

4141
// Some hardcoded files that reflect the file content of the first files in the Fantomas.Core project.
4242
// See https://github.com/fsprojects/fantomas/tree/0938a3daabec80a22d2e17f82aba38456bb793df/src/Fantomas.Core

tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ module Y.C
256256
257257
// This open statement does not do anything.
258258
// It can safely be removed, but because of its presence we need to link it to something that exposes the namespace X.
259-
// We try and pick the file with the lowest index
259+
// We try and pick the file with the lowest index
260260
open X
261261
262262
let c = 0
@@ -572,7 +572,7 @@ type Bar() =
572572
static member Foo () : unit =
573573
failwith ""
574574
575-
let Foo () : unit =
575+
let Foo () : unit =
576576
Bar.Foo ()
577577
"""
578578
(set [| 0 |])
@@ -792,7 +792,7 @@ open My.Great.Namespace
792792
type Foo = class end
793793
"""
794794
(set [| 0 |])
795-
795+
796796
sourceFile
797797
"Program"
798798
"""
@@ -897,7 +897,7 @@ do
897897
"""
898898
(set [| 0 |])
899899
]
900-
900+
901901
scenario
902902
"parentheses around module name in nameof expression"
903903
[
@@ -1042,4 +1042,52 @@ type Bar(foo: MyRootNamespace.A.Foo, s: string) = class end
10421042
"""
10431043
(set [| 0 |])
10441044
]
1045+
scenario
1046+
"Library with using global namespace"
1047+
[
1048+
sourceFile
1049+
"Library.fs"
1050+
"""
1051+
namespace Lib
1052+
module File1 =
1053+
let mutable discState = System.DateTime.Now
1054+
1055+
module File2 =
1056+
[<Struct>]
1057+
type DiscState(rep : int) =
1058+
member this.Rep = rep
1059+
1060+
let mutable discState = DiscState(0)
1061+
"""
1062+
Set.empty
1063+
sourceFile
1064+
"App.fs"
1065+
"""
1066+
module X
1067+
let v = global.Lib.File1.discState.Second
1068+
let v2 = global.Lib.File2.discState.Rep
1069+
"""
1070+
(set [| 0 |])
1071+
]
1072+
scenario
1073+
"Library with using global namespace as module alias"
1074+
[
1075+
sourceFile
1076+
"Library.fs"
1077+
"""
1078+
namespace Z
1079+
1080+
module N =
1081+
let mutable discState = System.DateTime.Now
1082+
"""
1083+
Set.empty
1084+
sourceFile
1085+
"App.fs"
1086+
"""
1087+
module X
1088+
1089+
module Y = global.Z.N
1090+
"""
1091+
(set [| 0 |])
1092+
]
10451093
]

0 commit comments

Comments
 (0)