Refactor #line processing - keep the original positions in the AST

[Martin521]

Description

Fixes #18553. Breaking change. Work in Progress.

This PR is non-breaking for two of the three #line use cases mentioned in #18553. It keeps the original ranges in the AST, stores the #line directives in a table and provides a method range.ApplyLineDirectives(). This method is applied in the following places

• construction of runtime diagnostics in pattern match compilation
• emitting debug information in quotation translation
• creating IL source markers (debug information)
• formatting fsc compiler diagnostics (CompilerDiagnostics.FormatDiagnosticLocation)
• creating service diagnostics (FSharpDiagnostics.CreateFromException)

This should make sure that all debugging information (in .pdb) and all diagnostics stay unchanged.

The third use case, symbol positions for IDE editors, is more tricky. FSharpSymbol has no range field or property that I could apply the transformation to, but just contains (and makes public) the AST items with their original ranges. I am considering different options here, this is still work in progress. One option is to leave calling ApplyLineDirectives to the user (e.g. FSAC). (To possibly offer both positions to the IDE using PositionLink.) I am still trying to find my way through FSAC and LSP protocol. I am assuming now that the target location (rather than the original) is needed only for the various go-to requests and some naviation items. Any thoughts or recommendations here? (@TheAngryByrd @auduchinok @abonie )

List of breaking changes

• AST has original ranges now (ignoring #line directives)
• __LINE__ and __SOURCE_FILE__ show original locations now
• Symbols t.b.d.

Checklist

• [ ] Test cases added
• [ ] Performance benchmarks added in case of performance changes
• [ ] Release notes entry updated

[github-actions[bot]]

:heavy_exclamation_mark: Release notes required

---

:white_check_mark: Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/10.0.100.md

[Martin521]

This is ready for review. See updated PR description. It introduces no changes for compiler users (except for the hardly used __LINE__ / __SOURCE_FILE__ in combination with #line). For tooling, see the remarks above.

[Martin521]

@T-Gro Is there anything I can do to support reviewing this PR?

[TheAngryByrd]

I expect this to not create any changes to the fsc output. I also assume there are no consumers of the compiler service that rely on having the target ranges in the AST. For them, this would be a breaking change. They would need to apply the map when dealing with these ranges.

Sorry for not getting around to responding sooner. We probably do with FSAC QuickFixes or possibly some analyzers but with all changes to FCS, we adapt. It might be enlightening to see what tests fail with a build of this against FSAC or FSharp.Analyzers.SDK/Ionide-Analyzers/GResearch's analyzers.

[Martin521]

Sorry for not getting around to responding sooner. We probably do with FSAC QuickFixes or possibly some analyzers but with all changes to FCS, we adapt. It might be enlightening to see what tests fail with a build of this against FSAC or FSharp.Analyzers.SDK/Ionide-Analyzers/GResearch's analyzers.

Good to hear. Thanks for looking into it. I will try to run the tests and report back.

[Martin521]

Here is what I found out.

Good news is that goto-symbol is working correctly with the FCS of this PR and the fix in FSAC's fcsRangeToLspLocation that I mentioned in the OP.

I also ran FsAutoComplete.Tests.Lsp

• In my environment (fresh dev container), this always (also with the current unchanged FSAC from the repo) ends up hanging in test FSAC.lsp.Ionide WorkspaceLoader.TransparentCompiler.empty file features.tests.type 'c' for checking error and autocompletion starts with 'abs'. So I can only report about the first 5486 tests. Running with the unchanged FSAC, they all pass.
• When building FSAC with the FCS from the current dotnet/fsharp repo's main branch and running the tests, I see random failures of one or more of the CodeFix-tests.AdjustConstant tests. I see the same when using the FCS of this PR. No extra failures from the changes of this PR. Which is somehow to be expected because there is no #line in any of the tests.

[edgarfgp]

@T-Gro @abonie Can we please get a review here. ?