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

Stop @newtype to warn for implicit conversions #63

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marcesquerra
Copy link

If you neither set the '-language:implicitConversions' flag, nor the 'import scala.language.implicitConversions', the '@newtype' construct warns for implicit conversions.

I've tried, and this PR prevents it.

@marcesquerra
Copy link
Author

This is the reopening of this PR (given the other had problems with the travis-ci cache):
#62

Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this trigger a warning if there is already such an import in the file?

@marcesquerra
Copy link
Author

@kailuowang No, not really

In fact, if you add your own custom code to the companion object, and your code has implicit conversions, that custom code will behave as expected.

That is, if you neither have the import nor the compiler flag, it will trigger the corresponding warning. It will do nothing otherwise.

@kailuowang
Copy link
Contributor

Sorry I am still on paternity leave. What I would like to test is if having delicate import trigger an unused import warning.

@marcesquerra
Copy link
Author

Hi @kailuowang , thanks for the feedback. After your comment, I've run a battery of tests and made some corrections to the PR out of the results.

All the experiments I've run with the following compiler flags:

scalacOptions ++= Seq(
  "-feature",
  "-Ywarn-unused:imports"   // on scala 2.12 and 2.13
  // "-Ywarn-unused-import" // on scala 2.11
)

I've run all the experiments with two versions of a very simple main file:

version 1 (case class):

import io.estatico.newtype.macros.newtype


object Main extends App {

  @newtype case class Foo(i: Int)

  println("Hello, World!")
}

version 2 (normal class):

import io.estatico.newtype.macros.newtype


object Main extends App {

  @newtype class Foo(i: Int)

  println("Hello, World!")
}

I tried to compile the two versions with scala 2.11.12, 2.12.11 and 2.13.1. Both with the current version of newtype and with the one inside this PR.

Note: HK means warning about missing the higherKinded language flag. IC means warning about missing the implicitConversions language flag.

Currently

        case class      | class
2.11    HK x 3, IC x 1  | HK x 3
2.12    HK x 3, IC x 1  | HK x 3
2.13    IC x 1          | -

PR

        case class      | class
2.11    -               | _
2.12    -               | _
2.13    -               | _

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

Successfully merging this pull request may close these issues.

2 participants