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 an FCS that understands F# 7 #1043

Merged
merged 18 commits into from
Feb 20, 2023
Merged

Conversation

baronfel
Copy link
Contributor

@baronfel baronfel commented Nov 21, 2022

Notable Changes:

  • Our workarounds module could be deleted
  • the type suffix for arrays was changed from [] to array
  • SRTP type annotations were changed from ^ to '
  • Several codefixes (and tests) needed to be (temporarily) removed due to upstream issues that have been logged.

Test failure categories:

  • We also seem to have an issue in completions where the full set of known names is being reported, and the namespace-probing isn't happening (e.g. so Regex isn't recognized and System.Text isn't offered as a suggested open). - punting to followup.
  • ! codefixes aren't returning (same with many others)
  • interpolation codefixes aren't finding an AST - anonymous modules aren't returning a good parse tree? - issue raised and resolved upstream already
  • Implement interface codefix tests are broken because of upstream issue (logged already), but the functionality mostly works - issue raised

@baronfel baronfel force-pushed the fcs-.net7 branch 2 times, most recently from 3d6d767 to 06f3b15 Compare November 21, 2022 15:13
.vscode/launch.json Show resolved Hide resolved
"rollForward": "latestPatch"
}
}
"sdk": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: add this back when we get a 6.0-compatible package.

src/FsAutoComplete.Core/CodeGeneration.fs Show resolved Hide resolved
src/FsAutoComplete.Core/Commands.fs Show resolved Hide resolved
src/FsAutoComplete.Core/UntypedAstUtils.fs Outdated Show resolved Hide resolved
@@ -497,10 +497,11 @@ let autoOpenTests state =
| Ok None -> failtest "Request none"
| Ok (Some res) ->
Expect.isFalse res.IsIncomplete "Result is incomplete"
let ci = res.Items |> Array.find (fun c -> c.Label = word)
let ci = res.Items |> Array.tryFind (fun c -> c.Label = word)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

debugging here to try and figure out why we can't get all the completions.

test/FsAutoComplete.Tests.Lsp/CoreTests.fs Show resolved Hide resolved
@edgarfgp
Copy link
Contributor

@baronfel FSharp.Core 7 will now print array instead of []. Does FsAutoComplete some specific logic that needs to be removed?

@baronfel
Copy link
Contributor Author

The current blocked test is FSAC.lsp.Ionide WorkspaceLoader.FSharpLspServer.Rename Tests.Across projects.Rename from usage across projects - it's failing to load TestCases\\RenameTest\\CrossProject\\LibB\\LibB.fsproj successfully.

@baronfel
Copy link
Contributor Author

@edgarfgp that change has already been taken into account in this PR - the default formatting changed and so I updated the tests' expectations to match 👍

@baronfel
Copy link
Contributor Author

Status:

Failed:    2
Errored:  89

The failures and errors are mostly in the autocomplete namespace suggestions tests, so once that's fixed it should just be a bit of whack-a-mole.

@nojaf
Copy link
Contributor

nojaf commented Jan 18, 2023

Hi @baronfel, could you use any help with this?
Is there anything blocking here? Like waiting for a dependency to update?
Or is the remaining work about fixing those last failing tests?

@vzarytovskii
Copy link

Hi @baronfel, could you use any help with this?
Is there anything blocking here? Like waiting for a dependency to update?
Or is the remaining work about fixing those last failing tests?

I would imagine it waits for new FCS update, the one without msbuild dependency. Should be out same time with 7.0.200

@nojaf
Copy link
Contributor

nojaf commented Jan 18, 2023

Would there be any reason not to use 43.7.200-preview.22569.1 if the MSBuild dependency thing indeed blocks everything?
I imagine all the dependencies would also need to be bumped to that preview version.

@nojaf
Copy link
Contributor

nojaf commented Jan 19, 2023

Sorry, that was a bad suggestion, I just noticed that the preview FSC has a dependency on an FSharp.Core version that is not on NuGet:

image

@baronfel baronfel marked this pull request as ready for review February 19, 2023 19:26
@baronfel baronfel changed the title WIP F# 7 update Update to an FCS that understands F# 7 Feb 19, 2023
@baronfel
Copy link
Contributor Author

There's one more test failure around the new ^-formatting for types, but other than that I think we might be good to go.

@baronfel baronfel merged commit 2ca73e9 into ionide:main Feb 20, 2023
@baronfel baronfel deleted the fcs-.net7 branch February 20, 2023 03:39
Booksbaum added a commit to Booksbaum/FsAutoComplete that referenced this pull request Feb 21, 2023
fixed with ionide#1043 (Upgrade to dotnet 7 & new FCS)
(see ionide#1037 (comment) )
Booksbaum added a commit to Booksbaum/FsAutoComplete that referenced this pull request Mar 3, 2023
fixed with ionide#1043 (Upgrade to dotnet 7 & new FCS)
(see ionide#1037 (comment) )
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