search

ionide / FsAutoComplete

F# language server using Language Server Protocol
Other
389 stars 151 forks source link

merge from main, bump FCS #1237

Closed baronfel closed 4 months ago

baronfel commented 4 months ago

cc @Booksbaum - the FCS changes impacted the explicit type annotation codefix. Broadly, SynSimplePats are no longer used for constructor parameters, so the existing visitors had to be updated a bit. The tests are all green again now, but I'd be curious if you wanted to look at them and make sure the changes make sense.

Booksbaum commented 4 months ago

I haven't keep up with FCS changes (or FSAC changes ... and forgot a lot about FCS usage in FSAC...). So I can't really judge the changes more than you.
But at a glance everything looks fine and reasonable 👍 (and cleaner structured 👍👍)
And hey: that's exactly what the extensive tests are for: If they are green everything should be fine 😄

(Though it is indeed strange to change VisitSimplePats to use SynPat... and not just remove VisitSimplePats completely in favor of VisitPat. But it seems to work so everything's fine... . And I remember the difference between SynPat and SynSimplePat was a constant headache... so this API change is great!)