8000 SI-4700 Add `@infix` annotation for type printing by VladUreche · Pull Request #5401 · scala/scala · GitHub
[go: up one dir, main page]

Skip to content

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

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

Closed
wants to merge 1 8000 commit into from

Conversation

VladUreche
Copy link
Contributor
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

Thanks to @lrytz for help!

@VladUreche
Copy link
Contributor Author

If all tests pass, review by @adriaanm and/or @retronym.

@@ -1405,6 +1405,8 @@ trait Definitions extends api.StandardDefinitions {
case _ => false
}

lazy val InfixAnnotationClass = rootMirror.getRequiredClass("scala.annotation.infix")
Copy link
Member

Choose a reason for hiding this comment

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

Prefer getClassIfDefined to allow the compiler to be able to run with older libraries

@soc
Copy link
Contributor
soc commented Sep 15, 2016
  • Wouldn't I be possible to come up with a more general rule, like printing symbolic names infix by default?
  • If we go the annotation route, @infix feels very general in the sense that there could be many different things such an annpttion could do ... Is there a more specific name for this feature?

8000

@lrytz
Copy link
Member
lrytz commented Sep 16, 2016

Wouldn't I be possible to come up with a more general rule, like printing symbolic names infix by default?

This wouldn't work for scalactic's Int Or ErrorMessage

If we go the annotation route, @infix feels very general in the sense that there could be many different things such an annpttion could do ... Is there a more specific name for this feature?

Agreed

@VladUreche
Copy link
Contributor Author
VladUreche commented Sep 19, 2016

@lrytz @soc You're right, the name is too general. How about @printAsInfix?

I would like to steer clear of:

  • Coming up with heuristics for what to print as infix (even the best heuristics won't be good enough)
  • Having a more general formatter (in theory, we could let programmers write @PrintAs("$1 WTF!?! $2") class AND[T, U], but that won't parse back an will mislead users even more)

@milessabin
Copy link
Contributor

How about showAsInfix?

Apologies for the bikeshedding ;-)

@VladUreche
8000 Copy link
Contributor Author

Sure, we can call it showAsInfix. Everyone else happy with this name?

@SethTisue
Copy link
Member

tbh I don't see any problem with the name @infix

@Ichoran
Copy link
Contributor
Ichoran commented Sep 19, 2016

I also don't see a problem with @infix. It could possibly mean other things, but when would those other things not apply if and only if you wanted to print infix? If we have actual examples where the two would diverge, fine. Otherwise, if the places to apply them are apparently the same, I'd just leave it.

```
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
```
@VladUreche VladUreche force-pushed the issue/SI-4700-infix-print branch from 11c9c2f to e60768b Compare September 19, 2016 18:07
@VladUreche
Copy link
Contributor Author
VladUreche commented Sep 19, 2016

I just pushed a version that updates the name to @showAsInfix. Let's see if all tests pass, and then we can take care of the name...

Also +1 for @showAsInfix since it doesn't clutter the namespace. @xeno-by wdyt?

@soc
Copy link
Contributor
soc commented Sep 26, 2016

@VladUreche I can see your concern about heuristics, but I think in this case it is simply convention-over-configuration. I think this doesn't deserve yet another tuning knob, of which Scala has too many already.

With a simple and clear convention, people who want infix printing know what they need to do.

A convention also makes it much easier for readers to see what's happening.

@nafg
Copy link
Contributor
nafg commented Sep 28, 2016

Are symbolic binary type constructors ever not infix?

@nafg
Copy link
Contributor
nafg commented Sep 28, 2016

BTW I didn't see anyone mention but I guess this somewhat relates to the proposal for an @infix annotation for symbolic methods (operators) naming the alphanumeric method it aliases

@SethTisue
Copy link
Member

Wouldn't I be possible to come up with a more general rule, like printing symbolic names infix by default

Vlad, you said you were against this, but didn't say why? It seems appealing to me (and others on this thread), but maybe I just need to hear your reasons.

@nafg
Copy link
Contributor
nafg commented Sep 28, 2016

@SethTisue @lrytz did mention an instance of a non-symbolic infix type constructor... But I don't see the compiler printing Or[Int, ErrorMessage] as a problem so pressing we need a new annotation

@paulp
Copy link
Contributor
paulp commented Sep 30, 2016

You should default symbolic names to infix, non-symbolic to non-infix, and have both @infix and @noinfix annotations. This gets 99.9% of cases right without intervention and lets people adjust the 1/1000. Anything else creates lots of busywork and inconsistency.

@soc
Copy link
Contributor
soc commented Oct 1, 2016

@paulp I'm with Paul on having sane defaults. I disagree on adding annotations. I don't want any annotations to configure more stuff. The cost of adding another two tuning knobs is not worth the benefits of getting the last 0.1%.

@adriaanm
Copy link
Contributor
adriaanm commented Oct 18, 2016

You should default symbolic names to infix, non-symbolic to non-infix

I agree with @paulp on defaults for symbolic names. I imagine this is compatible with most people's intuition. We should still have a mode (compiler option) that prints types in full using a standard format, to make it easier to google them. Once we have an @friendlyName for symbolic names, it should also replace symbolic names by their readable counterparts, so that you get canonical, googlable names consistently.

I could be convinced about adding the @infix and @noinfix annotations, but I'm a bit worried it would complicate more than it solves. We're recommending to add punctuation to things like xs filter f map g at the value level these days; so, is that something we want to encourage at the type level?

@paulp
Copy link
Contributor
paulp commented Oct 18, 2016

For the record I don't care about the annotations, I only care about the defaults.

The question of how to group types is interesting though.

trait ~>[F[_], G[_]] 
trait ->[+A, +B]
trait F[A]
trait G[A]

scala> def f(x: F ~> G => F ~> G -> Int): Unit = ()
f: (x: ~>[F,G] => ->[~>[F,G],Int])Unit

Personally I think the best way to write that type is the way I wrote it: F ~> G => F ~> G -> Int. If you don't know the precedence rules, you will not like that type. Yet always burying such types in parentheses is highly undesirable for someone who does know the precedence rules.

It's a given that if there can only be one way to print a type then there will be huge constituencies who lose. As configurable type printing doesn't appear to be on the horizon, all we can do is lobby for the merits of pleasing certain constituencies. As a member of the "deal with type errors all day long" constituency, anything which lowers the noise is a godsend.

@adriaanm
Copy link
Contributor

Thanks for pointing out the precedence issue. I am completely on board with improving type printing this way (print symbolic types infix, minimize parens using precedence) -- the function arrow itself sets the precedent.

I'd also love to see a design for an error message/reporting configuration mechanism to improve the (new) user experience. That would be something to flesh out in a doc before prototyping it, though. (I've already done some of the refactorings to make reporting easier to tweak.)

@VladUreche
Copy link
Contributor Author

Sounds good, but right now I'm flooded with work, so it may take a while to update the PR. If anyone has more time on their hands, please go ahead and submit an updated version.

@lrytz
Copy link
Member
lrytz commented Dec 9, 2016

Closing in favor of #5589, thanks a lot for pushing this forward, @VladUreche!

@lrytz lrytz closed this Dec 9, 2016
@SethTisue SethTisue removed this from the 2.12.2 milestone Feb 22, 2017
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.

0