tek / splain

better implicit errors for scala
MIT License
370 stars 29 forks source link

Singleton types are erased at display. #43

Open tribbloid opened 3 years ago

tribbloid commented 3 years ago

Here is a simple example:


    val a = 1
    val b = 2

    implicitly[a.type =:= b.type]

Generates the error::

[Error] /......:46: implicit error;
!I e (Cannot prove that a.type =:= b.type.): Int =:= Int
one error found

The first section Cannot prove that a.type =:= b.type is a customised implicitNotFound message, which is helpful, the second part Int =:= Int is added by splain, which is clueless.

If the ImplicitNotFound message hasn't been defined, or it is buried deep in a chain implicit resolution tree, there will be no way to figure out the cause.

My proposed fix is to show the full inheritance tree for not just the type, but all its parameters, e.g. for type a.type =:= b.type, the result should look like this:

-+ a.type =:= b.type
 :       `-+ [ 2 ARGS ] :
 :         !-+ a.type .................................................................................................................. [0]
 :         : !-+ Int ..................................................................................................................... [1]
 :         :   !-+ AnyVal
 :         :     !-- Any ..................................................................................................................... [2]
 :         !-+ b.type .................................................................................................................. [3]
 :           !-- Int ..................................................................................................................... [1]
 !-+ a.type <:< b.type
   :       `-+ [ 2 ARGS ] :
   :         !-- a.type .................................................................................................................. [0]
   :         !-- b.type .................................................................................................................. [3]
   !-+ java.io.Serializable
   : !-- Any ..................................................................................................................... [2]
   !-+ a.type => b.type
     :       `-+ [ 2 ARGS ] :
     :         !-- a.type .................................................................................................................. [0]
     :         !-- b.type .................................................................................................................. [3]

I already have the code extract the tree, I just don't know where to inject into your library. Could you point me to the right part of your code?

tek commented 3 years ago

interesting find!

you can add a SpecialFormatter subclass. its first parameter is a Type that should contain a.type =.= b.type, you can then pattern match and turn it into a Formatted, for which you can also define a new variant so it can be handled later here if needed.

you will need to add the SpecialFormatter object to this list.

do you need directions for setting up a unit test?

tribbloid commented 3 years ago

So It should be a 'SingleTypeFormatter' that handles UniqueSingleType. This is the easy part, in fact, the line of code of a can be easily found and displayed. (The same thing applies to path-dependent type)

The difficult part would be to find the code where the 'erasure' from a.type to Int happens: the code is displaying Int =:= Int, technically this is incorrect, as =:= is not covariant. I prefer to also get rid of it

The equally difficult part would be adding the parameter to display inheritance & ARG tree for All kinds of Formatted, not just UniqueSingleType. Should it be considered later for being too complex ?

BTW, this tree can be quite long and complex, but does covers all the edge cases when an implicit search fails. E.g. for a record type in shapeless:

    val book =
      ("author" ->> "Benjamin Pierce") ::
        ("price" ->> 44.11) ::
        HNil

It's tree is:

-+ String with shapeless.labelled.KeyTag[String("author"),String] :: Double with shapeless.labelled.KeyTag[String("price"),Double] :: shapeless.HNil
 :       `-+ [ 2 ARGS ] :
 :         !-+ String with shapeless.labelled.KeyTag[String("author"),String]
 :         : !-+ shapeless.labelled.KeyTag[String("author"),String]
 :         : : :       `-+ [ 2 ARGS ] :
 :         : : :         !-+ String("author")
 :         : : :         : !-- String .................................................................................................................. [0]
 :         : : :         !-- String .................................................................................................................. [0]
 :         : : !-- Object .................................................................................................................. [1]
 :         : !-+ String .................................................................................................................. [0]
 :         :   !-- CharSequence
 :         :   !-- Comparable[String]
 :         :   !-- java.io.Serializable .................................................................................................... [2]
 :         !-+ Double with shapeless.labelled.KeyTag[String("price"),Double] :: shapeless.HNil
 :           :       `-+ [ 2 ARGS ] :
 :           :         !-+ Double with shapeless.labelled.KeyTag[String("price"),Double]
 :           :         : !-+ shapeless.labelled.KeyTag[String("price"),Double]
 :           :         : : :       `-+ [ 2 ARGS ] :
 :           :         : : :         !-+ String("price")
 :           :         : : :         : !-- String .................................................................................................................. [0]
 :           :         : : :         !-- Double .................................................................................................................. [3]
 :           :         : : !-- Object .................................................................................................................. [1]
 :           :         : !-+ Double .................................................................................................................. [3]
 :           :         :   !-- AnyVal
 :           :         !-+ shapeless.HNil
 :           :           !-- shapeless.HList ......................................................................................................... [4]
 :           !-- shapeless.HList ......................................................................................................... [4]
 !-+ shapeless.HList ......................................................................................................... [4]
   !-+ java.io.Serializable .................................................................................................... [2]
   : !-- Any
   !-+ Product
   : !-- Equals
   !-- Object .................................................................................................................. [1]
tek commented 3 years ago

The difficult part would be to find the code where the 'erasure' from a.type to Int happens: the code is displaying Int =:= Int, technically this is incorrect, as =:= is not covariant. I prefer to also get rid of it

Not quite sure I know what the proper behaviour here would be – is Int =:= Int only an artifact of splain or is it also present in the regular error message?

The equally difficult part would be adding the parameter to display inheritance & ARG tree for All kinds of Formatted, not just UniqueSingleType. Should it be considered later for being too complex ?

What do you mean by "adding the parameter"? Do you want to have an option that toggles displaying the trees for all types? Would you then still want to treat singleton types separately?

tribbloid commented 3 years ago

@tek I'm quite sure Int =:= Int is only an artifact of splain. If I disable it in compiler plugin the only message being displayed is Cannot prove that a.type =:= b.type, displaying a.type =:= b.type should be the correct way

What do you mean by "adding the parameter"? Do you want to have an option that toggles displaying the trees for all types? Would you then still want to treat singleton types separately?

Yes That's my intention, otherwise the fix for the singleton types will make the Int type invisible in the error message. The added configuration option to display the entire tree will make it obvious that a.type <:< Int

tek commented 3 years ago

Wouldn't the tree algorithm work entirely on the root type? You could just check the option in formatSpecial and return a custom Formatted with the tree if it was enabled.

tribbloid commented 3 years ago

@tek sounds good, already implemented. How about my first question? Namely, how to avoid a.type being erased into Int?

Also I'm trying to run the test in my IDE (IntelliJ IDEA, not through sbt), I always got the following error:

bad option: -P:splain:color:false
bad option: -P:splain:bounds
bad option: -P:splain:tree:false

Any idea what could be the cause?

tek commented 3 years ago

regarding the erasure, I would start by debug printing the type in a function like symbolPath in Formatting and then going up or down the call stack depending on what it shows to find the rule that chooses a supertype. There are a lot of gaps in splain's coverage of the type system :slightly_smiling_face:

as for these errors: the tests use a reflect.ToolBox to load the plugin jar built by the main project and passes those options to it, then compiles the code at runtime. I would assume that IntelliJ somehow interfers with something in there, like via the classloader…but I have no idea how, never used IntelliJ.

tribbloid commented 3 years ago

Thanks a lot, made it temporarily working in IDEA by the following monkey patch:

  def base = Option(System.getProperty("splain.tests"))
      .getOrElse(userDir + "/" + "tests")

  val plugin = Option(System.getProperty("splain.jar")).getOrElse {
    userDir + "/target/scala-2.13/splain_2.13.4-0.5.9-SNAPSHOT.jar"
  }

Need to synchronise version with build file or discover the jar name automatically tho

tek commented 3 years ago

oh, huh. I would assume that the cause of this could be that the tests are executed in a different sbt scope by IntelliJ. If you can find out how it works please advise!

tribbloid commented 3 years ago

OK fixed without tree-visualizing capability

https://github.com/tek/splain/pull/46

I saw a lot of idiosyncratic black magic for type symbol manipulation. I'm afraid some of them needs a more generalisable overhaul. Some of them are quite far from scala convention. The simpliest & safest way to avoid erasing information is to display 2 Strings: 1 from scala team (tpe.toString directly) & 1 from you

tribbloid commented 3 years ago

I have to add some type annotation to avoid the 2.13 compiler from inferring singleton type automatically

Regardless, the only significant change I made is to use type.toString and type.prefixString directly, instead of type.typeSymbol and type.termSymbol. These implementations are incrementally & carelessly patched by a series of PhD students, I don't have a lot of faith in them

tek commented 3 years ago

yes, I made it up as I went along :slightly_smiling_face: I see you found a location where the singleton type was truncated, that's a little more obvious!

since the tests succeed, do you want it merged now or do you want to supply part 2 first?

tribbloid commented 3 years ago

If you think all the changes make sense then let's merge it, always easier to have many decoupled commits instead of a big one.

I'll need some time to figure out how to embed a tree with interlinked references (those ............ [1] notations) into a type display

tek commented 3 years ago

sounds good!