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.

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

Attempt to make links from single identifier module names. #16550

Merged
merged 4 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
15 changes: 14 additions & 1 deletion src/Compiler/Driver/GraphChecking/DependencyResolution.fs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module internal FSharp.Compiler.GraphChecking.DependencyResolution

open FSharp.Compiler.Syntax
open Internal.Utilities.Library

/// <summary>Find a path from a starting TrieNode and return the end node or None</summary>
let queryTriePartial (trie: TrieNode) (path: LongIdentifier) : TrieNode option =
Expand Down Expand Up @@ -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'

/// <summary>
/// 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.
Expand Down
46 changes: 45 additions & 1 deletion src/Compiler/Driver/GraphChecking/FileContentMapping.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand Down Expand Up @@ -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

[<return: Struct>]
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) ->
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 []
Expand Down
3 changes: 3 additions & 0 deletions src/Compiler/Driver/GraphChecking/Types.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
{
Expand Down
3 changes: 3 additions & 0 deletions src/Compiler/Driver/GraphChecking/Types.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
110 changes: 110 additions & 0 deletions tests/FSharp.Compiler.ComponentTests/TypeChecks/Graph/Scenarios.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 |])
]
]
Loading