From 36cc692d1310cb29bc19e6e1aa5844617d6a05ab Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 18 Jan 2024 17:32:32 +0100 Subject: [PATCH 1/4] Add scenarios where parentheses are around module name. --- .../TypeChecks/Graph/Scenarios.fs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs index c75aed594c3..6063366d4f7 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -800,4 +800,34 @@ printfn "Hello" """ Set.empty ] + scenario + "parentheses around module name in nameof pattern" + [ + sourceFile "A.fs" "module Foo" Set.empty + sourceFile + "B.fs" + """ +module Bar + +do + match "" with + | nameof ((Foo)) -> () + | _ -> () +""" + (set [| 0 |]) + ] + + scenario + "parentheses around module name in nameof expression" + [ + sourceFile "A.fs" "module Foo" Set.empty + sourceFile + "B.fs" + """ +module Bar + +let _ = nameof ((Foo)) +""" + (set [| 0 |]) + ] ] \ No newline at end of file From 160f637bb45db34ccafcbc0a8385107b15756fce Mon Sep 17 00:00:00 2001 From: nojaf Date: Thu, 18 Jan 2024 17:50:57 +0100 Subject: [PATCH 2/4] Address problem tighter to nameof usage. --- .../GraphChecking/FileContentMapping.fs | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs index 526a22ef099..46d6bcd4968 100644 --- a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs +++ b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs @@ -302,9 +302,27 @@ let visitSynTypeConstraint (tc: SynTypeConstraint) : FileContentEntry list = | SynTypeConstraint.WhereTyparIsEnum(typeArgs = typeArgs) -> List.collect visitSynType typeArgs | SynTypeConstraint.WhereTyparIsDelegate(typeArgs = typeArgs) -> List.collect visitSynType typeArgs +/// Special case of `nameof Module` type of expression +let (|NameofExpr|_|) (e: SynExpr) = + let rec stripParen (e: SynExpr) = + match e with + | SynExpr.Paren(expr = expr) -> stripParen expr + | _ -> e + + match e with + | SynExpr.App(flag = ExprAtomicFlag.NonAtomic; isInfix = false; funcExpr = SynExpr.Ident nameofIdent; argExpr = moduleNameExpr) -> + if nameofIdent.idText <> "nameof" then + None + else + match stripParen moduleNameExpr with + | SynExpr.Ident moduleNameIdent -> Some moduleNameIdent + | _ -> None + | _ -> None + let visitSynExpr (e: SynExpr) : FileContentEntry list = let rec visit (e: SynExpr) (continuation: FileContentEntry list -> FileContentEntry list) : FileContentEntry list = match e with + | NameofExpr moduleNameIdent -> continuation [ visitIdentAsPotentialModuleName moduleNameIdent ] | SynExpr.Const _ -> continuation [] | SynExpr.Paren(expr = expr) -> visit expr continuation | SynExpr.Quote(operator = operator; quotedExpr = quotedExpr) -> @@ -389,7 +407,7 @@ let visitSynExpr (e: SynExpr) : FileContentEntry list = | SynExpr.IfThenElse(ifExpr = ifExpr; thenExpr = thenExpr; elseExpr = elseExpr) -> let continuations = List.map visit (ifExpr :: thenExpr :: Option.toList elseExpr) Continuation.concatenate continuations continuation - | SynExpr.Typar _ -> continuation [] + | SynExpr.Typar _ | SynExpr.Ident _ -> continuation [] | SynExpr.LongIdent(longDotId = longDotId) -> continuation (visitSynLongIdent longDotId) | SynExpr.LongIdentSet(longDotId, expr, _) -> visit expr (fun nodes -> visitSynLongIdent longDotId @ nodes |> continuation) @@ -517,9 +535,32 @@ let visitSynExpr (e: SynExpr) : FileContentEntry list = visit e id +/// Special case of `| nameof Module ->` type of pattern +let (|NameofPat|_|) (pat: SynPat) = + let rec stripPats p = + match p with + | SynPat.Paren(pat = pat) -> stripPats pat + | _ -> p + + match pat with + | SynPat.LongIdent(longDotId = SynLongIdent(id = [ nameofIdent ]); typarDecls = None; argPats = SynArgPats.Pats [ moduleNamePat ]) -> + if nameofIdent.idText <> "nameof" then + None + else + match stripPats moduleNamePat with + | SynPat.LongIdent( + longDotId = SynLongIdent.SynLongIdent(id = [ moduleNameIdent ]; dotRanges = []; trivia = [ None ]) + extraId = None + typarDecls = None + argPats = SynArgPats.Pats [] + accessibility = None) -> Some moduleNameIdent + | _ -> None + | _ -> None + let visitPat (p: SynPat) : FileContentEntry list = let rec visit (p: SynPat) (continuation: FileContentEntry list -> FileContentEntry list) : FileContentEntry list = match p with + | NameofPat moduleNameIdent -> continuation [ visitIdentAsPotentialModuleName moduleNameIdent ] | SynPat.Paren(pat = pat) -> visit pat continuation | SynPat.Typed(pat = pat; targetType = t) -> visit pat (fun nodes -> nodes @ visitSynType t) | SynPat.Const _ -> continuation [] From 21e526d1fc1eb9e986cd95e9275b6e185b8ee30e Mon Sep 17 00:00:00 2001 From: nojaf Date: Fri, 19 Jan 2024 13:11:15 +0100 Subject: [PATCH 3/4] Restore missing commit and inline nameof ident check. --- .../GraphChecking/DependencyResolution.fs | 15 +++- .../GraphChecking/FileContentMapping.fs | 41 +++++----- src/Compiler/Driver/GraphChecking/Types.fs | 3 + src/Compiler/Driver/GraphChecking/Types.fsi | 3 + .../TypeChecks/Graph/Scenarios.fs | 80 +++++++++++++++++++ 5 files changed, 122 insertions(+), 20 deletions(-) diff --git a/src/Compiler/Driver/GraphChecking/DependencyResolution.fs b/src/Compiler/Driver/GraphChecking/DependencyResolution.fs index 9300385b483..11ba984ca51 100644 --- a/src/Compiler/Driver/GraphChecking/DependencyResolution.fs +++ b/src/Compiler/Driver/GraphChecking/DependencyResolution.fs @@ -1,7 +1,6 @@ module internal FSharp.Compiler.GraphChecking.DependencyResolution open FSharp.Compiler.Syntax -open Internal.Utilities.Library /// Find a path from a starting TrieNode and return the end node or None let queryTriePartial (trie: TrieNode) (path: LongIdentifier) : TrieNode option = @@ -118,6 +117,20 @@ let rec processStateEntry (trie: TrieNode) (state: FileContentQueryState) (entry FoundDependencies = foundDependencies } + | ModuleName name -> + // We need to check if the module name is a hit in the Trie. + let state' = + let queryResult = queryTrie trie [ name ] + processIdentifier queryResult state + + match state.OwnNamespace with + | None -> state' + | Some ns -> + // If there we currently have our own namespace, + // the combination of that namespace + module name should be checked as well. + let queryResult = queryTrieDual trie ns [ name ] + processIdentifier queryResult state' + /// /// For a given file's content, collect all missing ("ghost") file dependencies that the core resolution algorithm didn't return, /// but are required to satisfy the type-checker. diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs index 46d6bcd4968..549822604ec 100644 --- a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs +++ b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs @@ -18,6 +18,11 @@ let longIdentToPath (skipLast: bool) (longId: LongIdent) : LongIdentifier = let synLongIdentToPath (skipLast: bool) (synLongIdent: SynLongIdent) = longIdentToPath skipLast synLongIdent.LongIdent +/// In some rare cases we are interested in the name of a single Ident. +/// For example `nameof ModuleName` in expressions or patterns. +let visitIdentAsPotentialModuleName (moduleNameIdent: Ident) = + FileContentEntry.ModuleName moduleNameIdent.idText + let visitSynLongIdent (lid: SynLongIdent) : FileContentEntry list = visitLongIdent lid.LongIdent let visitLongIdent (lid: LongIdent) = @@ -302,6 +307,10 @@ let visitSynTypeConstraint (tc: SynTypeConstraint) : FileContentEntry list = | SynTypeConstraint.WhereTyparIsEnum(typeArgs = typeArgs) -> List.collect visitSynType typeArgs | SynTypeConstraint.WhereTyparIsDelegate(typeArgs = typeArgs) -> List.collect visitSynType typeArgs +[] +let inline (|NameofIdent|_|) (ident: Ident) = + if ident.idText = "nameof" then ValueSome() else ValueNone + /// Special case of `nameof Module` type of expression let (|NameofExpr|_|) (e: SynExpr) = let rec stripParen (e: SynExpr) = @@ -310,13 +319,10 @@ let (|NameofExpr|_|) (e: SynExpr) = | _ -> e match e with - | SynExpr.App(flag = ExprAtomicFlag.NonAtomic; isInfix = false; funcExpr = SynExpr.Ident nameofIdent; argExpr = moduleNameExpr) -> - if nameofIdent.idText <> "nameof" then - None - else - match stripParen moduleNameExpr with - | SynExpr.Ident moduleNameIdent -> Some moduleNameIdent - | _ -> None + | SynExpr.App(flag = ExprAtomicFlag.NonAtomic; isInfix = false; funcExpr = SynExpr.Ident NameofIdent; argExpr = moduleNameExpr) -> + match stripParen moduleNameExpr with + | SynExpr.Ident moduleNameIdent -> Some moduleNameIdent + | _ -> None | _ -> None let visitSynExpr (e: SynExpr) : FileContentEntry list = @@ -543,18 +549,15 @@ let (|NameofPat|_|) (pat: SynPat) = | _ -> p match pat with - | SynPat.LongIdent(longDotId = SynLongIdent(id = [ nameofIdent ]); typarDecls = None; argPats = SynArgPats.Pats [ moduleNamePat ]) -> - if nameofIdent.idText <> "nameof" then - None - else - match stripPats moduleNamePat with - | SynPat.LongIdent( - longDotId = SynLongIdent.SynLongIdent(id = [ moduleNameIdent ]; dotRanges = []; trivia = [ None ]) - extraId = None - typarDecls = None - argPats = SynArgPats.Pats [] - accessibility = None) -> Some moduleNameIdent - | _ -> None + | SynPat.LongIdent(longDotId = SynLongIdent(id = [ NameofIdent ]); typarDecls = None; argPats = SynArgPats.Pats [ moduleNamePat ]) -> + match stripPats moduleNamePat with + | SynPat.LongIdent( + longDotId = SynLongIdent.SynLongIdent(id = [ moduleNameIdent ]; dotRanges = []; trivia = [ None ]) + extraId = None + typarDecls = None + argPats = SynArgPats.Pats [] + accessibility = None) -> Some moduleNameIdent + | _ -> None | _ -> None let visitPat (p: SynPat) : FileContentEntry list = diff --git a/src/Compiler/Driver/GraphChecking/Types.fs b/src/Compiler/Driver/GraphChecking/Types.fs index 00538b6e599..c667a573f69 100644 --- a/src/Compiler/Driver/GraphChecking/Types.fs +++ b/src/Compiler/Driver/GraphChecking/Types.fs @@ -73,6 +73,9 @@ type internal FileContentEntry = /// Being explicit about nested modules allows for easier reasoning what namespaces (paths) are open. /// We can scope an `OpenStatement` to the everything that is happening inside the nested module. | NestedModule of name: string * nestedContent: FileContentEntry list + /// A single identifier that could be the name of a module. + /// Example use-case: `let x = nameof Foo` where `Foo` is a module. + | ModuleName of name: Identifier type internal FileContent = { diff --git a/src/Compiler/Driver/GraphChecking/Types.fsi b/src/Compiler/Driver/GraphChecking/Types.fsi index 468ef65889c..096719b6be7 100644 --- a/src/Compiler/Driver/GraphChecking/Types.fsi +++ b/src/Compiler/Driver/GraphChecking/Types.fsi @@ -67,6 +67,9 @@ type internal FileContentEntry = /// Being explicit about nested modules allows for easier reasoning what namespaces (paths) are open. /// For example we can limit the scope of an `OpenStatement` to symbols defined inside the nested module. | NestedModule of name: string * nestedContent: FileContentEntry list + /// A single identifier that could be the name of a module. + /// Example use-case: `let x = nameof Foo` where `Foo` is a module. + | ModuleName of name: Identifier /// File identifiers and its content extract for dependency resolution type internal FileContent = diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs index 6063366d4f7..80f7caecafb 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -800,6 +800,86 @@ printfn "Hello" """ Set.empty ] + scenario + "Nameof module with namespace" + [ + sourceFile + "A.fs" + """ +namespace X.Y.Z + +module Foo = + let x = 2 +""" + Set.empty + sourceFile + "B.fs" + """ +namespace X.Y.Z + +module Point = + let y = nameof Foo +""" + (set [| 0 |]) + ] + scenario + "Nameof module without namespace" + [ + sourceFile + "A.fs" + """ +module Foo + +let x = 2 +""" + Set.empty + sourceFile + "B.fs" + """ +module Point + +let y = nameof Foo +""" + (set [| 0 |]) + ] + scenario + "Single module name should always be checked, regardless of own namespace" + [ + sourceFile "X.fs" "namespace X.Y" Set.empty + sourceFile + "A.fs" + """ +module Foo + +let x = 2 +""" + Set.empty + sourceFile + "B.fs" + """ +namespace X.Y + +type T() = + let _ = nameof Foo +""" + (set [| 1 |]) + ] + scenario + "nameof pattern" + [ + sourceFile "A.fs" "module Foo" Set.empty + sourceFile + "B.fs" + """ +module Bar + +do + match "" with + | nameof Foo -> () + | _ -> () +""" + (set [| 0 |]) + ] scenario "parentheses around module name in nameof pattern" [ From ce96db19943552dfd85430f01d2ea4b0e089a104 Mon Sep 17 00:00:00 2001 From: nojaf Date: Mon, 22 Jan 2024 11:44:38 +0100 Subject: [PATCH 4/4] Add release note entry. --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md index 54f99f02acc..6544ab4685f 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -1,6 +1,7 @@ ### Fixed * Code generated files with > 64K methods and generated symbols crash when loaded. Use infered sequence points for debugging. ([Issue #16399](https://github.com/dotnet/fsharp/issues/16399), [#PR 16514](https://github.com/dotnet/fsharp/pull/16514)) +* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550)) ### Added