Skip to content

refactor: replace immutable.Traversable in the public io and stream APIs - #3468

Open
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:public-traversable-to-iterable
Open

refactor: replace immutable.Traversable in the public io and stream APIs#3468
pjfanning wants to merge 1 commit into
apache:mainfrom
pjfanning:public-traversable-to-iterable

Conversation

@pjfanning

Copy link
Copy Markdown
Member

Motivation

immutable.Traversable is a deprecated alias for immutable.Iterable, kept around for the 2.12 migration. It survives in the options parameter of several public messages and methods:

site
Tcp.scala:138,167 Tcp.Connect, Tcp.Bind
Udp.scala:118,139 Udp.Bind, Udp.SimpleSender
UdpConnected.scala:112 UdpConnected.Connect
stream/scaladsl/Tcp.scala:144,186,220 bind, bindWithTls, outgoingConnection
TcpConnection.scala:226, TcpIncomingConnection.scala:37 private[io], but share the signature

On the MiMa exclusions

You asked for exclusions with this. None are needed, and I would rather not add empty ones.

I had previously told you this change would move the Scala pickle signature and need filtering. That was wrong, and checking it is what changed the shape of this PR. immutable.Traversable is a type alias, so it is already erased and written as immutable.Iterable in the bytecode. javap on the pre-change build:

public scala.collection.immutable.Iterable<...Inet$SocketOption> options();
  descriptor: ()Lscala/collection/immutable/Iterable;
public org.apache.pekko.io.Tcp$Connect copy(java.net.InetSocketAddress, scala.Option<...>,
    scala.collection.immutable.Iterable<...Inet$SocketOption>, ...)

The generic signature already said Iterable, so copy and unapply are unchanged too — the case-class concern I raised earlier does not apply. actor/mimaReportBinaryIssues and stream/mimaReportBinaryIssues both pass unmodified against the pekko-actor:1.0.0 baseline that MiMa resolves.

If a reviewer would still like a .excludes file added defensively, say so and I will add one — but it would exclude nothing, and I did not want to imply a compatibility risk that the bytecode says is not there.

Also removed: eight stale suppressions

Five @nowarn("msg=deprecated") in the actor io sources and three @nowarn // Traversable deprecated in 2.13 in stream's Tcp. I verified each was stale by deleting it and recompiling.

This is the part with real value. @nowarn("msg=deprecated") on a class or method silences every deprecation warning in its scope, not just the one it was added for — on Udp.Bind and Tcp.Connect it sits on the whole case class. Left in, they would keep hiding unrelated deprecations indefinitely. The nowarn import is kept in stream's Tcp only because line 402 still uses it for something else.

Compatibility

  • Binary: unchanged, as above.
  • Source: unaffected for callers passing Seq, List, Nil or immutable.Iterable. A caller that has explicitly spelled immutable.Traversable at a call site still compiles, since the alias itself is not being removed.

Tests

No new tests — this is a type-alias substitution with no behaviour change and no signature change.

actor/compile, stream/compile, actor/mimaReportBinaryIssues and stream/mimaReportBinaryIssues all pass locally, and scalafmt has been run on both modules. I have not run the full test suites locally and am relying on CI for those.

Scope

Deliberately limited to immutable.Traversable. The GenTraversableOnce use in DnsMessage is in #3467 and is not touched here.

`immutable.Traversable` is a deprecated alias for `immutable.Iterable`,
retained for the 2.12 migration. It remains in the `options` parameter of
several public messages and methods:

  - Tcp.Connect, Tcp.Bind
  - Udp.Bind, Udp.SimpleSender
  - UdpConnected.Connect
  - stream's Tcp.bind, bindWithTls and outgoingConnection
  - TcpConnection.completeConnect and the TcpIncomingConnection
    constructor, which are private[io] but share the same signature

Use `immutable.Iterable` and drop the now stale suppressions: five
`@nowarn("msg=deprecated")` in the actor io sources and three
`@nowarn // Traversable deprecated in 2.13` in stream's Tcp. Each was
verified stale by removing it and recompiling. This matters beyond
tidiness, because `@nowarn("msg=deprecated")` on a class or method
silences every deprecation warning in its scope.

No MiMa exclusions are needed. `immutable.Traversable` is a type alias,
so it is already both erased and written as `immutable.Iterable` in the
bytecode and in the generic signature; `javap` on Tcp$Connect shows
`options()` returning `scala.collection.immutable.Iterable` before this
change. `actor/mimaReportBinaryIssues` and `stream/mimaReportBinaryIssues`
pass unchanged against the 1.0.0 baseline.

Source compatibility is unaffected for callers passing a `Seq`, `List`,
`Nil` or `immutable.Iterable`. A caller that has explicitly written
`immutable.Traversable` at a call site still compiles, since the alias
itself is not removed.
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.

1 participant