He-Pin commented on code in PR #3720:
URL: https://github.com/apache/pekko/pull/3720#discussion_r4396564843
##########
actor-testkit-typed/src/main/scala/org/apache/pekko/actor/testkit/typed/javadsl/LoggingTestKit.scala:
##########
@@ -94,7 +94,7 @@ import org.slf4j.event.Level
/**
* Matching events for which the supplied function returns `true`.
*/
- def withCustom(newCustom: Function[LoggingEvent, Boolean]): LoggingTestKit
+ def withCustom(newCustom: Function[? >: LoggingEvent, Boolean]):
LoggingTestKit
Review Comment:
`Function` here is `scala.Function1`, not a `japi` one (hence the
`scala.Function1` MiMa entries). `Function1[-T1, +R]` is already contravariant,
so `? >: LoggingEvent` accepts exactly the same values from Scala, but it
breaks Scala 2.13 lambda inference:
```
scala> object Api { def withCustom(f: Function[? >: String, Boolean]) = () }
scala> Api.withCustom(s => s.length > 2)
error: missing parameter type
```
(reproduced with scalac 2.13.18; Scala 3.3.8 accepts it). So
`LoggingTestKit.custom(ev => ...)` stops compiling for Scala callers, and CI
cannot catch it because every Scala caller in the repo imports the scaladsl
kit. On the Java side the only gain is passing a pre-typed `scala.Function1`
value, which is close to zero.
Suggest keeping `withCustom` and `custom` (line 208) invariant and dropping
their two excludes.
##########
cluster-sharding-typed/src/main/scala/org/apache/pekko/cluster/sharding/typed/javadsl/ShardedDaemonProcess.scala:
##########
@@ -115,7 +115,8 @@ abstract class ShardedDaemonProcess {
messageClass: Class[T],
name: String,
initialNumberOfInstances: Int,
- behaviorFactory: JFunction[ShardedDaemonProcessContext, Behavior[T]]):
ActorRef[ShardedDaemonProcessCommand]
+ behaviorFactory: JFunction[? >: ShardedDaemonProcessContext, ? <:
Behavior[T]])
Review Comment:
The four `init` overloads above (lines 61, 74, 87, 102) still take an
invariant `IntFunction[Behavior[T]]` while these `initWithContext` overloads
were widened. `IntFunction[? <: Behavior[T]]` would keep the class consistent
with the sibling-operator rule from #3712.
##########
cluster-typed/src/main/scala/org/apache/pekko/cluster/ddata/typed/javadsl/ReplicatorMessageAdapter.scala:
##########
@@ -75,7 +75,8 @@ class ReplicatorMessageAdapter[A, B <: ReplicatedData](
* the given `responseAdapter` function.
*/
def subscribe(
Review Comment:
`ReplicatorMessageAdapter` (line 63) is a public non-final class with a
public constructor and no `@DoNotInherit` / "Not for user extension" doc,
unlike every other type touched here (`javadsl.Source`, `javadsl.Flow`,
`BehaviorBuilder`, `TestProbe`, `ShardedDaemonProcess`, the `EffectBuilder`s
are all `final` or `@DoNotInherit`). A Java subclass that declares `subscribe`
/ `askUpdate` / `askGet` / `askDelete` / `askReplicaCount` with today's
signatures now fails to compile with "name clash: both methods have same
erasure, yet neither overrides the other".
Either leave this class out, or call it out in the PR description as an
accepted source break rather than claiming only final/object/`@DoNotInherit`
types changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
