Skip to content

Add symbol and type highlighting to F# diagnostics - #20097

Open
auduchinok wants to merge 7 commits into
dotnet:mainfrom
auduchinok:richDiagnostics
Open

Add symbol and type highlighting to F# diagnostics#20097
auduchinok wants to merge 7 commits into
dotnet:mainfrom
auduchinok:richDiagnostics

Conversation

@auduchinok

Copy link
Copy Markdown
Member

Implements highlighting for symbols and types in diagnostic messages to be used by IDEs and tools like F# Interactive.

To make it possible, this PR introduces RichText type which wraps TaggedText[] uses and making the code simpler to work with in existing FCS APIs like tooltips.

The FSComp helpers generation is changed in a way that the same format works without changes, and only an additional RichText flag is needed in the fsproj.

@auduchinok
auduchinok requested a review from a team as a code owner July 29, 2026 16:39
@auduchinok auduchinok changed the title Diagnostics: classify errors using RichText Add symbol and type highlighting to F# diagnostics Jul 29, 2026
@github-actions github-actions Bot added ⚠️ Affects-Bootstrap Tooling check: PR touches compiler bootstrap chain ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure labels Jul 29, 2026
@github-actions

This comment has been minimized.

@github-actions github-actions Bot added the ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen label Jul 29, 2026
@github-actions

This comment has been minimized.

@auduchinok

Copy link
Copy Markdown
Member Author

This is ready.

<Link>FSStrings.resx</Link>
<LogicalName>FSStrings.resources</LogicalName>
</EmbeddedResource>
<Compile Include="Utilities\NullHelpers.fs" CompileOrder="CompileFirst" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is the CompileOrder necessary?

FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkAlias(System.String)
FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkClass(System.String)
FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkDelegate(System.String)
FSharp.Compiler.Text.RichTextModule: FSharp.Compiler.Text.RichText mkEnum(System.String)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are all the maker functions public by intention? Who is the target user from different assemblies for making diagnostics?

@@ -11140,6 +11198,7 @@ FSharp.Compiler.Text.TextTag+Tags: Int32 TypeParameter
FSharp.Compiler.Text.TextTag+Tags: Int32 Union
FSharp.Compiler.Text.TextTag+Tags: Int32 UnionCase
FSharp.Compiler.Text.TextTag+Tags: Int32 UnknownEntity

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the distinction between UnknownEntity and UnresolvedName ?

[<Fact>]
let ``Can get help for FSComp.SR.considerUpcast`` () =
match FSharp.Compiler.Interactive.FsiHelp.Logic.Quoted.tryGetHelp <@ FSComp.SR.considerUpcast @> with
match FSharp.Compiler.Interactive.FsiHelp.Logic.Quoted.tryGetHelp <@ (FSComp.SR.considerUpcast: string * string -> int * string) @> with

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we have to type annotate now?
Is this change source-breaking for some code relying on type inference?

<PropertyGroup Condition="'$(MSBuildProjectName)' == 'FSharp.Compiler.Service' AND ('$(Configuration)' == 'Proto' OR !Exists('$(ProtoOutputPath)'))">
<LocalFSharpBuildAssemblyFile>$(ArtifactsDir)bin\FSharp.Build\$(Configuration)\netstandard2.0\FSharp.Build.dll</LocalFSharpBuildAssemblyFile>
</PropertyGroup>
<UsingTask TaskName="FSharpEmbedResourceText" AssemblyFile="$(LocalFSharpBuildAssemblyFile)" Override="true" TaskFactory="TaskHostFactory" Condition="'$(LocalFSharpBuildAssemblyFile)' != ''" />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please consolidate with the UsingTask and Override above this added block.

We only need one such block.

To better stand the test of time, the MSBuildProjectName condition could instead be moved to a dedicated property ("UseBoostrappedFsharpBuildTasks") declared at project level.

Meaning the block in this file would keep on living, but the property at individual .fsproj project(s) could be added/removed as the needs evolve and also as SDK tasks become sufficient.

enabledByLangFeature: bool

/// Creates a diagnostic exception whose text comes via SR.*
val Error: (int * string) * range -> exn

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would make sense marking this one Obsolete?
Or maybe a different name indicating a "tagless" string?

Some heuristic - if a diagnostic message has format holes, it should almost certainly be routed trough a RichError, not via classical Error? (or did I get it wrong?)

| [||] -> ""
| [| part |] -> part.Text
| parts ->
let buf = StringBuilder()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we allocate with a calculated/estimated upper bound size?

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Bootstrap, Affects-Build-Infra
Affects-Build-Infra: modifies FSharpBuild.Directory.Build.targets (adds UsingTask)
Affects-Bootstrap: modifies proto.proj

Generated by PR Tooling Safety Check · opus46 8.2M ·

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

Labels

⚠️ Affects-Bootstrap Tooling check: PR touches compiler bootstrap chain ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants