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 to 7.0.4xx FCS versions #1151

Merged
merged 67 commits into from
Aug 21, 2023
Merged

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Aug 8, 2023

WHAT

🤖 Generated by Copilot at dbdb1a0

This pull request updates the FsAutoComplete project to use the latest version of the FSharp.Compiler.Service dependency and makes some improvements to the F# checker, lexer, and code generation features. It modifies several files in the src/FsAutoComplete.Core folder to handle the new syntax and type information, performance parameters, and language version support.

🤖 Generated by Copilot at dbdb1a0

FSharp.Compiler.Service
Updated for performance
Autumn of bug fixes

✨⚡🔧

WHY

Release day! kermitflail.gif

HOW

🤖 Generated by Copilot at dbdb1a0

  • Updated the FSharp.Compiler.Service dependency to version 43.7.400 to use the latest features and bug fixes of the F# compiler service. (link)
  • Modified the FSharpChecker.Create function call to use additional parameters for parallel reference resolution, capture identifiers when parsing, and use syntax tree cache. These parameters are intended to improve the performance and accuracy of the F# checker. (link)
  • Refactored the tokenizeLine function to use active patterns to extract the define and langversion arguments from the args array. The langversion argument was also passed to the FSharpSourceTokenizer constructor to support different F# language versions. These changes are intended to make the code more readable and flexible. (link)
  • Added or updated the range of the tuple type parameter in the SynPat.Tuple pattern match cases in InlayHints.fs, UnionPatternMatchCaseGenerator.fs, and UntypedAstUtils.fs. This parameter was added in the FSharp.Compiler.Service 43.7.400 version and is needed to correctly handle the tuple type information. (link, link, link)
  • Removed the SynSimplePats.Typed pattern match cases from the walkSimplePats function in UntypedAstUtils.fs, as they are no longer used in the FSharp.Compiler.Service 43.7.400 version. The SynSimplePats.SimplePats cases were also updated to include the range of the simple pattern list parameter. These changes are intended to simplify the code and align with the latest F# compiler service API. (link, link)

dawedawe and others added 4 commits July 1, 2023 11:47
- Update to latest FCS beta package
* Having the latest fsc, we can make use of the now public Range property of PreXmlDoc to save resources

* format
enableBackgroundItemKeyStoreAndSemanticClassification = true,
keepAllBackgroundSymbolUses = true
enablePartialTypeChecking = not hasAnalyzers,
parallelReferenceResolution = true,
Copy link
Member

Choose a reason for hiding this comment

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

We're enabling this in Ionide via a config flag using an envvar. Could pass this along instead https://github.com/ionide/ionide-vscode-fsharp/blob/main/src/Core/LanguageService.fs#L714

Copy link
Member

Choose a reason for hiding this comment

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

I guess tangentially, how much of this stuff should we expose via config?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can progressively add it where there's no clear answer so that users can unblock themselves/try things out. I'll add it here for example. Something like enablePartialTypeChecking though has rules that we can infer/follow - e.g. any analyzers auto-disable the feature.

@baronfel
Copy link
Contributor Author

baronfel commented Aug 9, 2023

This'll probably be red for a few mroe days until I finish cherry-picking fixes from nightly over.

@baronfel baronfel changed the title minimal code changes to adjust to API surface area Update to 7.0.4xx FCS versions Aug 19, 2023
@baronfel
Copy link
Contributor Author

my general plan for the failing tests is:

  • I know nightly has fixes for the failing tests
  • this branch is different from nightly, because nightly has fallen behind merges from main
  • I'm going to merge main to nightly to pick up feature fixes/contributions, and the diff/compare/merge nightly to this branch (because it should be a smaller diff at that point.

@baronfel
Copy link
Contributor Author

Ok, I think this should be just about ready to go now. Before this merges we'll need to make a new ProjInfo, and hopefully unblock the upper version bound on that project - I'd rather not merge + release this until we're off of the prerelease of ProjInfo.

@baronfel
Copy link
Contributor Author

Green! I'll work on projinfo and try to get a new stable with fixed version ranges out.

@nojaf
Copy link
Contributor

nojaf commented Aug 21, 2023

Thanks Chet!

@nojaf
Copy link
Contributor

nojaf commented Aug 21, 2023

Kinda cute that FCS is considered a patch 😸

@baronfel
Copy link
Contributor Author

Re: 7.0 floats we don't really have an option here - FCS itself requires them. I did check them and the libraries themselves despite having 7.0 version numbers target the net6.0 TFM (and our test are passing when forcing net6.0) so I'm not incredibly worried.

@baronfel
Copy link
Contributor Author

New ProjInfo packages are pushed, waiting for indexing on NuGet.org, then I'll update here and we'll see if tests are still green.

@baronfel
Copy link
Contributor Author

Ok, all green! Time to merge this buckaroo!

@baronfel baronfel merged commit 452411b into ionide:main Aug 21, 2023
9 checks passed
@baronfel baronfel deleted the update-to-stable-FCS branch August 21, 2023 16:01
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.

5 participants