Fix syntax highlighting for optional parameters with ? prefix (#19162)

Author: Copilot

docs/release-notes/.FSharp.Compiler.Service/10.0.200.md

@@ -1,6 +1,7 @@
 ### Fixed
 
 * Type relations cache: optimize key generation ([Issue #19116](https://github.com/dotnet/fsharp/issues/18767)) ([PR #19120](https://github.com/dotnet/fsharp/pull/19120))
+* Fixed QuickParse to correctly handle optional parameter syntax with `?` prefix, resolving syntax highlighting issues. ([Issue #11008753](https://developercommunity.visualstudio.com/t/F-Highlighting-fails-on-optional-parame/11008753)) ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))
 * Fix `--preferreduilang` switch leaking into `fsi.CommandLineArgs` when positioned after script file ([PR #19151](https://github.com/dotnet/fsharp/pull/19151))
 
 ### Added

src/Compiler/Service/QuickParse.fs

@@ -90,6 +90,14 @@ module QuickParse =
                     && (lineStr[index] = '|' || IsIdentifierPartCharacter lineStr[index])
                     ->
                     Some index
+                // Handle optional parameter syntax: if we're on '?' and the next char is an identifier, use the next position
+                | _ when
+                    (index < lineStr.Length)
+                    && lineStr[index] = '?'
+                    && (index + 1 < lineStr.Length)
+                    && IsIdentifierPartCharacter lineStr[index + 1]
+                    ->
+                    Some(index + 1)
                 | _ -> None // not on a word or '.'
 
             let (|Char|_|) p =

tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj

@@ -54,6 +54,7 @@
     <Compile Include="RangeTests.fs" />
     <Compile Include="TooltipTests.fs" />
     <Compile Include="TokenizerTests.fs" />
+    <Compile Include="QuickParseTests.fs" />
     <Compile Include="CompilerTestHelpers.fs" />
     <Compile Include="ManglingNameOfProvidedTypes.fs" />
     <Compile Include="HashIfExpression.fs" />

tests/FSharp.Compiler.Service.Tests/QuickParseTests.fs

@@ -0,0 +1,73 @@
+module FSharp.Compiler.Service.Tests.QuickParseTests
+
+open Xunit
+open FSharp.Compiler.EditorServices
+
+// QuickParse.GetCompleteIdentifierIsland is used by language service features
+// to extract identifier context from source text at cursor positions.
+// When it returns None (as it did for '?' before the fix), downstream services
+// like semantic classification, completion, and hover can misinterpret the context.
+// This impacts Visual Studio's syntax highlighting - see issue #11008753
+
+[<Fact>]
+let ``QuickParse handles optional parameter identifier extraction when cursor is on question mark``() =
+    let lineStr = "member _.memb(?optional:string) = optional"
+    
+    // Test when cursor is exactly on the '?' character
+    let posOnQuestionMark = 14
+    Assert.Equal('?', lineStr[posOnQuestionMark])
+    
+    let island = QuickParse.GetCompleteIdentifierIsland false lineStr posOnQuestionMark
+    
+    // We expect to get "optional" as the identifier
+    Assert.True(Option.isSome island, "Should extract identifier island when positioned on '?'")
+    
+    match island with
+    | Some(ident, startCol, isQuoted) ->
+        Assert.Equal("optional", ident)
+        Assert.False(isQuoted)
+        // The identifier should start after the '?'
+        Assert.True(startCol >= 15, sprintf "Start column %d should be >= 15" startCol)
+    | None ->
+        Assert.Fail("Expected to find identifier 'optional' when positioned on '?'")
+
+[<Fact>]
+let ``QuickParse handles optional parameter identifier extraction when cursor is on identifier``() =
+    let lineStr = "member _.memb(?optional:string) = optional"
+    
+    // Test when cursor is on the identifier "optional" after the '?'
+    let posOnOptional = 17
+    Assert.Equal('t', lineStr[posOnOptional])
+    
+    let island = QuickParse.GetCompleteIdentifierIsland false lineStr posOnOptional
+    
+    // We expect to get "optional" as the identifier
+    Assert.True(Option.isSome island, "Should extract identifier island when positioned on identifier")
+    
+    match island with
+    | Some(ident, startCol, isQuoted) ->
+        Assert.Equal("optional", ident)
+        Assert.False(isQuoted)
+    | None ->
+        Assert.Fail("Expected to find identifier 'optional'")
+
+[<Fact>]
+let ``QuickParse does not treat question mark as identifier in other contexts``() =
+    let lineStr = "let x = y ? z"
+    
+    // Test when cursor is on the '?' in a different context (not optional parameter)
+    let posOnQuestionMark = 10
+    Assert.Equal('?', lineStr[posOnQuestionMark])
+    
+    let island = QuickParse.GetCompleteIdentifierIsland false lineStr posOnQuestionMark
+    
+    // In this context, '?' is followed by space, not an identifier start
+    // So we should get None or the next identifier 'z'
+    // Let's check what we actually get
+    match island with
+    | Some(ident, _, _) ->
+        // If we get something, it should be 'z' (the next identifier after the space)
+        Assert.Equal("z", ident)
+    | None ->
+        // Or we might get None, which is also acceptable
+        ()

tests/FSharp.Compiler.Service.Tests/TokenizerTests.fs

@@ -223,3 +223,26 @@ let ``Unfinished idents``() =
          ["IDENT", "```"]]
 
     actual |> Assert.shouldBe expected
+
+[<Fact>]
+let ``Tokenizer test - optional parameters with question mark``() =
+    let tokenizedLines =
+      tokenizeLines
+        [| "member _.memb(?optional:string) = optional" |]
+
+    let actual =
+        [ for lineNo, lineToks in tokenizedLines do
+            yield lineNo, [ for str, info in lineToks do yield info.TokenName, str ] ]
+    
+    let expected =
+        [(0,
+          [("MEMBER", "member"); ("WHITESPACE", " "); ("UNDERSCORE", "_"); ("DOT", ".");
+           ("IDENT", "memb"); ("LPAREN", "("); ("QMARK", "?");
+           ("IDENT", "optional"); ("COLON", ":"); ("IDENT", "string");
+           ("RPAREN", ")"); ("WHITESPACE", " "); ("EQUALS", "="); ("WHITESPACE", " ");
+           ("IDENT", "optional")])]
+    
+    if actual <> expected then
+        printfn "actual   = %A" actual
+        printfn "expected = %A" expected
+        actual |> Assert.shouldBeEqualWith expected (sprintf "actual and expected did not match,actual =\n%A\nexpected=\n%A\n" actual expected)

vsintegration/tests/FSharp.Editor.Tests/SemanticClassificationServiceTests.fs

@@ -240,3 +240,69 @@ module ``It should still show up as a keyword even if the type parameter is inva
 """
 
         verifyClassificationAtEndOfMarker (sourceText, marker, classificationType)
+
+    [<Fact>]
+    member _.``Optional parameters should be classified correctly``() =
+        let sourceText =
+            """
+type TestType() =
+    member _.memb(?optional:string) = optional
+"""
+
+        let ranges = getRanges sourceText
+
+        // The issue was that QuickParse returning None for '?' caused misclassification
+        // This test verifies that we get semantic classification data and nothing is
+        // incorrectly classified as a type or namespace due to the ? prefix
+
+        // Look for any identifier "optional" in the classifications
+        let text = SourceText.From(sourceText)
+
+        let optionalRanges =
+            ranges
+            |> List.filter (fun item ->
+                try
+                    // Get the actual text from the source using SourceText
+                    let span = RoslynHelpers.TryFSharpRangeToTextSpan(text, item.Range)
+
+                    match span with
+                    | ValueSome textSpan ->
+                        let actualText = text.GetSubText(textSpan).ToString()
+                        actualText = "optional"
+                    | ValueNone -> false
+                with _ ->
+                    false)
+
+        // Provide detailed diagnostics if test fails
+        let allClassifications =
+            ranges
+            |> List.map (fun item ->
+                try
+                    let span = RoslynHelpers.TryFSharpRangeToTextSpan(text, item.Range)
+
+                    let textStr =
+                        match span with
+                        | ValueSome ts -> text.GetSubText(ts).ToString()
+                        | ValueNone -> "[no span]"
+
+                    sprintf "Range %A: '%s' (%A)" item.Range textStr item.Type
+                with ex ->
+                    sprintf "Range %A: [error: %s] (%A)" item.Range ex.Message item.Type)
+            |> String.concat "\n"
+
+        let errorMessage =
+            sprintf
+                "Should have classification data for 'optional' identifier.\nFound %d ranges total.\nAll classifications:\n%s"
+                ranges.Length
+                allClassifications
+
+        Assert.True(optionalRanges.Length > 0, errorMessage)
+
+        // Verify that none of the "optional" occurrences are classified as type/namespace
+        // (which would indicate the bug is present)
+        for optionalRange in optionalRanges do
+            let classificationType =
+                FSharpClassificationTypes.getClassificationTypeName optionalRange.Type
+
+            Assert.NotEqual<string>(ClassificationTypeNames.ClassName, classificationType)
+            Assert.NotEqual<string>(ClassificationTypeNames.NamespaceName, classificationType)