Skip to content

Commit

Permalink
Classify nameof<'T> & match … with nameof ident -> … correctly (#…
Browse files Browse the repository at this point in the history
…18300)

* Resolve `nameof` in `nameof<'T>`

* Resolve `nameof` in `match … with nameof ident -> …`

* Add `nameof` classification tests

* Update release notes

* Might as well make them compilable

* Show `nameof` as keyword in `nameof<…>` even when the type parameter is invalid

* Touchup
  • Loading branch information
brianrourkeboll authored Feb 12, 2025
1 parent 96771a8 commit 79e8531
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 4 deletions.
3 changes: 2 additions & 1 deletion docs/release-notes/.FSharp.Compiler.Service/9.0.300.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
### Fixed
* Fix classification of `nameof` in `nameof<'T>`, `match … with nameof ident -> …`. ([Issue #10026](https://github.com/dotnet/fsharp/issues/10026), [PR #18300](https://github.com/dotnet/fsharp/pull/18300))
* Fix Realsig+ generates nested closures with incorrect Generic ([Issue #17797](https://github.com/dotnet/fsharp/issues/17797), [PR #17877](https://github.com/dotnet/fsharp/pull/17877))
* Fix optimizer internal error for records with static fields ([Issue #18165](https://github.com/dotnet/fsharp/issues/18165), [PR #18280](https://github.com/dotnet/fsharp/pull/18280))
* Fix nullness warning with flexible types ([Issue #18056](https://github.com/dotnet/fsharp/issues/18056), [PR #18266](https://github.com/dotnet/fsharp/pull/18266))
Expand All @@ -25,4 +26,4 @@
* Added nullability annotations to `.Using` builder method for `async`, `task` and compiler-internal builders ([PR #18292](https://github.com/dotnet/fsharp/pull/18292))

### Breaking Changes
* Struct unions with overlapping fields now generate mappings needed for reading via reflection ([Issue #18121](https://github.com/dotnet/fsharp/issues/17797), [PR #18274](https://github.com/dotnet/fsharp/pull/17877))
* Struct unions with overlapping fields now generate mappings needed for reading via reflection ([Issue #18121](https://github.com/dotnet/fsharp/issues/17797), [PR #18274](https://github.com/dotnet/fsharp/pull/17877))
5 changes: 4 additions & 1 deletion src/Compiler/Checking/CheckPatterns.fs
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,10 @@ and TcPatLongIdentNewDef warnOnUpperForId warnOnUpper (cenv: cenv) env ad valRep
| [arg]
when g.langVersion.SupportsFeature LanguageFeature.NameOf && IsNameOf cenv env ad m id ->
match TcNameOfExpr cenv env tpenv (ConvSynPatToSynExpr arg) with
| Expr.Const(Const.String s, m, _) -> TcConstPat warnOnUpper cenv env vFlags patEnv ty (SynConst.String(s, SynStringKind.Regular, m)) m
| Expr.Const(Const.String s, m, _) ->
// Record the resolution of the `nameof` usage so that we can classify it correctly later.
CallNameResolutionSink cenv.tcSink (id.idRange, env.NameEnv, Item.Value g.nameof_vref, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
TcConstPat warnOnUpper cenv env vFlags patEnv ty (SynConst.String(s, SynStringKind.Regular, m)) m
| _ -> failwith "Impossible: TcNameOfExpr must return an Expr.Const of type string"

| _ ->
Expand Down
6 changes: 6 additions & 0 deletions src/Compiler/Checking/Expressions/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9265,6 +9265,12 @@ and TcValueItemThen cenv overallTy env vref tpenv mItem afterResolution delayed
// Allow `nameof<'T>` for a generic parameter
match vref with
| _ when isNameOfValRef g vref && g.langVersion.SupportsFeature LanguageFeature.NameOf ->
// Record the resolution of the `nameof` usage so that we can classify it correctly later.
do
match afterResolution with
| AfterResolution.RecordResolution (_, callSink, _, _) -> callSink emptyTyparInst
| AfterResolution.DoNothing -> ()

match tys with
| [SynType.Var(SynTypar(id, _, false) as tp, _m)] ->
let _tpR, tpenv = TcTypeOrMeasureParameter None cenv env ImplicitlyBoundTyparsAllowed.NoNewTypars tpenv tp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,12 @@ type SemanticClassificationServiceTests() =
match ranges |> List.tryFind (fun item -> Range.rangeContainsPos item.Range markerPos) with
| None -> failwith "Cannot find colorization data for end of marker"
| Some item ->
FSharpClassificationTypes.getClassificationTypeName item.Type
|> Assert.shouldBeEqualWith classificationType "Classification data doesn't match for end of marker"
let actual = FSharpClassificationTypes.getClassificationTypeName item.Type

actual
|> Assert.shouldBeEqualWith
classificationType
$"Classification data doesn't match for end of marker: {classificationType} ≠ {actual} ({item.Type})"

let verifyNoClassificationDataAtEndOfMarker (fileContents: string, marker: string, classificationType: string) =
let text = SourceText.From(fileContents)
Expand Down Expand Up @@ -183,3 +187,56 @@ let g() =
"""

verifyNoClassificationDataAtEndOfMarker (sourceText, marker, classificationType)

[<Theory>]
[<InlineData("(*1*)", ClassificationTypeNames.Keyword)>]
[<InlineData("(*2*)", ClassificationTypeNames.Keyword)>]
[<InlineData("(*3*)", ClassificationTypeNames.Keyword)>]
[<InlineData("(*4*)", ClassificationTypeNames.LocalName)>]
[<InlineData("(*5*)", ClassificationTypeNames.LocalName)>]
[<InlineData("(*6*)", ClassificationTypeNames.LocalName)>]
[<InlineData("(*7*)", ClassificationTypeNames.Identifier)>]
[<InlineData("(*8*)", ClassificationTypeNames.Identifier)>]
[<InlineData("(*9*)", ClassificationTypeNames.ClassName)>]
[<InlineData("(*10*)", ClassificationTypeNames.ClassName)>]
[<InlineData("(*11*)", ClassificationTypeNames.ClassName)>]
[<InlineData("(*12*)", ClassificationTypeNames.ClassName)>]
[<InlineData("(*13*)", ClassificationTypeNames.ClassName)>]
[<InlineData("(*14*)", ClassificationTypeNames.TypeParameterName)>]
[<InlineData("(*15*)", ClassificationTypeNames.TypeParameterName)>]
[<InlineData("(*16*)", ClassificationTypeNames.Keyword)>]
[<InlineData("(*17*)", ClassificationTypeNames.Keyword)>]
[<InlineData("(*18*)", ClassificationTypeNames.Keyword)>]
member _.``nameof ident, nameof<'T>, match with nameof ident``(marker: string, classificationType: string) =
let sourceText =
"""
module ``Normal usage of nameof should show up as a keyword`` =
let f x = (*1*)nameof x
let g (x : 'T) = (*2*)nameof<'T>
let h x y = match x with (*3*)nameof y -> () | _ -> ()
module ``Redefined nameof should shadow the intrinsic one`` =
let a x = match x with (*4*)nameof -> ()
let b (*5*)nameof = (*6*)nameof
let (*7*)nameof = "redefined"
let _ = (*8*)nameof
type (*9*)nameof () = class end
let _ = (*10*)nameof ()
let _ = new (*11*)nameof ()
module (*12*)nameof =
let f x = x
let _ = (*13*)nameof.f 3
let c (x : '(*14*)nameof) = x
let d (x : (*15*)'nameof) = x
module ``It should still show up as a keyword even if the type parameter is invalid`` =
let _ = (*16*)nameof<>
let a (x : 'a) (y : 'b) = (*17*)nameof<'c> // FS0039: The type parameter 'c is not defined.
let _ = (*18*)nameof<int> // FS3250: Expression does not have a name.
"""

verifyClassificationAtEndOfMarker (sourceText, marker, classificationType)

0 comments on commit 79e8531

Please sign in to comment.