Skip to content

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Nov 7, 2025

Fixes #24333

Option aliases are SettingAlias which may have a Deprecation with an optional message.

@Gedochao Gedochao added backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. needs-minor-release This PR cannot be merged until the next minor release labels Nov 7, 2025
@Gedochao Gedochao added this to the 3.8.0 milestone Nov 7, 2025
val Vprint: Setting[List[String]] = PhasesSetting(VerboseSetting, "Vprint", "Print out program after", aliases = List("-Xprint"))
val XshowPhases: Setting[Boolean] = BooleanSetting(VerboseSetting, "Vphases", "List compiler phases.", aliases = List("-Xshow-phases"))
val Vprint: Setting[List[String]] = PhasesSetting(VerboseSetting, "Vprint", "Print out program after", aliases = List("-Xprint*"))
val XshowPhases: Setting[Boolean] = BooleanSetting(VerboseSetting, "Vphases", "List compiler phases.", aliases = List("-Xshow-phases*"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be clearer to add a new list deprecatedAliases?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1
that'd actually be more readable for when the API would be read outside of the compiler, as well.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, the people have spoken!

@som-snytt
Copy link
Contributor Author

som-snytt commented Nov 7, 2025

rando download error

Edit: followed by need to update shapeless-3 options.

@som-snytt
Copy link
Contributor Author

Technically, the number of parameters to a method should be capped at a gazillion, which was one reason to like the first, cute solution.

@som-snytt som-snytt marked this pull request as ready for review November 9, 2025 22:03
preferPrevious: Boolean = false,
propertyClass: Option[Class[?]] = None,
deprecation: Option[Deprecation] = None,
deprecatedAliases: List[(String, Deprecation)] = Nil,
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: wouldn't it increase readability to have a

case class SettingAlias(name: String, deprecation: Option[Deprecation] = None)

?
or at the very least, use a named tuple here 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'm not accustomed to such fancy language features. I'll update it.

I found consolation in the thought that deprecated aliases are rare, and it's OK if they are awkward to write.

@Gedochao Gedochao requested a review from tgodzik November 12, 2025 07:29
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM

@som-snytt
Copy link
Contributor Author

Pushed @Gedochao suggestion, which improves read- and write-ability.

A unit test broke that string-interpolated alias, which changed type. I needed -Wtostring-interpolated today.

I hope the code base can progress and turn on warnings!

@som-snytt
Copy link
Contributor Author

This will fail until shapeless-3 community project is updated.

@Gedochao
Copy link
Contributor

This will fail until shapeless-3 community project is updated.

If that's the case, then I suppose this won't go in until 3.10.
cc @WojciechMazur

@som-snytt
Copy link
Contributor Author

@Gedochao I don't understand. We can't deprecate options?

@WojciechMazur
Copy link
Contributor

The PR to dotty-staging was merged, please set the submodule so that it points to current main, seems like the notification about created PR were lost.

Deprecations of existing scalacOptions would be quite important for LTS 2, so these can be possibly removed for LTS 3 if we'd need to. Base don that it's likely going to be back-ported to RC2

@Gedochao It was only a local change to replace -Xfatal-warnings with -Werror so it should be fine. It has has not made it in time for RC1 becouse it was stuck on dotty-stagging fork PR (also we'd need to re-evaluate write access to that repo, seems like currently only a single person can merge PRs there)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. needs-minor-release This PR cannot be merged until the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement a deprecation mechanism for aliases of existing options

5 participants