search

scalacenter / scalafix

Refactoring and linting tool for Scala
https://scalacenter.github.io/scalafix/
BSD 3-Clause "New" or "Revised" License
832 stars 186 forks source link

Scala 3 support #1316

Closed bjaglin closed 2 years ago

bjaglin commented 3 years ago

Opening this for visibility and to track the upcoming work for supporting Scala 3

mlachkar commented 3 years ago

To cross-build for Scala3 , we need metaconfig to be release for scala 3 https://github.com/scalameta/metaconfig/issues/103, or use another config library that doesn't contain macros or release for scala 3

tgodzik commented 3 years ago

This seems to be an issue also in mdoc. I think we might need to migrate to something like sconfig or make metaconfig support Scala 3. Not sure what is the best solution here.

bjaglin commented 3 years ago

Thanks to @mlachkar, there has been good progress in inferring the parsing dialect based on the provided scalaVersion CLI/API args, and the sbt-scalafix companion with support for the built-in dotty flags for semanticdb output is almost ready.

Before ticking the first item and ahead of documentation updates, I still wonder how Scala3 rules (or portable Scala2/3 rules) could safely consume/produce Scala3 ASTs with new syntax.

  1. Quasiquotes seem out of the picture for parsing & patches.
    • They haven't been updated to support the new Scala3 nodes
    • Even if they were, we would still have to inject the proper dialect in scope of fix as it's currently coming from InternalDialect, which defaults to the scalaVersion scalafix runs with https://github.com/scalameta/scalameta/search?q=%22trait+InternalDialect%22. That would be a breaking change in the fix signature I believe (which of course could be an option).
  2. For patches, the "Scala 3" AST nodes documented in https://scalameta.org/docs/trees/examples.html are not attached to any dialect as such, but the prettyprinter triggred on the .syntax call in PatchInternals captures an implicit Dialect to adapt its behavior.
  3. Significant indentation is supported on the parser, but not on the prettyprinter, so patches will always contain curly braces from what I see.
    • I guess that's a limitation that won't go away anytime soon and thus should just be documented, unless there is an effort in supporting such an output in Scalameta?

@tgodzik is my analysis correct?

bjaglin commented 3 years ago

Regarding (2) and (3): actually most of the patches take strings as parameters to inject (except addGlobalImport which seems to be the only factory method for Patch that can take an arbitrary tree), so there is not much scalafix-core can/has to do to check the rule emits correct code... The questions remain for addGlobalImport though, but the scope is very limited.

tgodzik commented 3 years ago

@bjaglin

  1. We haven't been thinking about those, most should work though, but we should open up an issue in scalameta to track those that do not work. For example enums should be ok, but enum cases probably don't work as well as givens. We can try to fix them along the way.
  2. We can add more checks to fail in the instances that an unsupported dialect is detected. I didn't think it would be needed that much.
  3. We have a couple of choices here:
    • always skip braces when the dialect supports it
    • print with braces, scalafix would drop them afterwards based on settings
    • add an option to .syntax to print with indentation

Overall, please open up issue about anything problematic, I will try to fix them as quickly as possible. We've been mostly figuring things out for Scalafmt use cases up until now.

bjaglin commented 3 years ago

@tgodzik thanks for the prompt feedback and your hard work on Scala 3 support in scalameta! I/we will follow-up on scalameta as the needs materialize. As mentioned above, I probably over-estimated the impact of (2) and (3) anyway!

tgodzik commented 3 years ago

Some other things that might be useful would be to be able to declare different rules for a subset of files. We could have similar thing to filOverrides in Scalafmt.

The context here is that I am trying to run the current rules on metals, but getting some issues like:

[error] (mtags3 / Compile / scalafix) scalafix.sbt.InvalidArgument: 2 errors
[error] [E0] The Scala compiler option "-Ywarn-unused" is required to use OrganizeImports with "OrganizeImports.removeUnused" set to true. To fix this problem, update your build to use at least one Scala compiler option like -Ywarn-unused-import (2.11 only), -Ywarn-unused, -Xlint:unused (2.12.2 or above) or -Wunused (2.13 only).
[error] [E1] The Scala compiler option "-Ywarn-unused" is required to use RemoveUnused
[error] To fix this problem, update your build to use at least one Scala compiler
[error] option like -Ywarn-unused, -Xlint:unused (2.12.2 or above), or -Wunused (2.13 only)
[error] Total time: 38 s, completed Jun 5, 2021 3:23:01 PM

and

[error] error: The ExplicitResultTypes rule needs to run with the same Scala binary version as the one used to compile target sources (3.0). To fix this problem, either remove ExplicitResultTypes from .scalafix.conf or make sure the scalafixScalaBinaryVersion setting key matches 3.0.
bjaglin commented 3 years ago

Some other things that might be useful would be to be able to declare different rules for a subset of files. We could have similar thing to fileOverrides in Scalafmt.

I was about to suggest using a different configuration file (since I believe we don't need file-level granularity?) but I see https://github.com/scalameta/metals/pull/2860/files did precisely that!

rvacaru commented 2 years ago

Hi all, as part of GSoC 2022 I'm currently working on cross compiling scalafix-core, scalafix-rules, scalafix-reflect and scalafix-cli to scala 3, hence allowing for the implementation of ExplicitResultTypes afterwards.

@bjaglin I'm not able to update the issue by checking out the checkbox related to this, can I get permissions to do so or could you do it for me?

bjaglin commented 2 years ago

@rvacaru I am going to update the description now and will tick the box once we have the artifacts published

bjaglin commented 2 years ago

"Scala 3 support" is quite broad. Now that we have had support for implementing syntactic and semantic rules targeting Scala 3 source files using Scala 2.x since Scalafix 0.9.28, closing this in favor of: