Skip to content

Commit 954c5d3

Browse files
committed
Stabilize order of annotations in the class file
Regressed in scala#6846, which added support for encoding repeated annotations. Test failure before replacing `groupBy` with `LinkedHashMap`: ``` $ sbt junit/testOnly scala.tools.nsc.DeterminismTest ... java.lang.AssertionError: assertion failed: Difference detected between recompiling List(b.scala, Annot1.java) Run: jardiff -r /var/folders/tz/p8vd07wn7wxck3b9v54grlzw0000gp/T/reference814657788418452571 /var/folders/tz/p8vd07wn7wxck3b9v54grlzw0000gp/T/recompileOutput4882243280168823330 $ jardiff -r /var/folders/tz/p8vd07wn7wxck3b9v54grlzw0000gp/T/reference814657788418452571 /var/folders/tz/p8vd07wn7wxck3b9v54grlzw0000gp/T/recompileOutput4882243280168823330 diff --git a/Test.class.asm b/Test.class.asm index 98bfd80..a056f9a 100644 --- a/Test.class.asm +++ b/Test.class.asm @@ -4,10 +4,10 @@ // compiled from: b.scala - @LAnnot2;(value=java.lang.Object.class) - @LAnnot1;(value="foo") + @LAnnot2;(value=java.lang.Object.class) + @Lscala/reflect/ScalaSignature;(bytes="\u0006\u0001u1AAA\u0002\u0001\r!)Q\u0002\u0001C\u0001\u001d\u0009!A+Z:u\u0015\u0005!\u0011a\u0002\u001ff[B$\u0018PP\u0002\u0001'\u0009\u0001q\u0001\u0005\u0002\u0009\u00175\u0009\u0011BC\u0001\u000b\u0003\u0015\u00198-\u00197b\u0013\u0009a\u0011B\u0001\u0004B]f\u0014VMZ\u0001\u0007y%t\u0017\u000e\u001e \u0015\u0003=\u0001\"\u0001\u0005\u0001\u000e\u0003\rAC\u0001\u0001\n\u0016-A\u0011\u0001cE\u0005\u0003)\r\u0011a!\u00118o_R\u0014\u0014!\u0002<bYV,7%A\u0004)\u0009\u0001ARc\u0007\u0009\u0003!eI!AG\u0002\u0003\r\u0005sgn\u001c;2C\u0005a\u0012a\u00014p_\u0002") ``` WIP stabilize annotation ordering
1 parent 1e1a162 commit 954c5d3

File tree

2 files changed

+100
-7
lines changed

2 files changed

+100
-7
lines changed

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,11 +1436,13 @@ abstract class RefChecks extends Transform {
14361436
checkTypeRefBounds(ann.tpe, tree)
14371437
}
14381438

1439-
annots
1440-
.map(_.transformArgs(transformTrees))
1441-
.groupBy(_.symbol)
1442-
.flatMap((groupRepeatableAnnotations _).tupled)
1443-
.toList
1439+
val annotsBySymbol = new mutable.LinkedHashMap[Symbol, ListBuffer[AnnotationInfo]]()
1440+
val transformedAnnots = annots.map(_.transformArgs(transformTrees))
1441+
for (transformedAnnot <- transformedAnnots) {
1442+
val buffer = annotsBySymbol.getOrElseUpdate(transformedAnnot.symbol, new ListBuffer)
1443+
buffer += transformedAnnot
1444+
}
1445+
annotsBySymbol.iterator.flatMap(x => groupRepeatableAnnotations(x._1, x._2.toList)).toList
14441446
}
14451447

14461448
// assumes non-empty `anns`

test/junit/scala/tools/nsc/DeterminismTest.scala

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package scala.tools.nsc
22

3+
import java.io.{File, OutputStreamWriter}
4+
import java.nio.charset.Charset
35
import java.nio.file.attribute.BasicFileAttributes
46
import java.nio.file.{FileVisitResult, Files, Path, SimpleFileVisitor}
57
import java.util
68

9+
import javax.tools.ToolProvider
710
import org.junit.Test
811

9-
import scala.collection.JavaConverters.asScalaIteratorConverter
12+
import scala.collection.JavaConverters.{asScalaIteratorConverter, seqAsJavaListConverter}
13+
import scala.collection.immutable
1014
import scala.language.implicitConversions
1115
import scala.reflect.internal.util.{BatchSourceFile, SourceFile}
1216
import scala.reflect.io.PlainNioFile
@@ -187,6 +191,78 @@ class DeterminismTest {
187191
test(List(code))
188192
}
189193

