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

[RFC FS-1114] Make long-established deprecation warning messages into errors #12063

Merged
merged 12 commits into from
Sep 5, 2021

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Aug 30, 2021

@dsyme dsyme added the Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language. label Aug 31, 2021
@dsyme
Copy link
Contributor Author

dsyme commented Sep 1, 2021

@KevinRansom This is now ready (though I know you're prioritizing the F# 6.0 labelling, that comes first).

The revisions go into preview with this PR. These don't have to be part of F# 6.0, happy for you to decide when to integrate

The PR also adds LangVersion to LexBuffer and this simplifies the many places we pass functions through. FCS now respects the language version for ParseFile operations, e.g. if one comes in explicitly via the ParsingOptions (which will stem from project options). This is correct API behaviour and the tests have been adjusted for it, though it won't change any actual IDE behaviour since no IDEs just parse files, they always ParseAndCheck files.

@auduchinok
Copy link
Member

though it won't change any actual IDE behaviour since no IDEs just parse files, they always ParseAndCheck files

Just for the record, we use ParseFile a lot in Rider.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 1, 2021

Just for the record, we use ParseFile a lot in Rider.

Right, I'm assuming you still collect diagnostics from CheckFile (of course).

Anyway either way it's still right that ParseFile respect language version.

@auduchinok
Copy link
Member

auduchinok commented Sep 1, 2021

Right, I'm assuming you still collect diagnostics from CheckFile (of course).

We currently report parse and type check errors separately, and the parse ones come from ParseFile. It was implemented this way before the LangVersion was introduced, and I didn't know ParseFile worked differently these days.

It seems we have to either use ParseAndCheck for parse errors too, or to just wait until this PR is merged. Given we have some time before the next release, I'll probably just wait for this PR to land.

@dsyme
Copy link
Contributor Author

dsyme commented Sep 1, 2021

We currently report parse and type check errors separately, and the parse ones come from ParseFile. It was implemented this way before the LangVersion was introduced, and I didn't know ParseFile worked differently these days.

Today, ParseFile effectively uses preview language version

  • FSharpParsingOptions doesn't have any langversion info.

  • When parsing "lexbuf.SupportsFeature" is implemented as a lambda returning true, meaning all features are considered supported

In practice I don't think there's a case where this matters today. However with this PR we start to emit errors for the long-deprecated features, but that depends on langversion, so it's important to pass the right langversion through.

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Thanks for this

@dsyme dsyme merged commit c88b795 into dotnet:main Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Theme-Simple-F# A cross-community initiative called "Simple F#", keeping people in the sweet spot of the language.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants