Skip to content

Comments

[Repo Assist] fix: emit Keyword semantic token for null in nullable type annotations#1466

Open
github-actions[bot] wants to merge 4 commits intomainfrom
repo-assist/fix-issue-1381-null-keyword-semantic-token-913b37c2a2f415f2
Open

[Repo Assist] fix: emit Keyword semantic token for null in nullable type annotations#1466
github-actions[bot] wants to merge 4 commits intomainfrom
repo-assist/fix-issue-1381-null-keyword-semantic-token-913b37c2a2f415f2

Conversation

@github-actions
Copy link
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Closes #1381

Problem

The null keyword in F# 9 nullable type annotations (e.g. string | null) was not receiving a semantic token classification. This caused editors to display null without keyword highlighting in these positions, even though null elsewhere in code does receive keyword highlighting.

Root Cause

FCS.GetSemanticClassification does not emit tokens for the null keyword in SynType.WithNull positions — it handles the | null portion of the type through the type-checking phase differently, and the resulting keyword token is simply absent from the emitted classification list.

Fix

Walk the untyped parse tree (ParseTree) for SynType.WithNull nodes and emit Keyword semantic tokens for each null occurrence found. These are collected in UntypedAstUtils.NullableTypes.collectNullKeywordRanges and appended to the FCS-provided tokens inside handleSemanticTokens in AdaptiveFSharpLspServer.fs.

Implementation details:

  • Added module NullableTypes in UntypedAstUtils.fs with a SyntaxCollectorBase subclass that walks SynType.WithNull nodes and computes the null keyword range from the full type range (the null keyword is always exactly the last 4 characters of the WithNull full range).
  • Added the corresponding signature in UntypedAstUtils.fsi.
  • handleSemanticTokens calls collectNullKeywordRanges and appends (lspRange, Keyword, []) entries; if a range filter is active, only tokens within that range are included.

Trade-offs

  • The fix uses the untyped AST, so it fires on parse without type-checking. This is consistent with how other semantic token supplements work in FSAC and avoids any perf regression on the type-checking path.
  • The position of null is computed as the last 4 characters of the SynType.WithNull full range. This is reliable because F# syntax requires null to be spelled exactly (no aliases), but it does depend on the FCS SynType.WithNull full range including the null keyword and no trailing whitespace — which is the current behaviour.

Test Status

Build: dotnet build — succeeded with 0 warnings, 0 errors.

Tests: dotnet test -f net8.0 ./test/FsAutoComplete.Tests.Lsp/FsAutoComplete.Tests.Lsp.fsproj --filter Highlighting16/16 passed (including the new (28u, 26u) Keyword assertion for null in let withNull (x: string | null) = ()).

Generated by Repo Assist

To install this workflow, run gh aw add githubnext/agentics/workflows/repo-assist.md@b87234850bf9664d198f28a02df0f937d0447295. View source at https://github.com/githubnext/agentics/tree/b87234850bf9664d198f28a02df0f937d0447295/workflows/repo-assist.md.

Fixes #1381 - the 'null' keyword in F# 9 nullable type annotations
(e.g. 'string | null') was not receiving a semantic token classification,
causing editors to display it without keyword highlighting.

Root cause: FCS GetSemanticClassification does not emit tokens for the
'null' keyword in SynType.WithNull positions.

Fix: Walk the untyped parse tree for SynType.WithNull nodes and emit
Keyword tokens for each 'null' occurrence. These are appended to the
FCS-provided tokens in handleSemanticTokens.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Krzysztof-Cieslak Krzysztof-Cieslak marked this pull request as ready for review February 24, 2026 23:19
@baronfel
Copy link
Contributor

/repo-assist add a test that validates that the invariant about Null being the last 4 chars in the range is held, so that we are protected if the FCS behavior changes

…ith 'null'

Adds a new FcsInvariantTests module to CoreUtilsTests.fs that validates
the invariant relied upon by NullableTypes.collectNullKeywordRanges: that
the FCS SynType.WithNull fullRange ends exactly at the 'null' keyword
(i.e. the last 4 characters of the range cover the literal 'null').

Four test cases cover:
- single nullable parameter annotation: let f (x: string | null) = ()
- multiple nullable annotations on one line
- nullable annotation on a non-first line (multiline function signature)
- nullable return-type annotation: let f () : string | null = null

If FCS ever changes the SynType.WithNull fullRange invariant these tests
will fail loudly rather than silently emitting wrong semantic tokens.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Contributor Author

Commit pushed: 6ecf638

Generated by Repo Assist

@github-actions
Copy link
Contributor Author

🤖 This is an automated response from Repo Assist.

Done! I've added a new FcsInvariantTests module to CoreUtilsTests.fs (commit 63ac035).

What the tests do

Rather than directly subclassing SyntaxCollectorBase (the .fsi exposes it as fully abstract, preventing object expressions), the tests call NullableTypes.collectNullKeywordRanges on parsed ASTs and then extract the actual source text at each returned range, asserting it equals "null".

This is the right level to test the invariant: if FCS ever changes SynType.WithNull.fullRange (e.g. adds trailing whitespace, or stops including the null keyword at the end), the computed ranges will shift and these tests will fail loudly.

Four test cases

Test Source snippet
Single nullable parameter let f (x: string | null) = ()
Multiple nullable parameters let f (x: string | null) (y: string | null) = ()
Nullable on a non-first line let f\n (x: string | null) = ()
Nullable return-type let f () : string | null = null

All 4 pass locally:

EXPECTO! 4 tests run in 00:00:02.19 – 4 passed, 0 ignored, 0 failed, 0 errored. Success!

Generated by Repo Assist for issue #1466

To install this workflow, run gh aw add githubnext/agentics/workflows/repo-assist.md@b87234850bf9664d198f28a02df0f937d0447295. View source at https://github.com/githubnext/agentics/tree/b87234850bf9664d198f28a02df0f937d0447295/workflows/repo-assist.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support semantic token for | null

1 participant