194+
@Test def testAnnotations1(): Unit = {
195+
def code = List[SourceFile](
196+
source("a.scala",
197+
"""
198+
|class Annot1(s: String) extends scala.annotation.StaticAnnotation
199+
|class Annot2(s: Class[_]) extends scala.annotation.StaticAnnotation
200+
|
201+
""".stripMargin),
202+
source("b.scala",
203+
"""
204+
|@Annot1("foo")
205+
|@Annot2(classOf[AnyRef])
206+
|class Test
207+
""".stripMargin)
208+
)
209+
test(List(code))
210+
}
211+
212+
@Test def testAnnotationsJava(): Unit = {
213+
def code = List[SourceFile](
214+
source("Annot1.java",
215+
"""
216+
|import java.lang.annotation.*;
217+
|@Retention(RetentionPolicy.RUNTIME)
218+
|@Target(ElementType.TYPE)
219+
|@Inherited
220+
|@interface Annot1 { String value() default ""; }
221+
|
222+
|@Retention(RetentionPolicy.RUNTIME)
223+
|@Target(ElementType.TYPE)
224+
|@Inherited
225+
|@interface Annot2 { Class value(); }
226+
|
227+
""".stripMargin),
228+
source("b.scala",
229+
"""
230+
|@Annot1("foo") @Annot2(classOf[AnyRef]) class Test
231+
""".stripMargin)
232+
)
233+
test(List(code))
234+
}
235+
236+
@Test def testAnnotationsJavaRepeatable(): Unit = {
237+
val javaAnnots = source("Annot1.java",
238+
"""
239+
|import java.lang.annotation.*;
240+
|@Repeatable(Annot1.Container.class)
241+
|@Retention(RetentionPolicy.RUNTIME)
242+
|@Target(ElementType.TYPE)
243+
|@interface Annot1 { String value() default "";
244+
|
245+
| @Retention(RetentionPolicy.RUNTIME)
246+
| @Target(ElementType.TYPE)
247+
| public static @interface Container {
248+
| Annot1[] value();
249+
| }
250+
|}
251+
|
252+
|@Retention(RetentionPolicy.RUNTIME)
253+
|@Target(ElementType.TYPE)
254+
|@Inherited
255+
|@interface Annot2 { Class value(); }
256+
""".stripMargin)
257+
def code =
258+
List(source("dummy.scala", ""), source("b.scala",
259+
"""
260+
|@Annot1("foo") @Annot2(classOf[String]) @Annot1("bar") class Test
261+
""".stripMargin)
262+
)
263+
test(List(javaAnnots) :: code :: Nil)
264+
}
265+
190266
def source(name: String, code: String): SourceFile = new BatchSourceFile(name, code)
191267
private def test(groups: List[List[SourceFile]]): Unit = {
192268
val referenceOutput = Files.createTempDirectory("reference")
@@ -202,7 +278,22 @@ class DeterminismTest {
202278
val r = new Run
203279
// println("scalac " + files.mkString(" "))
204280
r.compileSources(files)
205-
assert(!storeReporter.hasErrors, storeReporter.infos.mkString("\n"))
281+
Predef.assert(!storeReporter.hasErrors, storeReporter.infos.mkString("\n"))
282+
files.filter(_.file.name.endsWith(".java")) match {
283+
case Nil =>
284+
case javaSources =>
285+
def tempFileFor(s: SourceFile): Path = {
286+
val f = output.resolve(s.file.name)
287+
Files.write(f, new String(s.content).getBytes(Charset.defaultCharset()))
288+
}
289+
val options = List("-d", output.toString)
290+
val javac = ToolProvider.getSystemJavaCompiler
291+
val fileMan = javac.getStandardFileManager(null, null, null)
292+
val javaFileObjects = fileMan.getJavaFileObjects(javaSources.map(s => tempFileFor(s).toAbsolutePath.toString): _*)
293+
val task = javac.getTask(new OutputStreamWriter(System.out), fileMan, null, options.asJava, Nil.asJava, javaFileObjects)
294+
val result = task.call()
295+
Predef.assert(result)
296+
}
206297
}
207298

208299
for (group <- groups.init) {

0 commit comments

Comments
 (0)