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

confusion around necessity of adding explicit result types and implicits #334

Open
OlegYch opened this issue May 30, 2022 · 7 comments
Open

Comments

@OlegYch
Copy link

OlegYch commented May 30, 2022

https://github.com/scalacenter/scala3-migrate#fix-syntax-incompatibilities does not mention adding result types to anything, but it currently does via ExplicitResultTypes rule
it looks like it should not because adding result types is not necessary to fix syntax
moreover afaik it is only necessary to add result types to implicits to compile with scala3, adding them everywhere is noisy
i guess it would be enough to just remove ExplicitResultTypes usage, since migrate would later invoke InferTypes anyway

and is ExplicitImplicitsRule really necessary?

@mlachkar
Copy link
Contributor

mlachkar commented Jun 2, 2022

You're right. It's not mentioned there. The readme is a outdated a little, the most recent documentation is here https://docs.scala-lang.org/scala3/guides/migration/scala3-migrate.html (which doesn't mention the explicitResultType rule neither)
In this post we explain why it's important to have public api typed: link

First step: Make sure that all methods/functions of the public API have an explicit result type. 
If not, we require to run Explicit ResultType rule, which will type explicitly public and protected methods. 
Those types will be kept even after scala 3 migration. This is a good practice anyway, even in the context 
of a single Scala version, so that the APIs do not change based on the whims of the compiler’s 
type inference. It will ensure that the following steps do not alter the public API.

this step is done in the migrate-syntax, maybe you can skip it ?

@OlegYch
Copy link
Author

OlegYch commented Jun 2, 2022

@mlachkar i realize having explicit return types is a good practice, but it is not related to migration and actually hinders it

@adpi2
Copy link
Member

adpi2 commented Jun 2, 2022

@OlegYch Having explicit return types is a necessary prerequisite of the migrate step because migrate is incremental: it migrates one file at a time. The migration of one file should not impact the migration of other files. In other words the migrate task should only operate on private types.

@OlegYch
Copy link
Author

OlegYch commented Jun 2, 2022

@adpi2 hm, can adding explicit result types be moved to migrate or separate step then? because currently migrate is not working on one of my projects (consuming > 20gb of heap) and does nothing on another, but id like to use migrate-syntax for other things it does

@adpi2
Copy link
Member

adpi2 commented Jun 2, 2022

Yes, maybe we could try to move the adding result types into the migrate step. Would you like to contribute?

Or, as an alternative, you can add the sbt-scalafix plugin in your project and execute the rules used by migrate-syntax.

@OlegYch
Copy link
Author

OlegYch commented Jun 2, 2022

@adpi2 i'm not sure if there is a separate rule for adding result types just to implicit definitions, can you perhaps give a hint?

@adpi2
Copy link
Member

adpi2 commented Jun 2, 2022

I don't think there is unfortunately. An "implicit only" configuration would be a good feature to add to the ExplicitResultTypes rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants