SI-4700 Types with symbolic names print in infix by default #5589

ghost · 2016-12-08T02:54:33Z

before:

scala> implicitly[Int =:= Int]
res0: =:=[Int,Int] = <function1>

after:

scala> implicitly[Int =:= Int]
res0: Int =:= Int = <function1>

Building off of @VladUreche's work in #5401, here's a PR that addresses some of the comments there.

makes two-parameter types with symbolic names print infixly by default, and
adds a @showAsInfix annotation to enable/disable override that for types.

Review by @lrytz, @retronym, and others?

--allison

@infix

``` scala> import scala.annotation.infix import scala.annotation.infix scala> @infix class &&[T, U] defined class $amp$amp scala> def foo: Int && Boolean = ??? foo: Int && Boolean ```

lrytz · 2016-12-09T12:06:59Z

Thanks @allisonhb for picking this up, it looks great in the current form! CC @adriaanm

However, it adds a public class to the standard library, which is not forwards binary compatible. @SethTisue do you know why this is not flagged by MiMa? Could we ignore this case?

retronym · 2016-12-09T12:29:41Z

I we fixed a MiMa bug recently in our build. If this PR were rebased on 2.12.x, the compat violation ought to be reported.

I'm not totally against the idea of allowing annotations in a minor release. Need to give it some thought.

Last time we considered this was with @compileTimeOnly, which we decided had to wait until 2.11.0 to move from reflect.internal to annotation.

ghost · 2016-12-09T13:30:59Z

So for what little it's worth, I can't think of any places where someone would rather keep the prefix notation for symbolic type names, and only a few (refined comes to mind) where infix notation is preferred for non-symbolic names.

On the other hand, I know that at least in my workplace, types such as shapeless.::[Int, shapeless.::[(Long, Long), shapeless.::[String, shapeless.HNil]]] are a major cause for complaint. If you like I can rebase this onto 2.12.x, drop the annotation, and reinstate it in a 2.13.x PR.

--allison

edit: woops accidentally pushed "close + commit" button...

milessabin · 2016-12-09T22:38:47Z

@allisonhb I'd be very keen to merge this in Typelevel Scala for the upcoming 2.12.1 release ... please let me know if you'd be happy with that.

ghost · 2016-12-10T20:41:08Z

@milessabin that sounds great! I'll make a PR against TLS as soon as I get home.

milessabin · 2016-12-10T21:30:30Z

@allisonhb no need ... I can merge this PR to TLS as it stands. Thanks for working on this!

adriaanm · 2016-12-12T20:05:34Z

src/reflect/scala/reflect/internal/Types.scala

+    /* It only makes sense to show 2-ary type constructors infix.
+     * By default we do only if it's a symbolic name. */
+    override def isShowAsInfixType: Boolean =
+      args.size == 2 &&


It's best to avoid calling size on a list. We have a more performant convenience method for this: hasLength(args, 2)

Add ability to disable this via the @showAsInfix annotation.

lrytz · 2016-12-20T12:44:53Z

I did some testing what will happen if a class annotated @showAsInfix is used (on the classpath) by an older compiler, where the annotation doesn't exist.

scala> cls.annotations
res15: List[$r.intp.global.AnnotationInfo] = List(showAsInfix(showAsInfix.<init>$default$1))

scala> cls.annotations.head.atp.typeSymbol
res16: $r.intp.global.Symbol = class showAsInfix

scala> cls.annotations.head.atp.typeSymbol.getClass
res17: Class[_ <: $r.intp.global.Symbol] = class scala.reflect.internal.Symbols$StubClassSymbol

so this seems to be safe.

lrytz · 2016-12-20T12:50:43Z

LGTM, veto by @retronym?

SethTisue · 2017-02-10T23:57:42Z

Not sure about the details of the code, but I definitely support this design change.

milessabin · 2017-02-13T08:35:29Z

FYI: this has been merged in Typelevel Scala 2.12.1.

adriaanm · 2017-02-16T20:26:48Z

Based on Lukas's testing, I think this is ready to go.

little missing piece of scala#5589; not sure why it didn't fail PR validation

little missing piece of scala#5589

fommil · 2017-04-19T17:01:12Z

we get failures in the ensime-server tests that I think are because of this... looks like our inferred scaladoc URLs are changing, e.g. having to add to our CI

        equal(Some("docs/scala-library-" + c.scalaVersion + "-javadoc.jar/scala/Some.html#flatten[B](implicitev:<:<[A,Option[B]]):Option[B]")) or
        equal(Some("docs/scala-library-" + c.scalaVersion + "-javadoc.jar/scala/Some.html#flatten[B](implicitev:A<:<Option[B]):Option[B]"))

fommil · 2017-04-19T17:03:58Z

looks like the tags of the actual scaladocs did change with this. Was that expected?

