Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefixed module nameof #16743

Merged
merged 5 commits into from
Feb 22, 2024
Merged

Prefixed module nameof #16743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87a3c5a
Consider prefixed module name in nameof expression.
nojaf Feb 21, 2024
9870c4d
Consider prefixed module name in nameof pattern.
nojaf Feb 21, 2024
007ba8a
Remove foo
nojaf Feb 21, 2024
327b20f
Update release notes
nojaf Feb 21, 2024
942b776
Update src/Compiler/Driver/GraphChecking/FileContentMapping.fs
nojaf Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
41 changes: 35 additions & 6 deletions src/Compiler/Driver/GraphChecking/FileContentMapping.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
[<RequireQualifiedAccess; NoComparison>]
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 LongIdent 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
Expand All @@ -320,14 +339,20 @@ 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) ->
Comment on lines +342 to +343
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nojaf I think this may require more cases that can be parsed here and where a module name is allowed. For example, consider this:

nameof GenericType<Module.TypeInsideModule>

Maybe I'm missing something, but I thought this would be covered by a separate case in SynExpr.
If that's correct, I'd suggest going through all of its cases and checking what is allowed in nameof.

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

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) ->
Expand Down Expand Up @@ -551,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 []
Expand Down
31 changes: 31 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,8 @@ type Foo = class end
sourceFile
"Program"
"""
module Program

printfn "Hello"
"""
Set.empty
Expand Down Expand Up @@ -907,6 +909,35 @@ 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

[<Description(nameof X.Y.Z)>]
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 |])
]
Expand Down
Loading