Skip to content

Commit

Permalink
Keep annotation order
Browse files Browse the repository at this point in the history
This change makes sure non-repeated annotations are kept in the order
they were found in the source code.

The motivation is not necessarily to have them in the original order,
but to have them in an order that is deterministic across rebuilds
(potentially even across different machines), for reasons discussed
further in scala#7661 and the corresponding scala/scala-dev#405

Some integration tests were added in `tests/pos` to be picked up by
`IdempotencyCheck.scala`, but unfortunately I haven't successfully
reproduced the nondeterminism that way.

I didn't see an obvious place for a 'unit test' of this code, I'd be
happy to add one when someone can recommend a good place to put it.

This is basically the dotty equivalent of
scala/scala@954c5d3

Fixes scala#14743
  • Loading branch information
raboof authored and bishabosha committed Oct 18, 2022
1 parent 3f44f5c commit 862f0c1
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import Constants._
import Types._
import Decorators._

import scala.collection.mutable

class RepeatableAnnotations extends MiniPhase:

override def phaseName: String = RepeatableAnnotations.name
Expand All @@ -28,7 +30,7 @@ class RepeatableAnnotations extends MiniPhase:
tree

private def aggregateAnnotations(annotations: Seq[Annotation])(using Context): List[Annotation] =
val annsByType = annotations.groupBy(_.symbol)
val annsByType = stableGroupBy(annotations, _.symbol)
annsByType.flatMap {
case (_, a :: Nil) => a :: Nil
case (sym, anns) if sym.derivesFrom(defn.ClassfileAnnotationClass) =>
Expand All @@ -50,6 +52,14 @@ class RepeatableAnnotations extends MiniPhase:
case (_, anns) => anns
}.toList

private def stableGroupBy[A, K](ins: Seq[A], f: A => K): scala.collection.MapView[K, List[A]] =
val out = new mutable.LinkedHashMap[K, mutable.ListBuffer[A]]()
for (in <- ins) {
val buffer = out.getOrElseUpdate(f(in), new mutable.ListBuffer)
buffer += in
}
out.view.mapValues(_.toList)

object RepeatableAnnotations:
val name: String = "repeatableAnnotations"
val description: String = "aggregate repeatable annotations"
8 changes: 8 additions & 0 deletions tests/pos/Annotations.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package foo.bar

import jdk.jfr.Enabled

@Enabled
@Deprecated
final class Annotations {
}
2 changes: 2 additions & 0 deletions tests/pos/annotations1/a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class Annot1(s: String) extends scala.annotation.StaticAnnotation
class Annot2(s: Class[_]) extends scala.annotation.StaticAnnotation
3 changes: 3 additions & 0 deletions tests/pos/annotations1/b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
@Annot1("foo")
@Annot2(classOf[AnyRef])
class Test
5 changes: 5 additions & 0 deletions tests/pos/annotationsJava/Annot1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import java.lang.annotation.*;
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
@interface Annot1 { String value() default ""; }
5 changes: 5 additions & 0 deletions tests/pos/annotationsJava/Annot2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import java.lang.annotation.*;
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
@interface Annot2 { Class value(); }
1 change: 1 addition & 0 deletions tests/pos/annotationsJava/b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Annot1("foo") @Annot2(classOf[AnyRef]) class Test
13 changes: 13 additions & 0 deletions tests/pos/annotationsJavaRepeatable/Annot1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import java.lang.annotation.*;

@Repeatable(Annot1.Container.class)
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@interface Annot1 { String value() default "";

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
public static @interface Container {
Annot1[] value();
}
}
6 changes: 6 additions & 0 deletions tests/pos/annotationsJavaRepeatable/Annot2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import java.lang.annotation.*;

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.TYPE)
@Inherited
@interface Annot2 { Class value(); }
1 change: 1 addition & 0 deletions tests/pos/annotationsJavaRepeatable/b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
@Annot1("foo") @Annot2(classOf[String]) @Annot1("bar") class Test

0 comments on commit 862f0c1

Please sign in to comment.