adriaanm · 2017-04-19T17:26:56Z

I hadn't expected that, no! Could you file a bug at scala/bug?

…

On Wed, Apr 19, 2017 at 12:04 Sam Halliday ***@***.***> wrote: looks like the tags of the actual scaladocs did change with this. Was that expected? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#5589 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAFjy-dvWwGUlUVCJjREm1jiE4wzOUGmks5rxj5_gaJpZM4LHYgz> .

fommil · 2017-04-19T17:28:34Z

@adriaanm I'm not sure if it is a bug, it's a regression if anything, but flip flopping on regressions is not great either

SethTisue · 2017-04-20T18:42:28Z

@allisonhb since this PR is linked directly from the 2.12.2 release notes, I've edited your PR description to include a "before/after" pair

This a partial port of scala/scala#5589 to dotty: when pretty-printing an applied type, if its type constructor has a symbolic name, then always print it in infix form. The PR in scalac also adds an `@showAsInfix` annotation to control this behavior, but we cannot do the same in dotty since we still rely on the standard library from Scala 2.11 and the annotation only exists in 2.12 and up.

eparejatobes · 2017-08-15T11:32:11Z

Would be really nice if types would display accordingly in the scaladoc output.

fommil · 2017-08-15T12:44:42Z

I'd settle for scalac providing utility methods that converted between all the different kinds of names that a thing can have. This is the bulk of our work interfacing with the interactive compiler in ensime.

SethTisue · 2018-02-14T03:24:30Z

Would be really nice if types would display accordingly in the scaladoc output

that would make a great scala/bug ticket (if there isn't one already?), and a nice manageably sized issue, I expect, for a volunteer contributor to tackle

was thinking of this just now because I'm watching a talk where the speaker is demoing HList from Shapeless, and those types sure do look nice in infix.

SI-4700 Add @infix annotation for type printing

e60768b

``` scala> import scala.annotation.infix import scala.annotation.infix scala> @infix class &&[T, U] defined class $amp$amp scala> def foo: Int && Boolean = ??? foo: Int && Boolean ```

scala-jenkins added this to the 2.12.2 milestone Dec 8, 2016

ghost changed the title ~~(SI-5401) The thrilling continuation to the infix type printing saga.~~ SI-4700 The thrilling continuation to the infix type printing saga. Dec 8, 2016

lrytz mentioned this pull request Dec 9, 2016

SI-4700 Add @infix annotation for type printing #5401

Closed

ghost closed this Dec 9, 2016

ghost reopened this Dec 9, 2016

adriaanm reviewed Dec 12, 2016

View reviewed changes

allisonhb added 2 commits December 13, 2016 20:29

SI-4700 Show infix types with as few parentheses as needed.

8badcad

SI-4700 Make infix notation default for symbolic types.

fab1db5

Add ability to disable this via the @showAsInfix annotation.

adriaanm assigned lrytz Dec 19, 2016

scala-jenkins added the reviewed label Dec 20, 2016

lrytz assigned retronym Dec 21, 2016

milessabin mentioned this pull request Jan 25, 2017

Prepare 2.12.1 release typelevel/scala#129

Closed

milessabin mentioned this pull request Feb 13, 2017

SI-4700 Add @infix annotation for type printing typelevel/scala#118

Closed

adriaanm merged commit e2be7c4 into scala:2.12.x Feb 16, 2017

SethTisue added a commit to SethTisue/scala that referenced this pull request Feb 17, 2017

update MiMa whitelist for infix annotation

f8a271b

little missing piece of scala#5589; not sure why it didn't fail PR validation

SethTisue mentioned this pull request Feb 17, 2017

update MiMa whitelist for infix annotation #5712

Merged

SethTisue added a commit to SethTisue/scala that referenced this pull request Feb 17, 2017

update MiMa whitelist for infix annotation

78eee86

little missing piece of scala#5589

SethTisue mentioned this pull request Feb 18, 2017

[ci: last-only] merge 2.12.x onto 2.13.x (Feb 17, 2017) #5718

Merged

SethTisue added the release-notes worth highlighting in next release notes label Feb 22, 2017

scabug mentioned this pull request Apr 7, 2017

print type with symbolic name and two arguments as infix scala/bug#4700

Closed

SethTisue changed the title ~~SI-4700 The thrilling continuation to the infix type printing saga.~~ SI-4700 Types with symbolic names print in infix by default Apr 14, 2017

smarter mentioned this pull request Jun 8, 2017

SI-4700 Types with symbolic names print in infix by default scala/scala3#2718

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SI-4700 Types with symbolic names print in infix by default #5589

SI-4700 Types with symbolic names print in infix by default #5589

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SI-4700 Types with symbolic names print in infix by default #5589

SI-4700 Types with symbolic names print in infix by default #5589

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!