From 87a3c5a5751a8cea6790576e81ad5fbe6cf239d7 Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 21 Feb 2024 15:18:22 +0100 Subject: [PATCH 1/5] Consider prefixed module name in nameof expression. --- .../GraphChecking/FileContentMapping.fs | 35 +++++++++++++++++-- .../TypeChecks/Graph/Scenarios.fs | 18 ++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs index 15355de5a30..6d74a66d2dc 100644 --- a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs +++ b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs @@ -310,8 +310,27 @@ let visitSynTypeConstraint (tc: SynTypeConstraint) : FileContentEntry list = let inline (|NameofIdent|_|) (ident: Ident) = if ident.idText = "nameof" then ValueSome() else ValueNone +/// nameof X.Y.Z can be used in expressions and patterns +[] +type NameofResult = + /// Example: nameof X + /// Where X is a module name + | SingleIdent of potentialModuleName: Ident + /// Example: nameof X.Y.Z + /// Where Z is either a module name or something from inside module or namespace Y. + /// Both options need to be explored. + | LongIdent of longIdent: LongIdent + +let visitNameofResult (nameofResult: NameofResult) : FileContentEntry = + match nameofResult with + | NameofResult.SingleIdent moduleName -> visitIdentAsPotentialModuleName moduleName + | NameofResult.LongIdent longIdent -> + // In this case the last part of the part could be a module name. + // So we should not cut off the last part. + FileContentEntry.PrefixedIdentifier(longIdentToPath false longIdent) + /// Special case of `nameof Module` type of expression -let (|NameofExpr|_|) (e: SynExpr) = +let (|NameofExpr|_|) (e: SynExpr) : NameofResult option = let rec stripParen (e: SynExpr) = match e with | SynExpr.Paren(expr = expr) -> stripParen expr @@ -320,14 +339,19 @@ let (|NameofExpr|_|) (e: SynExpr) = 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 + | SynExpr.Ident moduleNameIdent -> Some(NameofResult.SingleIdent moduleNameIdent) + | SynExpr.LongIdent(longDotId = longIdent) -> + if longIdent.LongIdent.IsEmpty then + None + else + Some(NameofResult.LongIdent(longIdent.LongIdent)) | _ -> 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 ] + | NameofExpr nameofResult -> continuation [ visitNameofResult nameofResult ] | SynExpr.Const _ -> continuation [] | SynExpr.Paren(expr = expr) -> visit expr continuation | SynExpr.Quote(operator = operator; quotedExpr = quotedExpr) -> @@ -645,6 +669,11 @@ let visitSynMatchClause (SynMatchClause(pat = pat; whenExpr = whenExpr; resultEx ] let visitBinding (SynBinding(attributes = attributes; headPat = headPat; returnInfo = returnInfo; expr = expr)) : FileContentEntry list = + let foo = visitSynAttributes attributes + + if not foo.IsEmpty then + () + [ yield! visitSynAttributes attributes match headPat with diff --git a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs index 80f7caecafb..4aa93cec499 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -796,6 +796,8 @@ type Foo = class end sourceFile "Program" """ +module Program + printfn "Hello" """ Set.empty @@ -907,6 +909,22 @@ do module Bar let _ = nameof ((Foo)) +""" + (set [| 0 |]) + ] + scenario + "prefixed module name in nameof expression" + [ + sourceFile "A.fs" "module X.Y.Z" Set.empty + sourceFile + "B.fs" + """ +module B + +open System.ComponentModel + +[] +let v = 2 """ (set [| 0 |]) ] From 9870c4ddbcb897ec5d4118e9ae3f82ae2f2191fa Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 21 Feb 2024 15:36:18 +0100 Subject: [PATCH 2/5] Consider prefixed module name in nameof pattern. --- .../GraphChecking/FileContentMapping.fs | 19 ++++++++++++------- .../TypeChecks/Graph/Scenarios.fs | 13 +++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs index 6d74a66d2dc..3faf98d4f8f 100644 --- a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs +++ b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs @@ -341,10 +341,11 @@ let (|NameofExpr|_|) (e: SynExpr) : NameofResult option = match stripParen moduleNameExpr with | SynExpr.Ident moduleNameIdent -> Some(NameofResult.SingleIdent moduleNameIdent) | SynExpr.LongIdent(longDotId = longIdent) -> - if longIdent.LongIdent.IsEmpty then - None - else - Some(NameofResult.LongIdent(longIdent.LongIdent)) + match longIdent.LongIdent with + | [] -> None + // This is highly unlikely to be produced by the parser + | [ moduleNameIdent ] -> Some(NameofResult.SingleIdent moduleNameIdent) + | lid -> Some(NameofResult.LongIdent(lid)) | _ -> None | _ -> None @@ -575,18 +576,22 @@ let (|NameofPat|_|) (pat: SynPat) = | 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 ]) + longDotId = SynLongIdent.SynLongIdent(id = longIdent) extraId = None typarDecls = None argPats = SynArgPats.Pats [] - accessibility = None) -> Some moduleNameIdent + accessibility = None) -> + match longIdent with + | [] -> None + | [ moduleNameIdent ] -> Some(NameofResult.SingleIdent moduleNameIdent) + | lid -> Some(NameofResult.LongIdent lid) | _ -> 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 ] + | NameofPat moduleNameIdent -> continuation [ visitNameofResult 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/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs index 4aa93cec499..524556dae0a 100644 --- a/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs +++ b/tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs @@ -925,6 +925,19 @@ open System.ComponentModel [] let v = 2 +""" + (set [| 0 |]) + ] + scenario + "prefixed module name in nameof pattern" + [ + sourceFile "A.fs" "module X.Y.Z" Set.empty + sourceFile + "B.fs" + """ +module B + +do ignore (match "" with | nameof X.Y.Z -> () | _ -> ()) """ (set [| 0 |]) ] From 007ba8a32f108c213866d907ecf14d2e1e2df91b Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 21 Feb 2024 15:37:53 +0100 Subject: [PATCH 3/5] Remove foo --- src/Compiler/Driver/GraphChecking/FileContentMapping.fs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs index 3faf98d4f8f..fe36168143a 100644 --- a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs +++ b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs @@ -674,11 +674,6 @@ let visitSynMatchClause (SynMatchClause(pat = pat; whenExpr = whenExpr; resultEx ] let visitBinding (SynBinding(attributes = attributes; headPat = headPat; returnInfo = returnInfo; expr = expr)) : FileContentEntry list = - let foo = visitSynAttributes attributes - - if not foo.IsEmpty then - () - [ yield! visitSynAttributes attributes match headPat with From 327b20fb2116267766eb72754c5aaf8edd49456d Mon Sep 17 00:00:00 2001 From: nojaf Date: Wed, 21 Feb 2024 15:39:55 +0100 Subject: [PATCH 4/5] Update release notes --- docs/release-notes/.FSharp.Compiler.Service/8.0.300.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 672209d2ed1..af927a7ceee 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/8.0.300.md @@ -2,7 +2,7 @@ * Fix missing warning for recursive calls in list comprehensions. ([PR #16652](https://github.com/dotnet/fsharp/pull/16652)) * 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)) +* `nameof Module` expressions and patterns are processed to link files in `--test:GraphBasedChecking`. ([PR #16550](https://github.com/dotnet/fsharp/pull/16550), [PR #16743](https://github.com/dotnet/fsharp/pull/16743)) * Graph Based Checking doesn't throw on invalid parsed input so it can be used for IDE scenarios ([PR #16575](https://github.com/dotnet/fsharp/pull/16575), [PR #16588](https://github.com/dotnet/fsharp/pull/16588), [PR #16643](https://github.com/dotnet/fsharp/pull/16643)) * Various parenthesization API fixes. ([PR #16578](https://github.com/dotnet/fsharp/pull/16578), [PR #16666](https://github.com/dotnet/fsharp/pull/16666)) * Keep parens for problematic exprs (`if`, `match`, etc.) in `$"{(…):N0}"`, `$"{(…),-3}"`, etc. ([PR #16578](https://github.com/dotnet/fsharp/pull/16578)) From 942b776993811ea4271d317dea010bb188d14277 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Wed, 21 Feb 2024 17:04:49 +0100 Subject: [PATCH 5/5] Update src/Compiler/Driver/GraphChecking/FileContentMapping.fs Co-authored-by: dawe --- src/Compiler/Driver/GraphChecking/FileContentMapping.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs index fe36168143a..c19edc6ac57 100644 --- a/src/Compiler/Driver/GraphChecking/FileContentMapping.fs +++ b/src/Compiler/Driver/GraphChecking/FileContentMapping.fs @@ -325,7 +325,7 @@ let visitNameofResult (nameofResult: NameofResult) : FileContentEntry = match nameofResult with | NameofResult.SingleIdent moduleName -> visitIdentAsPotentialModuleName moduleName | NameofResult.LongIdent longIdent -> - // In this case the last part of the part could be a module name. + // In this case the last part of the LongIdent could be a module name. // So we should not cut off the last part. FileContentEntry.PrefixedIdentifier(longIdentToPath false longIdent)