WarningConfig DSL

[satorg]

See #4. I just went ahead and copied the suggested DSL into the project (kudos to @laughedelic, thank you!).

Topics to discuss:

1. The DSL classes location. Personally I would prefer to keep DSL and utility classes separated. I.e. I'd like to import DSLs from a dedicated package like:

   import org.typelevel.scalacoptions.dsl._
   

   which would only import names corresponding to the target DSL, i.e. silent, error, cat, msg, etc, but NOT WarningConfig nor WarningFilter.

   The same thoughts regarding more common option names like deprecation, feature, unchecked, lint, fatalWarnings – from my prospective all those should be separated from such utility methods like optionsForVersion, tokensForVersion and so on.

2. Rendering to the target language via toString. Just not sure if it is the best way to render.

3. The asScalacOption method. Usually utility methods inside ScalacOptions take on building ScalacOption, so perhaps this logic would be more appropriate out there.

[laughedelic]

Thank you for opening a pull request @satorg. I refrained from doing that to have a discussion on the design with the maintainers before contributing it, but I guess it can happen here as well.

I would appreciate if you included me as a co-author in the git commit message:

Co-authored-by: Alexey Alekhin <[email protected]>

[satorg]

@laughedelic oh, sorry. Seems I misread your last sentence in the issue's description as a suggestion to open a pr on your behalf. Sorry about that. Feel free to open your pr please and I'm going to close this one. Thanks!

[laughedelic]

Oh, I really didn't mean that I have anything against this PR. Sorry if it appeared so! 😅

Please, feel free to reopen and carry on, I think it will be better since you seem more familiar with the codebase here 👍 My only ask was an acknowledgement for the initial contribution, but it's not really important.

[satorg]

Sure, here it is. Sorry for the misunderstanding and thank you for the great idea!

[satorg]

Even more things to consider. It seems that Scala3 has got a different set of message filters comparing to Scala2. Below is output from Scala v3.2.0 for -Wconf:help:

Configure compiler warnings.
Syntax: -Wconf:<filters>:<action>,<filters>:<action>,...
multiple <filters> are combined with &, i.e., <filter>&...&<filter>

<filter>
  - Any message: any

  - Message categories: cat=deprecation, cat=feature, cat=unchecked

  - Message content: msg=regex
    The regex need only match some part of the message, not all of it.

  - Message id: id=E129
    The message id is printed with the warning.

  - Message name: name=PureExpressionInStatementPosition
    The message name is printed with the warning in verbose warning mode.

Note that there's no origin nor site filter anymore whereas a couple of new filters (comparing to all Scala2 compilers) was introduced: id and name.

Therefore, it seems that instances of MessageFilter should be ScalaVersion-bound (in a similar way as ScalacOption instances are).

Furthermore, it seems that different Scala versions may have different sets of supported cat parameters.

[som-snytt]

Scala 3 is quite different, so it's unlikely -Wconf will converge. I assume it will evolve for usability. Maybe all it needs is a sweet DSL.

[satorg]

That's fair. I wonder though, could we rely on this basic syntax remains unchanged:

Syntax: -Wconf:<filters>:<action>,<filters>:<action>,...
multiple <filters> are combined with &, i.e., <filter>&...&<filter>

Despite that filters and actions can be different in future Scala3 versions...

[DavidGregory084]

Thanks for raising this @satorg and thanks @laughedelic for the suggestion!

Personally I would prefer to keep DSL and utility classes separated.

I agree that it would probably be better to provide this via an explicit import rather than adding it to the base package of the library. Perhaps something like import org.typelevel.scalacoptions.warnings.dsl._ would be good?

Alternatively, we could nest all of the DSL classes inside an object WarningConfig. Then users could import org.typelevel.scalacoptions.WarningConfig._ or use qualified names according to their preference?

Rendering to the target language via toString. Just not sure if it is the best way to render.

Personally I prefer to avoid this, just because for debug purposes it obscures the existence of the WarningConfig case class. Perhaps the logic of toString should be inlined into asScalacOption?

Usually utility methods inside ScalacOptions take on building ScalacOption, so perhaps this logic would be more appropriate out there.

Yes, I like that idea - what if we provided a def warningConfig(rules: (WarningFilter, WarningAction)*) in ScalacOptions, which would allow us to remove the WarningConfig class entirely and keep only the actions and filters?

[satorg]

Perhaps something like import org.typelevel.scalacoptions.warnings.dsl._ would be good? Alternatively, we could nest all of the DSL classes inside an object WarningConfig. Then users could import org.typelevel.scalacoptions.WarningConfig._ or use qualified names according to their preference?

Yes it would :) At least, looks good to me.

Either way would work I think. But personally, I would like to have it consistent with the "core" DSL. I mean, currently the core DSL is imported from the ScalacOptions object. So if we like it being located there, then the "warnings" DSL would be better to have placed in the WarningConfig object as well.

Otherwise if we prefer more package-like import path (i.e. org.typelevel.scalacoptions.warnings.dsl._) then the core DSL would be better to relocate into something like org.typelevel.scalacoptions.dsl._ or org.typelevel.scalacoptions.core.dsl._.

[satorg]

I would say initially I was considering a package-like import paths everywhere. But I've kinda reconsidered that and now leaning towards just putting it into the WarningConfig object. Why? Just because it makes easier the following style to follow:

or use qualified names according to their preference?

I.e. it makes easier to switch between either importing'em all or importing just DSL objects and use them as prefixes for DSL entries.