Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update FSharp.Analyzers.SDK to 0.13.0 #1163

Closed
wants to merge 1 commit into from

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Sep 8, 2023

WHAT

🤖 Generated by Copilot at 9bd005f

Improved the loading and handling of analyzers in the F# LSP servers. Refactored the analyzerHandler function to use a more suitable type.

🤖 Generated by Copilot at 9bd005f

To load analyzers with more grace
We pass a logging function in place
It reports any errors
That might cause us terrors
When we use the SDK.Client interface

📝🛠️🚨

WHY

HOW

🤖 Generated by Copilot at 9bd005f

  • Simplify and improve the analyzerHandler function by using the ParseAndCheckResults type instead of passing individual fields (link, link, link, link)

There is no way the analyzers loaded with the current FSharp.Analyzers.SDK could ever work at runtime. The mismatch in FCS version will most likely blow up for everyone.
I'm trying to address this in this PR.

There is one catch though: We currently don't have the full project-type check information.
This perhaps is a bit of a too-strict requirement to load analyzers. It makes sense from the CLI point of view but maybe not for the editor.
Let me know what makes the most sense and we can act accordingly.

@TheAngryByrd
Copy link
Member

There is one catch though: We currently don't have the full project-type check information.
This perhaps is a bit of a too-strict requirement to load analyzers. It makes sense from the CLI point of view but maybe not for the editor.
Let me know what makes the most sense and we can act accordingly.

Yeah I started this discussion over here: ionide/FSharp.Analyzers.SDK#73 (comment)

I'm probably not gonna accept any updates to the analyzer SDK while we're still making a lot of breaking changes over there.

@nojaf
Copy link
Contributor Author

nojaf commented Jan 14, 2024

Closing in favour of #1217

@nojaf nojaf closed this Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants