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

Scala 3 support #1316

Closed
2 of 5 tasks
bjaglin opened this issue Jan 21, 2021 · 11 comments
Closed
2 of 5 tasks

Scala 3 support #1316

bjaglin opened this issue Jan 21, 2021 · 11 comments
Labels

Comments

@bjaglin
Copy link
Collaborator

bjaglin commented Jan 21, 2021

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

@mlachkar mlachkar added this to the Scala 3 support milestone Mar 10, 2021
@mlachkar
Copy link
Collaborator

mlachkar commented Mar 30, 2021

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

@tgodzik
Copy link
Contributor

tgodzik commented Mar 30, 2021

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
Copy link
Collaborator Author

bjaglin commented May 11, 2021

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.
  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
Copy link
Collaborator Author

bjaglin commented May 12, 2021

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
Copy link
Contributor

tgodzik commented May 12, 2021

@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
Copy link
Collaborator Author

bjaglin commented May 12, 2021

@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
Copy link
Contributor

tgodzik commented Jun 5, 2021

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
Copy link
Collaborator Author

bjaglin commented Jun 5, 2021

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
Copy link
Contributor

rvacaru commented Jul 15, 2022

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
Copy link
Collaborator Author

bjaglin commented Jul 15, 2022

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

@bjaglin
Copy link
Collaborator Author

bjaglin commented Oct 2, 2022

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

@bjaglin bjaglin closed this as completed Oct 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants