scalacenter / scalafix

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

scala 3: ExplicitResultTypes #1583

Open mlachkar opened 2 years ago

mlachkar commented 2 years ago

Adapt Explicit result types for Scala 3

The current state

ExplicitResultTypes rule calls the scala 2 presentation compiler to infer types for each val, def or var. To make this rule available for scala 3, we need to re-write it to interface this time with the Scala 3 presentation compiler, in other words, ExplicitResultTypes rule for Scala 3, need to be written in Scala 3.

How to implement ?

flowchart LR
A(Scalafix-cli) -->|depends on| B(Scalafix-rules)

The actual design of Scalafix requires that the module scalafix-rules uses the same Scala Version than the Scalafix-cli module. To be able to use the Scala 3 presentation compiler, the rule needs to be rewritten in Scala 3 and therefore scalafix-cli needs to be able to load a Scala 3 rule.

Most likely implementation

  1. Create a Java interface for ExplicitResultTypes that provides the minimum needed methods for it to work. The interface would look like below:
    public interface ExplictResultTypesInterface {
    List<PositionWithString> fix(Optional<Object>: global, Path: relativePathToSemanticDbFile)
    Object createGlobal(scalacClasspath, scalacOptions)
    }
    • Currently ExplicitResultTypes rule needs a SemanticDocument, that cannot be passed as a parameter to the java method, so each implementation of this rule will need to recreate it from its relative path.
    • The fix method would return a list of Position and String, which should be enough to create a scalafix.Patch.
    • The global instance needs also to be created differently depending on the Scala Version. We can maybe stop using ScalafixGlobal and use directly scala.tools.nsc.interactive.Global (in Scala 2) and dotty.tools.dotc.interactive.InteractiveDriver (in Scala 3). ScalafixGlobal is almost only wrapping the Global from the presentation compiler. In this Java interface, it would be an Object, and then each implementation would cast it as a Global instance or InteractiveDriver instance depending on the Scala version.

Here is an example of the interface implemented by Metals to deal with the same issue. Their global is called PresentationCompiler, and in their case, they only infer the type for one position, whereas ExplicitResultTypes needs to do it for the entire file.

  1. The module Scalafix-cli or Scalafix-rules (still need to be defined) would need some specific code to be able to classload the right jar of ExplicitResultTypes , in other words either the version written in Scala 2 or Scala 3. (link to metals that does that, or other project that does that too)
  1. Write the new ExplicitResultTypes for scala 3, that implements the ExplictResultTypesInterface interface. We can definitely get inspired by Metals implementation for inferType code action (link)

Other possible implementation

If we are able to cross compiler scalafix-core, scalafix-cli in Scala 3 , it should be possible to write a specific version of Explicit Result types for Scala 3.

mlachkar commented 2 years ago

With a minimal build:

val scala3Version = "3.1.2"
lazy val root = project
  .in(file("."))
  .settings(
    scalaVersion := scala3Version,
    libraryDependencies += "org.scalameta" %% "scalameta" % "4.5.6" cross CrossVersion.for3Use2_13
  )

I m able to compile and run the following code

import scala.meta.*

@main def hello: Unit =
  val program = """object Main extends App { print("Hello!") }"""
  val tree = program.parse[Source].get
  println(Term.Apply(Term.Name("function"), List(Term.Name("argument"))))
  println(s"$tree")
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 commented 2 years ago

To clarify, we've gone towards the "Other possible implementation", so this is therefore blocked by https://github.com/scalacenter/scalafix/issues/1680

guizmaii commented 1 year ago

Any news maybe? It's been a year since the last comment

bjaglin commented 1 year ago

Hi @guizmaii ! Unfortunately there hasn't been any work on that topic since the GSoC last year. I am the only maintainer with limited time and no longer use Scala in my day-to-day job, so the last few months have been merely about keeping Scalafix up to date with the ecosystem. Contributions are of course very welcome!

tgodzik commented 1 year ago

Potentially, could we reuse the presentation compiler module that will come for 3.3.2 with a potential fallback on the metlas mtags?

Did we discuss that at any point? That is a binary compatible interface https://github.com/scalameta/metals/blob/63a65d297c0c70173869603e8f1cf0950a665cd6/mtags-interfaces/src/main/java/scala/meta/pc/PresentationCompiler.java#L136

bjaglin commented 11 months ago

could we reuse the presentation compiler module that will come for 3.3.2

TIL: https://contributors.scala-lang.org/t/stable-presentation-compiler-api/6139 & https://github.com/lampepfl/dotty/pull/17528 :muscle: :star_struck:

Did we discuss that at any point? That is a binary compatible interface https://github.com/scalameta/metals/blob/63a65d297c0c70173869603e8f1cf0950a665cd6/mtags-interfaces/src/main/java/scala/meta/pc/PresentationCompiler.java#L136

We haven't, I was not aware that the mtags implementation had effectively been upstreamed - it looks like a somewhat low-hanging fruit to integrate it to scalafix!

I am unsure about the need for a Java API wrapper (thinking about the classloading penalty mostly), as we already build Scala 3 artifacts thanks to the 2022 GSoC. There are a few items to iron out before publishing, but there is nothing particularly complex. The biggest challenge was obviously the lack of scalameta quasiquote support in Scala 3, preventing many community rules to easily cross-build and therefore requiring support for loading 2.13 quasiquotes-backed community rules with scalafix-cli_3 if we want a smooth transition to scalafixBinaryScalaVersion := 3 (driven by ExplicitResultType and the usage of the PC). But I believe there is and ongoing effort on porting the macros to Scala 3?

tgodzik commented 11 months ago

I am unsure about the need for a Java API wrapper (thinking about the classloading penalty mostly),

We don't have to do a separate classloader for sure, we needed it in Metals mostly for supporting multiple Scala versions at the same time.

But I believe there is and ongoing effort on porting the macros to Scala 3?

Yes! I almost managed to get it cross publishing, I should get back to it soon.

TonioGela commented 3 months ago

Any news on this front @tgodzik ?

tgodzik commented 3 months ago

I will take a look at a PoC if I have time on the weekend.

tgodzik commented 3 months ago

So this PoC seems to work -> https://github.com/scalacenter/scalafix/compare/main...tgodzik:scalafix:add-inferred-type?expand=1 with the major caveat:

Some further work:

@bjaglin What do you think about this approach? Is there anything disqualifying about it?

Currently the only thing that is really changed is defnType method and the constructor