Add scala.annotation.nowarn for cross-compiling

[lrytz]

No description provided.

[lrytz]

We could also add this to the 2.12.12 standard library, as Scala annotations don't break forwards binary compatibility (unless abused). But adding it here would make it available sooner.

[NthPortal]

The change looks good in principle, but I do wonder if it belongs in this library. the discussion in #219 never really reached a resolution

[ijuma]

This would be super helpful for the reasons mentioned in the class documentation. LGTM.

[dwijnand]

Just writing down my thoughts, I'm not sure what's best:

1. keep scala.annotation in scala-library.jar, don't split more packages across jars, also make @nowarn more accessible; or
2. keep the no-op stubs out of the main (scala/scala) codebase

🤔

Compat libraries split between those that provide old APIs on new implementations (backwards compatibility?), and those that provide new APIs on old implementations (forwards compatibility?). I get the feeling that the former is what historically "compatibility libraries" were, but I like the latter more, and it's what macro-compat, sbt-compat, and scala-collection-compat (this) do.

These new-API-on-old-impl compact libraries are good because by putting it in a separate library they add a little barrier which encourages migration of API (instead).

That doesn't make sense here. We don't want to discourage the use of @nowarn. So I think I'm favouring putting this in a 2.12 release instead.

[lrytz]

In the meantime, until the annotation finally ends up in 2.12.12, we could suggest cross-building projects to use version-specific source files and

• define a type alias in 2.13
• define their own nowarn annotation class in 2.11/2.12

The 2.13 version would be

scala> :pa -raw
// Entering paste mode (ctrl-D to finish)

package my { package object project { type nowarn = scala.annotation.nowarn } }

// Exiting paste mode, now interpreting.


scala> @my.project.nowarn def foo = 1 → 2
def foo: (Int, Int)

I think there's also a way to make it work with a wildcard import, maybe even without version-specific source files? @sjrd does these kind of tricks I believe.

[sjrd]

I do think @nowarn belongs in scala-collection-compat. I know it hasn't anything to do with collections, but TBH I wouldn't mind scala-collection-compat to be in fact compat for the entire std lib. Also putting it in scala-collection-compat means it's also available in 2.11, not just 2.12. A large number of libraries cross-compile for 2.11-2.13.

As for a local trick without version-specific files, perhaps the following works (untested):

package my

object Compat {
  object Inner1 {
    class nowarn(...) extends scala.annotation.Annotation
  }

  import Inner1._
  object Inner2 {
    import scala.annotation._

    type nowarncompat = nowarn
  }

  type nowarncompat = Inner2.nowarncompat
}

then use:

import my.{nowarncompat => nowarn}

@nowarn(...)
def whatever

[SethTisue]

I think we should go ahead and merge this and roll a release. You can argue (we have now argued) it both ways, but the point about cross-compiling to 2.11 too, not just 2.12, tips the scales for me. Also, aesthetically it seems a little nicer to me to keep compat-only stuff like this out of stdlib.

The "scala-collection-compat" naming ended up being unfortunate, but I don't think that should stand in the way of helping our users. And anyway, I wouldn't be against renaming the project, but as a separate matter, I don't think it's worth holding @nowarn up over.

So let's roll? It would be nice to have this shipped by the time 2.13.2 comes out, and that won't be much longer.

[ijuma]

I tested this and the annotation doesn't seem to be present in the binary jar. Compilation fails with Scala 2.12:

/home/ijuma/src/kafka/core/src/main/scala/kafka/admin/ConfigCommand.scala:42: object nowarn is not a member of package annotation
import scala.annotation.nowarn
^
/home/ijuma/src/kafka/core/src/main/scala/kafka/admin/ConfigCommand.scala:301: not found: type nowarn
@nowarn("cat=deprecation")
^

[lrytz]

d'oh

probably due to the osgi export-package clause

scala-collection-compat/build.sbt

Line 86 in c4b85d4

s"scala.collection.compat.*;version=${version.value},scala.jdk.*;version=${version.value}")

[lrytz]

Adding scala.annotation.* to the export-package clause above does too much, in that the resulting jar (publishLocal) now has scala/annotation/nowarn.class, but also copies of all other class files in scala/annotation from scala-library.jar (like scala/annotation/switch.class, scala/annotation/meta/field.class, ...).

I remember coming across this before in the context of @dwijnand's efforts to jardiff scala-collection-compat artifacts before and after the new vector.

OSGi is really a pain, because

1. we don't know what we should do
2. we don't know if what we do is correct
3. we don't know if anyone relies on what we do, and
4. (I believe) noone around here knows much about OSGi

Hopefully we can copy what Stefan did for parallel collections (scala/scala-parallel-collections@7558933). I'll experiment tomorrow.

The other modules are less problematic. The additional difficulty here is that this module defines classes in packages that overlap with the standard library.

[dwijnand]

Seth mentions

the point about cross-compiling to 2.11 too, not just 2.12, tips the scales for me

For me

We don't want to discourage the use of @nowarn.

tips the scales in the other way.

I say let's just add this to the 2.12 standard library and keep it simple.

[SethTisue]

I suggest we drop OSGi support in this repo. we did so in scala-collection-contrib already: scala/scala-collection-contrib#72 (and note, "it would be nice if sbt-scala-module provided a setting key to selectively disable OSGi")

I doubt the OSGI stuff in scala-parallel-collections stuff is actually right; I am proposing dropping OSGi there as well: scala/scala-parallel-collections#80

[NthPortal]

I vote that we either do as Seth suggests, or we add a counter somewhere to keep track of all the times OSGI has broken a release

[NthPortal]

Seth mentions

the point about cross-compiling to 2.11 too, not just 2.12, tips the scales for me

For me

We don't want to discourage the use of @nowarn.

tips the scales in the other way.

I say let's just add this to the 2.12 standard library and keep it simple.

@dwijnand could you elaborate on why not wanting to discourage the use of @nowarn means we should put it in the standard library and not in this compat library? it's not 100% clear to me

[dwijnand]

People are discouraged from adding libraries (and, in some cases, they have to jump through legal/bureaucratic hoops to do so), so adding it to the new version of the standard library avoids that. And @nowarn is related to cross-building, so there's even more reason to have it most available (unlike, Using, as an example).

[lrytz]

I tried adding ;-split-package:=first to the osgi export pacakge clause, like Stefan did in parallel collections.

He notes

The first entry on the classpath is the project's target classes dir but sbt-osgi also passes all dependencies to bnd. Any "merge" strategy for split packages would include the classes from scala-library.

However, it turns out that on my machine, the first entry passed to to bnd happens to be the scala library, not the project's target classes dir. So I end up with a jar that includes all annotations from scala-library, and misses nowarn.class.

So the classpath order assumption is probably wrong.

I'll go ahead with disabling OSGi.

[NthPortal]

People are discouraged from adding libraries

You're gonna have a rough time cross-compiling without this library if you touch collections anyway, so I think it's reasonable to have it here

[dwijnand]

(I actually avoided this library, sometimes just taking (source copying) a type alias. Mostly because it was going through breaking changing (awkwardly), but also just to avoid adding another dependency. But that's just my datapoint - I can see how your statement could be true in the large.)