scalafix icon indicating copy to clipboard operation
scalafix copied to clipboard

Scala 3 support

Open bjaglin opened this issue 4 years ago • 10 comments

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

  • [x] running syntactic & semantic rules against Scala 3 sources
    • scalafix
      • ~new parameter (optional for backward compatibility) to feed the input/output dialect~ reuse scalaVersion, see https://github.com/scalacenter/scalafix/pull/1392#pullrequestreview-655259487
      • support scala3 as input&output in testkit
        • sbt 1.5 must be used to compile input/output
        • bootstrap scala-3[.0.0-RC2] folder with all revelevant tests (maybe check if sbt can remove the RC suffix?) & move scala 2.12 syntax currently in scala to scala-2.12.-x folders
        • update https://github.com/scalacenter/scalafix.g8
      • document what is not yet supported in Scala3
        • try out the most common community rules to prioritize adding support for what's needed
    • sbt-scalafix
      • ~provide the input/output dialect based on the build scalaVersion~ always provide scalaVersion, see https://github.com/scalacenter/scalafix/pull/1392#pullrequestreview-655259487
      • support scalaVersion := "3" in scalafixEnable
      • document that to use scala3, the user must rely on SemanticdbPlugin to abstract usage of semanticdb-scalac
  • [ ] cross-building scalafix-cli , scalafix-core & scalafix-rules for Scala 3?
  • [ ] adapt ExplicitResultTypes usage of the Scala 2 presentation compiler to Scala 3 https://github.com/scalacenter/scalafix/issues/1583
    • ~support only 2.13 & 3 via TASTy~ we don't have enough information to shorten names
      • ~stop using presentaton compiler completely?~
      • ~drop support for 2.12 and earlier?~
    • ~can we expose TASTy info through scalafix API to avoid duplicated code in scala-rewrites?~

bjaglin avatar Jan 21 '21 12:01 bjaglin

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

mlachkar avatar Mar 30 '21 12:03 mlachkar

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.

tgodzik avatar Mar 30 '21 13:03 tgodzik

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.
    • The default InternalDialect instance seems to be used as well, so we need to forward the parsing dialect all the way to the patch evaluation if we want to be able to use Scala3 syntax as output.
    • Even if this is done, I am not sure the prettyprinter fails fast for all cases when an unsupported node (from the dialect perspective) is found: for example, https://github.com/scalameta/scalameta/blob/1ea01c78674038f73f46fdd59c8528dd0c96fc54/scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala#L275 shows that usage of extension methods in a dialect that does not allow it might generate broken code instead of failing (I haven't actually tried!).
  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 avatar May 11 '21 19:05 bjaglin

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.

bjaglin avatar May 12 '21 13:05 bjaglin

@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.

tgodzik avatar May 12 '21 14:05 tgodzik

@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!

bjaglin avatar May 12 '21 19:05 bjaglin

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.

tgodzik avatar Jun 05 '21 13:06 tgodzik

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!

bjaglin avatar Jun 05 '21 19:06 bjaglin

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?

rvacaru avatar Jul 15 '22 12:07 rvacaru

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

bjaglin avatar Jul 15 '22 13:07 bjaglin

"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:

  • https://github.com/scalacenter/scalafix/issues/1680
  • https://github.com/scalacenter/scalafix/issues/1583
  • https://github.com/scalacenter/scalafix/issues/1682

bjaglin avatar Oct 02 '22 20:10 bjaglin