Fix #6190: eta-expand companion object if functions are expected

[liufengyun]

Fix #6190: eta-expand companion object if functions are expected

As suggested by @odersky in #6190, we stop generating function types as parents for companion objects of case class. To compensate, we insert C.apply and eta-expand if the expected type is a function type.

Clarification:

In the following code, .tupled cannot be omitted --- auto tupling does not work here.

case class Foo(key: Int, value: String)

val a = Map(1 -> "a", 2 -> "b").map(Foo.apply.tupled)

[sjrd]

I believe this is good migration story for dropping the automatic function type parent.

However, I'd like this to only be a migration story, and not encourage new code to use that "feature" for any kind of object with an apply method. So I would suggest that the compiler emits a warning when it eta-expands an object.

The reason is that

• Either we also do this for any other term, not just objects, and it will be very confusing because that would essentially become a silent implicit conversion that we cannot get rid of, and that is not even type-based!
• Or we have a weird inconsistency between terms that happen to refer to objects and ones that do not. Which is not much better.

So I would recommend: do it only for objects, and warn when you do it that it is just a migration thing, and that they should either eta-expand themselves or write .apply themselves.

[liufengyun]

@sjrd The insertion only happens for the companion object of a case class. And a warning is issued as suggested.

[odersky]

While we are at it, should we drop the auto-generated apply method as well? It's no longer needed since we can now construct instances without new anyway. This step would remove some major complication about suppressing auto-generated apply methods that clash with user-defined ones.

[liufengyun]

While we are at it, should we drop the auto-generated apply method as well?

It's a nice idea. The only concern is migration. From the experience with community build, there is a significant amount of code using companion objects as function values for map. If we can compensate for that, then migration is not a problem.

[odersky]

Looking at the extensive changes this needs in the community build I am a bit nervous to do this now. The breakage might be too extensive, in particular in connection with tupled. Is there a way to apply automatic source code rewriting to mitigate this?

[liufengyun]

I think rewriting is possible to be implemented in scalafix.

Meanwhile, I think it would be better to postpone changes in this PR until 3.1, so that it will not create more hassles in the migration.

[odersky]

Put on hold until after 3.0.

[odersky]

Closing for now; to be revived when we get started on 3.1

[odersky]

I am re-opening the PR since it affects the binary API. So according to our versioning policy we won't be able to do it in a minor version post 3.0.

A question: Are applys inserted only for eta expanding case class companions or for other objects as well? I think it should be only case class conpanions.

I also agree with @sjrd that ideally we should warn when inserting an apply method for eta expansion.

So, @liufengyun Can you find out how many manual fixes we need in the current community build, and how many new eta expansion warnings we get? That should give us a better basis to decide what to do.

[liufengyun]

I am re-opening the PR since it affects the binary API. So according to our versioning policy we won't be able to do it in a minor version post 3.0.

A question: Are applys inserted only for eta expanding case class companions or for other objects as well? I think it should be only case class conpanions.

The implementation follows this rule.

I also agree with @sjrd that ideally we should warn when inserting an apply method for eta expansion.

The implementation follows this rule.

So, @liufengyun Can you find out how many manual fixes we need in the current community build, and how many new eta expansion warnings we get? That should give us a better basis to decide what to do.

Here are the number of warnings by community projects:

|---------------+-----|
| scalap        |  39 |
|---------------+-----|
| ScalaPB       | 144 |
|---------------+-----|
| cats-effect-3 |   1 |
|---------------+-----|
| cats          |   3 |
|---------------+-----|
| TOTAL         | 187 |
|---------------+-----|

• In scalap, nearly all warnings come from the usage in parser combinators, for example:

  lazy val typeEntry: EntryParser[Type] = oneOf(
      11 -^ NoType,
      12 -^ NoPrefixType,
      13 -~ symbolRef ^^ ThisType,
      14 -~ typeRef ~ symbolRef ^~^ SingleType,
      ...

• In ScalaPB, nearly all warnings come from the usage in map, for example:

case 30 => noBox.map(_root_.scalapb.descriptors.PBoolean).getOrElse(_root_.scalapb.descriptors.PEmpty)

[odersky]

@liufengyun One more question: If we want to avoid the warnings in scalap and scalaPB by adding companion objects that explicitly extend FunctionN, how many companion objects need to be added or modified?

[odersky]

LGTM

[liufengyun]

@liufengyun One more question: If we want to avoid the warnings in scalap and scalaPB by adding companion objects that explicitly extend FunctionN, how many companion objects need to be added or modified?

Hi @odersky , this is the data:

|----------+----------+---------|
|          | warnings | changes |
|----------+----------+---------|
| scalap   |       39 |      38 |
|----------+----------+---------|
| ScalaPBB |      144 |      13 |
|----------+----------+---------|

For ScalaPB, the function parent has to be made explicit for the following companion objects (13 in total):

  _root_.scalapb.descriptors.PBoolean
  _root_.scalapb.descriptors.PByteString
  _root_.scalapb.descriptors.PDouble
  _root_.scalapb.descriptors.PEnum
  _root_.scalapb.descriptors.PInt
  _root_.scalapb.descriptors.PLong
  _root_.scalapb.descriptors.PRepeated
  _root_.scalapb.descriptors.PString
  com.google.protobuf.struct.Value.Kind.BoolValue
  com.google.protobuf.struct.Value.Kind.ListValue
  com.google.protobuf.struct.Value.Kind.NumberValue
  com.google.protobuf.struct.Value.Kind.StringValue
  com.google.protobuf.struct.Value.Kind.StructValue

For scalap, the function parent has to be made explicit for the following companion objects (38 in total):

  AliasSymbol
  AnnotInfo
  AnnotatedType
  AnnotatedWithSelfType
  Annotation
  AnnotationElement
  ArrayValue
  Attribute
  AttributeInfo
  Children
  ClassFile
  ClassFileHeader
  ClassInfoIndex
  ClassInfoType
  ClassSymbol
  ConstValueIndex
  ConstantPool
  ConstantType
  EnumConstValue
  ExistentialType
  ExternalSymbol
  Field
  Method
  MethodSymbol
  MethodType
  MethodType
  NullaryMethodType
  ObjectSymbol
  PolyType
  RefinedType
  ScalaSig
  SingleType
  SourceFileInfo
  SymbolInfo
  ThisType
  TypeBoundsType
  TypeRefType
  TypeSymbol