Minor architectural clean-up

[anatoliykmetyuk]

No description provided.

[allanrenucci]

Otherwise LGTM

[allanrenucci]

Now that it's public, I think compiler/src/dotty/tools/dotc/ast/tpd.scala is a better place for this

[anatoliykmetyuk]

Hmm, this one is quite annotation-specific. I think tpd has lots of things already in it - why do you think it would look better there?

[allanrenucci]

Doesn't it work with any types? I realize it is currently only used for annotations, but I would expect there are other places where this could be used. Here is a possible candidate:

https://github.com/lampepfl/dotty/blob/d8ac6cdf97ce14018e862c84b85d44f88d83a540/compiler/src/dotty/tools/backend/sjs/JUnitBootstrappers.scala#L253-L257

If this is only used for annotations, then I agree Annotations.scala is a better place.

[anatoliykmetyuk]

Indeed, makes sense, it can well be used for other types. I'm not sure of your example though, the lack of type parameters handling and the absence of applyOverloaded make me wonder about the semantic differences.

[allanrenucci]

There is no semantic difference in this particular use case. The author is performing overloading resolution manually. He is looking for an overload for the constructor of NoSuchMethodException. There is no type parameter and using the following predicate is enough to select the right constructor:

c.info.paramInfoss.head.size == 1 && c.info.paramInfoss.head.head.isRef(defn.StringClass)

So we could rewrite the code as follow:

--- a/compiler/src/dotty/tools/backend/sjs/JUnitBootstrappers.scala
+++ b/compiler/src/dotty/tools/backend/sjs/JUnitBootstrappers.scala
@@ -250,11 +250,7 @@ class JUnitBootstrappers extends MiniPhase {
         ValDef(castInstanceSym, instanceParamRef.cast(testClass.typeRef)) :: Nil,
         tests.foldRight[Tree] {
           val tp = junitdefn.NoSuchMethodExceptionType
-          val constr = tp.member(nme.CONSTRUCTOR).suchThat { c =>
-            c.info.paramInfoss.head.size == 1 &&
-            c.info.paramInfoss.head.head.isRef(defn.StringClass)
-          }.symbol.asTerm
-          Throw(New(tp, constr, nameParamRef :: Nil))
+          Throw(resolveConstructor(tp, nameParamRef :: Nil))
         } { (test, next) =>
           If(Literal(Constant(test.name.toString)).select(defn.Any_equals).appliedTo(nameParamRef),
             genTestInvocation(testClass, test, ref(castInstanceSym)),

[allanrenucci]

Here is another candidate:

--- a/compiler/src/dotty/tools/backend/sjs/JUnitBootstrappers.scala
+++ b/compiler/src/dotty/tools/backend/sjs/JUnitBootstrappers.scala
@@ -229,8 +229,7 @@ class JUnitBootstrappers extends MiniPhase {
         val testAnnot = test.getAnnotation(junitdefn.TestAnnotClass).get
         if (testAnnot.arguments.nonEmpty)
           ctx.error("@Test annotations with arguments are not yet supported in Scala.js for dotty", testAnnot.tree.sourcePos)
-        val noArgConstr = junitdefn.TestAnnotType.member(nme.CONSTRUCTOR).suchThat(_.info.paramInfoss.head.isEmpty).symbol.asTerm
-        val reifiedAnnot = New(junitdefn.TestAnnotType, noArgConstr, Nil)
+        val reifiedAnnot = resolveConstructor(junitdefn.TestAnnotType, Nil)
         New(junitdefn.TestMetadataType, List(name, ignored, reifiedAnnot))
       }
       JavaSeqLiteral(metadata, TypeTree(junitdefn.TestMetadataType))

[anatoliykmetyuk]

Ok, let's try...

[anatoliykmetyuk]

/cc @sjrd since it touches ScalaJS codebase