From dfaed03cbab8ccc1f431c2eccc2365acfa263e81 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Mon, 22 Jan 2024 13:03:12 +0100 Subject: [PATCH 1/4] Merge --- docs/release-notes/.FSharp.Core/8.0.200.md | 1 + .../GraphChecking/DependencyResolution.fs | 15 ++- .../GraphChecking/FileContentMapping.fs | 46 +++++++- src/Compiler/Driver/GraphChecking/Types.fs | 3 + src/Compiler/Driver/GraphChecking/Types.fsi | 3 + .../TypeChecks/Graph/Scenarios.fs | 110 ++++++++++++++++++ 6 files changed, 176 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/.FSharp.Core/8.0.200.md b/docs/release-notes/.FSharp.Core/8.0.200.md index 8a1feafaf64..03bc01dc130 100644 --- a/docs/release-notes/.FSharp.Core/8.0.200.md +++ b/docs/release-notes/.FSharp.Core/8.0.200.md @@ -2,3 +2,4 @@ * More inlines for Result module. ([PR #16106](https://github.com/dotnet/fsharp/pull/16106)) * Added a new parameterless constructor for `CustomOperationAttribute` ([PR #16475](https://github.com/dotnet/fsharp/pull/16475), part of implementation for [fslang-suggestions/1250](https://github.com/fsharp/fslang-suggestions/issues/1250)) +* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550)) 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 bfdaf4beea8..f86298308a9 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,9 +307,28 @@ 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) = + 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) -> + 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 +413,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 +541,29 @@ 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 ]) -> + 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 [] 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 c75aed594c3..80f7caecafb 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -800,4 +800,114 @@ 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" + [ + 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 8c4036895b1a00e4336fb9e6019dd71bc8a4a4a2 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 22 Jan 2024 18:36:53 +0100 Subject: [PATCH 2/4] Bump version --- eng/Versions.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/Versions.props b/eng/Versions.props index 598487cde55..464fe046968 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -16,7 +16,7 @@ 8 0 - 200 + 201 0 From b2e083efaa0e29f30fe0d3961817603df2d7ed7d Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 22 Jan 2024 18:40:04 +0100 Subject: [PATCH 3/4] release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.201.md | 3 +++ docs/release-notes/.FSharp.Core/8.0.200.md | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 docs/release-notes/.FSharp.Compiler.Service/8.0.201.md diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.201.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.201.md new file mode 100644 index 00000000000..b952784419d --- /dev/null +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.201.md @@ -0,0 +1,3 @@ +### Fixed + +* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16570](https://github.com/dotnet/fsharp/pull/16570)) \ No newline at end of file diff --git a/docs/release-notes/.FSharp.Core/8.0.200.md b/docs/release-notes/.FSharp.Core/8.0.200.md index 03bc01dc130..8a1feafaf64 100644 --- a/docs/release-notes/.FSharp.Core/8.0.200.md +++ b/docs/release-notes/.FSharp.Core/8.0.200.md @@ -2,4 +2,3 @@ * More inlines for Result module. ([PR #16106](https://github.com/dotnet/fsharp/pull/16106)) * Added a new parameterless constructor for `CustomOperationAttribute` ([PR #16475](https://github.com/dotnet/fsharp/pull/16475), part of implementation for [fslang-suggestions/1250](https://github.com/fsharp/fslang-suggestions/issues/1250)) -* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550)) From 09a7f5963d90b571315fe2c97a3ebe5ee5acca42 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Thu, 22 Feb 2024 14:44:22 +0100 Subject: [PATCH 4/4] Versioning --- .../.FSharp.Compiler.Service/{8.0.201.md => 8.0.202.md} | 2 +- eng/Versions.props | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename docs/release-notes/.FSharp.Compiler.Service/{8.0.201.md => 8.0.202.md} (80%) diff --git a/docs/release-notes/.FSharp.Compiler.Service/8.0.201.md b/docs/release-notes/.FSharp.Compiler.Service/8.0.202.md similarity index 80% rename from docs/release-notes/.FSharp.Compiler.Service/8.0.201.md rename to docs/release-notes/.FSharp.Compiler.Service/8.0.202.md index b952784419d..a34e8e6442c 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.201.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.202.md @@ -1,3 +1,3 @@ ### Fixed -* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16570](https://github.com/dotnet/fsharp/pull/16570)) \ No newline at end of file +* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16570](https://github.com/dotnet/fsharp/pull/16570)) diff --git a/eng/Versions.props b/eng/Versions.props index 464fe046968..0091f154651 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -16,7 +16,7 @@ 8 0 - 201 + 202 0