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

Accept legacy language features #20454

Merged
merged 1 commit into from
May 23, 2024

Conversation

noti0na1
Copy link
Member

Fix #20419

  • Accept legacy language features
  • For other unknown options, it will still fail.

@noti0na1 noti0na1 marked this pull request as ready for review May 22, 2024 22:48
Copy link
Contributor

@som-snytt som-snytt left a comment

Choose a reason for hiding this comment

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

A nice compromise. It just silently ignores legacy cruft. Legacy choices are not advertised in help message. I might expect a warning. But it does not warn for repeated choice -language:postfixOps,postfixOps, compare -feature -feature, so a relaxed approach is fine. It might have been nice to unify legacyArgs, where the legacy form is empty string, but that is a stretch.

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

The solution hard-codes the supported language imports. That means everytime we add a new one we also have to update this part. It might be better to explore a different solution. Don't check anything when we build Settings, but when we first compute enabledLanguageFeaturesBySetting in Feature. There we can check each option to see whether it corresponds to an object in the language object.

The tricky thing is how to do the validation only once per run. We could have a flag in SettingsState that tells us whether language imports have been validated.

The other tricky thing is how to add help messages without duplicating all imports in Feature, like we do now. I don't have an answer to that. Without an answer I am not sure this alternative approach will work out.

@som-snytt
Copy link
Contributor

This PR just lists strings not to error on. They have no other representation.

(The principled solution would introduce deprecated feature flags, with labored text that they are no-ops for backward convenience; but that seems overkill.)

@noti0na1
Copy link
Member Author

Don't check anything when we build Settings, but when we first compute enabledLanguageFeaturesBySetting in Feature. There we can check each option to see whether it corresponds to an object in the language object.

Since we want to display every available feature and check users' input at the very beginging, I don't think it is a better solution to encode them in the language object.

@noti0na1 noti0na1 merged commit 7bdeb0b into scala:main May 23, 2024
19 checks passed
@noti0na1 noti0na1 deleted the accept-scala2-language-features branch May 23, 2024 14:19
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.

Unknown -language option yields an error
4 participants