Overriding of `IterableOps.toIterable` is deprecated, but it is an abstract method

[noresttherein]

toIterable is implemented canonically in Iterable, not in IterableOps. This means that it is impossible to implement without triggering a warning a class like:

class ArrayAsSeq[E](override val coll :Array[E]) extends mutable.IndexedSeqOps[E, Array, Array[E]]

Classes like that are useful particularly in conjunction with extension methods working for various Ops types. Note that extending Iterable in the example above is impossible, because Iterable defines

  final override def coll :this.type = this

And ArrayAsSeq must return an Array[E], rather than this. Possibly there are more conflicts in this particular case, but this one will exist for any such 'ops view'. This particular problem may also be solved with making coll not final in Iterable, but @deprecatedOverriding on an abstract method will always be limiting, and unless there is a good reason to essentially require

trait IndexedSeqOps[E, CC[_], C] { this :IndexedSeq[E] => }

it would perhaps be better to move deprecation to Iterable.

[SethTisue]

@scala/collections

[Ichoran]

I don't understand the complaint.

ArrayAsSeq as written works fine as an IndexedSeqOps. You get the operations as requested. Many of them can't be chained because you don't produce an ArrayAsSeq but rather an Array...but...that's what you asked for!

If you actually want an Iterable backed by an array, you simply don't call the backing array coll, and you're all good there, too. (See collection.mutable.ArraySeq for an example.)

The IndexedSeqOps implementation is hard to manage as written in the example, but that's not because of the return signature of coll; rather, it's because there's no ClassTag and thus no way to generate new arrays of the correct type.

[som-snytt]

All the new recruits to the collections collective just ran away.

[He-Pin]

I think it would be better to remove the @deprecated in IterableOps and only keep the Iterable. But there is already an ArraySeq exist , so what's the reason you need another extension?

And the toIterable seems will be removed in the future from the PublicApi

For the warning, you can and @nowarn("cat=deprecation")

And you may want to use IterableOnce

I think your example will not work, when you return the Array[T] instead of this type, then the type information is lost. so I still not get why. Maybe you just need extends AnyVal to add some operators there?

@noresttherein Can you provide an example of what you want to do, there maybe other ways to achieve that.

[jxnu-liguobin]

ArrayAsSeq looks the same as ArraySeq, I would like to know why we need to implement our own ArraySeq, what did I miss?

For the warning, you can and @nowarn("cat=deprecation")

To ensure that a @nowarn annotation actually suppresses a warning, enable -Xlint:unused or -Wunused:nowarn.

[yzia2000]

Keeping the need for ArrayAsSeq aside.

See this in Iterable.scala

trait IterableOps[+A, +CC[_], +C] extends Any with IterableOnce[A] with IterableOnceOps[A, CC, C] {
  /**
    * @return This collection as an `Iterable[A]`. No new collection will be built if `this` is already an `Iterable[A]`.
    */
  // Should be `protected def asIterable`, or maybe removed altogether if it's not needed
  @deprecated("toIterable is internal and will be made protected; its name is similar to `toList` or `toSeq`, but it doesn't copy non-immutable collections", "2.13.7")
  def toIterable: Iterable[A]

Based on that comment it seems that toIterable may be removed or changed to protected in IterableOps itself so removing the deprecation flag seems like a bad idea - unless that's not the case.

If you want to extend IterableOps itself, does the linter still throw warnings if you do this:

class ArrayAsSeq[E](override val coll :Array[E]) extends mutable.IndexedSeqOps[E, Array, Array[E]] {

  @deprecated("toIterable is internal and will be made protected; its name is similar to `toList` or `toSeq`, but it doesn't copy non-immutable collections", "2.13.7")
  final def toIterable: this.type = ??? // whatever your implementation may be here
}

So putting deprecation in both places? Or do I not understand the issue.

[noresttherein]

First: ArrayAsSeq is here primarily as an ilustration. I do not propose introducing it to the library :) The point of the issue is that it is impossible to extend IterableOps without extending Iterable, or overriding/implementing deprecated API. So, if deprecation forbodes a future removal, then this is a problem. I think that eliminating the possibility of extending IterableLike without Iterable is an unnecessary limitation which prevents some valid use. As another example, I have a Ranking class which is a sequence of unique elements, but not a Seq (appending/prepending behaves differently to retain uniqueness). It is sometimes convenient - again, typically in the context of extension methods - to however treat it as a Seq, hence the need to implement IndexedSeqOps without implementing Seq.

And second, the difference from ArrayOps is that it is an IterableOnce with IterableOnceOps, and thus gains all interoperability of such, but at the same time, unlike ArraySeq, it produces again an array. The motivation is to use it with extension methods (especially using Scala 2 style extension classes):

implicit class IterableOnceExtension[E, CC[_], C](val self :IterableOnceOps[E, CC, C]) {
  //extension methods
}
implicit def ArrayAsSeqExtension[E](a :Array[E])  = new IterableOnceExtension(new ArrayAsSeq(a))

I don't know what is supposed not to work - it's a simplification of a class I've been using for some time now. I hope I made things a bit more clear.

I think that the best path to the removal of toIterable, if it is what you want, would be to first implement it in IterableOps, so custom collections like in the example can actually remove their implementation of the method, and only some time after remove the method completely.

[som-snytt]

The deprecating PR says:

The method is a noop on any Iterable, it only does something for an object that is an IterableOps but not an Iterable. This should never be the case in user code.

I don't understand what is at issue here, but if this is a correct summary:

The point of the issue is that it is impossible to extend IterableOps without extending Iterable, or overriding/implementing deprecated API.

then I think there's nothing wrong with deprecating a deferred definition. One deprecates API, not implementation.

Any progress on this ticket would need a concrete example and not a description of an example.

Note that implementing IterableOps is not impossible, and implementing a deprecated definition doesn't even warn, to answer a previous question.

Also worth adding that deprecation means "don't use me", not necessarily that it will be removed.