Repository: 7mind/izumi (scala)

Issue:
Improve error message for missing dependency when there are named bindings of the same type or bindings to subtypes
Reported on gitter https://gitter.im/7mind/izumi?at=60896bdbb38a713b2fb4a31c

When there's missing dependency error for type `X` and there exist defined named bindings for the same type such as `X @Id("x")`, what could happen is that the user made a typo or an forgot the qualifier, to help with these situations, the error message should also list all existing bindings that share the same type such as:

```
Couldn't find `X @Id("a")` required by refs: A, B, C

Found other bindings the same type (did you forget to add or remove `@Id` annotation?):
  - X
  - X @Id("b")
  - X @Id("c")
```

As well, the list of related bindings could also feature those with a compatible type (by subtyping comparison), so that the user may find the correct binding for an interface if they tried to request a supertype of an existing binding:

```
Couldn't find `AnyVal` required by refs: A, B, C

Found other bindings the same type (did you forget to add or remove `@Id` annotation?):
  - AnyVal @Id("other")

Found other bindings for related subtypes (did you mean to summon one of them?):
  - Int
  - Boolean
  - Double @Id("NaN")
```

Pull request:
closes #1465

Patch:
diff --git a/distage/distage-core-api/src/main/scala/izumi/distage/model/definition/errors/ProvisionerIssue.scala b/distage/distage-core-api/src/main/scala/izumi/distage/model/definition/errors/ProvisionerIssue.scala
index 4c99c8fa48..31c979ccf3 100644
--- a/distage/distage-core-api/src/main/scala/izumi/distage/model/definition/errors/ProvisionerIssue.scala
+++ b/distage/distage-core-api/src/main/scala/izumi/distage/model/definition/errors/ProvisionerIssue.scala
@@ -1,5 +1,6 @@
 package izumi.distage.model.definition.errors
 
+import izumi.distage.model.definition.Binding
 import izumi.distage.model.exceptions.runtime.IntegrationCheckException
 import izumi.distage.model.plan.ExecutableOp
 import izumi.distage.model.plan.ExecutableOp.{ImportDependency, MonadicOp, ProxyOp}
@@ -25,7 +26,7 @@ object ProvisionerIssue {
     final case class UnexpectedIntegrationCheck(key: DIKey, problem: Throwable) extends ProvisionerExceptionIssue
   }
 
-  final case class MissingImport(op: ImportDependency) extends ProvisionerIssue {
+  final case class MissingImport(op: ImportDependency, similarSame: Set[Binding], similarSub: Set[Binding]) extends ProvisionerIssue {
     override def key: DIKey = op.target
   }
 
diff --git a/distage/distage-core-api/src/main/scala/izumi/distage/model/exceptions/runtime/MissingInstanceException.scala b/distage/distage-core-api/src/main/scala/izumi/distage/model/exceptions/runtime/MissingInstanceException.scala
index bc373ca551..c2123a8a3a 100644
--- a/distage/distage-core-api/src/main/scala/izumi/distage/model/exceptions/runtime/MissingInstanceException.scala
+++ b/distage/distage-core-api/src/main/scala/izumi/distage/model/exceptions/runtime/MissingInstanceException.scala
@@ -1,5 +1,6 @@
 package izumi.distage.model.exceptions.runtime
 
+import izumi.distage.model.definition.Binding
 import izumi.distage.model.exceptions.DIException
 import izumi.distage.model.reflection.DIKey
 import izumi.fundamentals.platform.strings.IzString.*
@@ -7,18 +8,30 @@ import izumi.fundamentals.platform.strings.IzString.*
 class MissingInstanceException(message: String, val key: DIKey) extends DIException(message)
 
 object MissingInstanceException {
-  def format(target: DIKey, references: Set[DIKey]): String = {
+  def format(target: DIKey, references: Set[DIKey], similarSame: Set[Binding], similarSub: Set[Binding]): String = {
     // TODO: this representation is imperfect, we will have to change this once we have https://github.com/zio/izumi-reflect/issues/367 implemented
     import izumi.reflect.macrortti.LTTRenderables.Short._
 
     val refRepr = target.tpe.tag.ref.render()
 
-    s"""Instance is not available in the object graph: $target.
-       |Required by refs: ${references.niceList(prefix = "* ")}
-       |
-       |You may add missing binding with code alike to
-       |
-       |  make[$refRepr]
-       |""".stripMargin
+    def formatRelevant(similar: Iterable[Binding], header: String) = {
+      Option(similar)
+        .filterNot(_.isEmpty)
+        .map {
+          bb =>
+            s"$header: ${bb.map(b => s"${b.key} defined at ${b.origin}").niceList()}".stripMargin
+        }
+        .toSeq
+    }
+    val similarSameHints = formatRelevant(similarSame, "Found other bindings for the same type (did you forget to add or remove `@Id` annotation?)")
+    val similarSubHints = formatRelevant(similarSub, "Found other bindings for related subtypes (did you mean to summon one of them?)")
+
+    (Seq(
+      s"""Instance is not available in the object graph: $target.
+         |Required by refs: ${references.niceList(prefix = "* ")}""".stripMargin,
+      s"""You may add missing binding with code alike to
+         |
+         |  make[$refRepr]""".stripMargin,
+    ) ++ similarSameHints ++ similarSubHints).mkString("\n\n")
   }
 }
diff --git a/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/PlanInterpreter.scala b/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/PlanInterpreter.scala
index ace85e2101..090031303f 100644
--- a/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/PlanInterpreter.scala
+++ b/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/PlanInterpreter.scala
@@ -107,8 +107,8 @@ object PlanInterpreter {
                 case UnexpectedIntegrationCheck(key, problem) =>
                   IzumiProject.bugReportPrompt(s"unexpected exception while processing integration check for $key", stackTrace(problem))
 
-                case MissingImport(op) =>
-                  MissingInstanceException.format(op.target, op.references)
+                case MissingImport(op, similarSame, similarSub) =>
+                  MissingInstanceException.format(op.target, op.references, similarSame, similarSub)
 
                 case IncompatibleEffectTypes(op, provisionerEffectType, actionEffectType) =>
                   IncompatibleEffectTypes.format(op, provisionerEffectType, actionEffectType)
diff --git a/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/strategies/ImportStrategy.scala b/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/strategies/ImportStrategy.scala
index 0143d66384..c66031b690 100644
--- a/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/strategies/ImportStrategy.scala
+++ b/distage/distage-core-api/src/main/scala/izumi/distage/model/provisioning/strategies/ImportStrategy.scala
@@ -2,8 +2,9 @@ package izumi.distage.model.provisioning.strategies
 
 import izumi.distage.model.definition.errors.ProvisionerIssue.MissingImport
 import izumi.distage.model.plan.ExecutableOp.ImportDependency
+import izumi.distage.model.plan.Plan
 import izumi.distage.model.provisioning.{NewObjectOp, ProvisioningKeyProvider}
 
 trait ImportStrategy {
-  def importDependency(context: ProvisioningKeyProvider, op: ImportDependency): Either[MissingImport, Seq[NewObjectOp]]
+  def importDependency(context: ProvisioningKeyProvider, plan: Plan, op: ImportDependency): Either[MissingImport, Seq[NewObjectOp]]
 }
diff --git a/distage/distage-core/src/main/scala/izumi/distage/planning/solver/PlanVerifier.scala b/distage/distage-core/src/main/scala/izumi/distage/planning/solver/PlanVerifier.scala
index 87e932a696..247102cd64 100644
--- a/distage/distage-core/src/main/scala/izumi/distage/planning/solver/PlanVerifier.scala
+++ b/distage/distage-core/src/main/scala/izumi/distage/planning/solver/PlanVerifier.scala
@@ -1,7 +1,7 @@
 package izumi.distage.planning.solver
 
 import distage.TagK
-import izumi.distage.model.definition.ModuleBase
+import izumi.distage.model.definition.{Binding, ModuleBase}
 import izumi.distage.model.definition.conflicts.{Annotated, Node}
 import izumi.distage.model.exceptions.PlanVerificationException
 import izumi.distage.model.exceptions.runtime.MissingInstanceException
@@ -15,6 +15,7 @@ import izumi.distage.planning.BindingTranslator
 import izumi.distage.planning.solver.PlanVerifier.PlanIssue.*
 import izumi.distage.planning.solver.PlanVerifier.{PlanIssue, PlanVerifierResult}
 import izumi.distage.planning.solver.SemigraphSolver.SemiEdgeSeq
+import izumi.distage.provisioning.strategies.ImportStrategyDefaultImpl
 import izumi.functional.IzEither.*
 import izumi.fundamentals.collections.{ImmutableMultiMap, MutableMultiMap}
 import izumi.fundamentals.collections.IzCollections.*
@@ -79,7 +80,7 @@ class PlanVerifier(
     var after = before
     val issues =
       try {
-        trace(allAxis, mutVisited, matrixToTrace, execOpIndex, justMutators, providedKeys, excludedActivations, rootKeys, effectType)
+        trace(allAxis, mutVisited, matrixToTrace, execOpIndex, justMutators, providedKeys, excludedActivations, rootKeys, effectType, bindings)
       } finally {
         after = System.currentTimeMillis()
       }
@@ -103,6 +104,7 @@ class PlanVerifier(
     excludedActivations: Set[NonEmptySet[AxisPoint]],
     rootKeys: Set[DIKey],
     effectType: SafeType,
+    bindings: ModuleBase,
   ): Set[PlanIssue] = {
 
     @inline def go(visited: Set[DIKey], current: Set[(DIKey, DIKey)], currentActivation: Set[AxisPoint]): RecursionResult = RecursionResult(current.iterator.map {
@@ -111,7 +113,9 @@ class PlanVerifier(
           Right(Iterator.empty)
         } else {
           @inline def reportMissing[A](key: DIKey, dependee: DIKey): Left[List[MissingImport], Nothing] = {
-            Left(List(MissingImport(key, dependee, allImportingBindings(matrix, currentActivation)(key, dependee))))
+            val origins = allImportingBindings(matrix, currentActivation)(key, dependee)
+            val similarBindings = ImportStrategyDefaultImpl.findSimilarImports(bindings, key)
+            Left(List(MissingImport(key, dependee, origins, similarBindings.similarSame, similarBindings.similarSub)))
           }
 
           @inline def reportMissingIfNotProvided[A](key: DIKey, dependee: DIKey)(orElse: => Either[List[PlanIssue], A]): Either[List[PlanIssue], A] = {
@@ -451,10 +455,10 @@ object PlanVerifier {
     def key: DIKey
   }
   object PlanIssue {
-    final case class MissingImport(key: DIKey, dependee: DIKey, origins: Set[OperationOrigin]) extends PlanIssue {
+    final case class MissingImport(key: DIKey, dependee: DIKey, origins: Set[OperationOrigin], similarSame: Set[Binding], similarSub: Set[Binding]) extends PlanIssue {
       override def toString: String = {
         // FIXME: reuse formatting from conflictingAxisTagsHint [show multiple origins with different axes]
-        MissingInstanceException.format(key, Set(dependee))
+        MissingInstanceException.format(key, Set(dependee), similarSame, similarSub)
       }
     }
 
diff --git a/distage/distage-core/src/main/scala/izumi/distage/provisioning/PlanInterpreterNonSequentialRuntimeImpl.scala b/distage/distage-core/src/main/scala/izumi/distage/provisioning/PlanInterpreterNonSequentialRuntimeImpl.scala
index 50dc282cd0..bdbc91f64f 100644
--- a/distage/distage-core/src/main/scala/izumi/distage/provisioning/PlanInterpreterNonSequentialRuntimeImpl.scala
+++ b/distage/distage-core/src/main/scala/izumi/distage/provisioning/PlanInterpreterNonSequentialRuntimeImpl.scala
@@ -179,7 +179,7 @@ class PlanInterpreterNonSequentialRuntimeImpl(
       before <- F.maybeSuspend(System.nanoTime())
       res <- op match {
         case op: ImportDependency =>
-          F.pure(importStrategy.importDependency(context.asContext(), op))
+          F.pure(importStrategy.importDependency(context.asContext(), context.plan,  op))
         case op: NonImportOp =>
           operationExecutor.execute(context.asContext(), op)
       }
diff --git a/distage/distage-core/src/main/scala/izumi/distage/provisioning/strategies/ImportStrategyDefaultImpl.scala b/distage/distage-core/src/main/scala/izumi/distage/provisioning/strategies/ImportStrategyDefaultImpl.scala
index b718c843f8..81b597d22d 100644
--- a/distage/distage-core/src/main/scala/izumi/distage/provisioning/strategies/ImportStrategyDefaultImpl.scala
+++ b/distage/distage-core/src/main/scala/izumi/distage/provisioning/strategies/ImportStrategyDefaultImpl.scala
@@ -1,17 +1,39 @@
 package izumi.distage.provisioning.strategies
 
+import distage.{DIKey, ModuleBase}
+import izumi.distage.model.definition.Binding
 import izumi.distage.model.definition.errors.ProvisionerIssue.MissingImport
 import izumi.distage.model.plan.ExecutableOp.ImportDependency
+import izumi.distage.model.plan.Plan
 import izumi.distage.model.provisioning.strategies.ImportStrategy
 import izumi.distage.model.provisioning.{NewObjectOp, ProvisioningKeyProvider}
 
 class ImportStrategyDefaultImpl extends ImportStrategy {
-  override def importDependency(context: ProvisioningKeyProvider, op: ImportDependency): Either[MissingImport, Seq[NewObjectOp]] = {
+  override def importDependency(context: ProvisioningKeyProvider, plan: Plan, op: ImportDependency): Either[MissingImport, Seq[NewObjectOp]] = {
     context.importKey(op.target) match {
       case Some(v) =>
         Right(Seq(NewObjectOp.NewImport(op.target, v)))
       case _ =>
-        Left(MissingImport(op))
+        val similar = ImportStrategyDefaultImpl.findSimilarImports(plan.input.bindings, op.target)
+        Left(MissingImport(op, similar.similarSame, similar.similarSub))
     }
   }
+
+}
+
+object ImportStrategyDefaultImpl {
+  case class SimilarBindings(similarSame: Set[Binding], similarSub: Set[Binding])
+
+  def findSimilarImports(bindings: ModuleBase, target: DIKey): SimilarBindings = {
+    def isSimilarSameTypeBinding(to: DIKey)(binding: Binding): Boolean = {
+      to.tpe =:= binding.key.tpe
+    }
+
+    def isSimilarSubTypeBinding(to: DIKey)(binding: Binding): Boolean = {
+      !(to.tpe =:= binding.key.tpe) && (binding.key.tpe <:< to.tpe)
+    }
+    val similarSame = bindings.bindings.filter(isSimilarSameTypeBinding(target))
+    val similarSub = bindings.bindings.filter(isSimilarSubTypeBinding(target))
+    SimilarBindings(similarSame, similarSub)
+  }
 }


Test patch:
diff --git a/distage/distage-core/src/test/scala/izumi/distage/fixtures/BasicCases.scala b/distage/distage-core/src/test/scala/izumi/distage/fixtures/BasicCases.scala
index e193910c5b..4c1e37a772 100644
--- a/distage/distage-core/src/test/scala/izumi/distage/fixtures/BasicCases.scala
+++ b/distage/distage-core/src/test/scala/izumi/distage/fixtures/BasicCases.scala
@@ -247,4 +247,13 @@ Forest fire, climbin' higher, real life, it can wait""")
     abstract class NoArgAbstractClass
   }
 
+  object BasicCase9 {
+    trait T1
+    class Dep1() extends T1
+    trait T2
+    class Dep2() extends T2
+
+    class Out(val dep1: T1, val dep2: T2)
+  }
+
 }
diff --git a/distage/distage-core/src/test/scala/izumi/distage/injector/BasicTest.scala b/distage/distage-core/src/test/scala/izumi/distage/injector/BasicTest.scala
index eaca737b2a..4ad0afc6c9 100644
--- a/distage/distage-core/src/test/scala/izumi/distage/injector/BasicTest.scala
+++ b/distage/distage-core/src/test/scala/izumi/distage/injector/BasicTest.scala
@@ -17,7 +17,6 @@ import org.scalatest.exceptions.TestFailedException
 import org.scalatest.wordspec.AnyWordSpec
 
 class BasicTest extends AnyWordSpec with MkInjector {
-
   "maintain correct operation order" in {
     import BasicCase1.*
     val definition = PlannerInput(
@@ -560,4 +559,28 @@ class BasicTest extends AnyWordSpec with MkInjector {
     assert(instance == max)
   }
 
+  "provide reasonable hints for missing dependencies" in {
+    import BasicCase9.*
+    val definition = PlannerInput(
+      new ModuleDef {
+        make[Out]
+        make[Dep1]
+        make[T2].named("wrong").from[Dep2]
+      },
+      Activation.empty,
+      DIKey[Out],
+    )
+
+    val injector = mkInjector()
+    val plan = injector.planUnsafe(definition)
+    assert(plan.stepsUnordered.exists(_.isInstanceOf[ImportDependency]))
+
+    val exc = intercept[ProvisioningException] {
+      injector.produce(plan).unsafeGet()
+    }
+
+    assert(exc.getMessage.contains("related subtypes"))
+    assert(exc.getMessage.contains("same type"))
+    println(exc.getMessage)
+  }
 }
diff --git a/distage/distage-core/src/test/scala/izumi/distage/injector/PlanVerifierTest.scala b/distage/distage-core/src/test/scala/izumi/distage/injector/PlanVerifierTest.scala
index 8b4a9c0b34..5584e0101b 100644
--- a/distage/distage-core/src/test/scala/izumi/distage/injector/PlanVerifierTest.scala
+++ b/distage/distage-core/src/test/scala/izumi/distage/injector/PlanVerifierTest.scala
@@ -2,6 +2,7 @@ package izumi.distage.injector
 
 import distage.{Activation, DIKey, Injector, Lifecycle, Roots}
 import izumi.distage.fixtures.PlanVerifierCases.*
+import izumi.distage.model.definition.Binding.SetElementBinding
 import izumi.distage.model.definition.ModuleDef
 import izumi.distage.model.exceptions.planning.InjectorFailed
 import izumi.distage.model.plan.operations.OperationOrigin
@@ -167,7 +168,13 @@ class PlanVerifierTest extends AnyWordSpec with MkInjector {
     val result = PlanVerifier().verify[Identity](definition, Roots.target[Fork1], Injector.providedKeys(), Set.empty)
     assert(result.verificationFailed)
     assert(result.issues.size == 1)
-    assert(result.issues.fromNonEmptySet == Set(MissingImport(DIKey[Fork2], DIKey[Fork1], Set(implBOrigin))))
+//    val issues = result.issues.toList.flatMap(_.toList)
+//    assert(issues.size == 1)
+//
+//    val imports = issues.collect {case m: MissingImport => m}
+//    assert(imports.size == 1)
+//    assert(imports.head.key == )
+    assert(result.issues.fromNonEmptySet == Set(MissingImport(DIKey[Fork2], DIKey[Fork1], Set(implBOrigin), Set.empty, Set.empty)))
   }
 
   "Verifier flags conflicting activations" in {
@@ -510,13 +517,22 @@ class PlanVerifierTest extends AnyWordSpec with MkInjector {
 
     val result1 = PlanVerifier().verify[Identity](definition, Roots.target[X], Injector.providedKeys(), Set.empty)
     assert(result1.issues.fromNonEmptySet.map(_.getClass) == Set(classOf[MissingImport], classOf[UnsaturatedAxis]))
+    val brokenBinding = definition.bindings.filter(b => b.isInstanceOf[SetElementBinding])
     assert(
       result1.issues.fromNonEmptySet == Set(
-        MissingImport(DIKey[ExternalDep], DIKey[X], result1.issues.fromNonEmptySet.collect { case MissingImport(_, d, origins) if d == DIKey[X] => origins }.flatten),
         MissingImport(
           DIKey[ExternalDep],
-          result1.issues.get.collectFirst { case MissingImport(_, d, _) if d.isInstanceOf[DIKey.SetElementKey] => d }.get,
-          result1.issues.fromNonEmptySet.collect { case MissingImport(_, d, origins) if d.isInstanceOf[DIKey.SetElementKey] => origins }.flatten,
+          DIKey[X],
+          result1.issues.fromNonEmptySet.collect { case MissingImport(_, d, origins, _, _) if d == DIKey[X] => origins }.flatten,
+          brokenBinding,
+          Set.empty,
+        ),
+        MissingImport(
+          DIKey[ExternalDep],
+          result1.issues.get.collectFirst { case MissingImport(_, d, _, _, _) if d.isInstanceOf[DIKey.SetElementKey] => d }.get,
+          result1.issues.fromNonEmptySet.collect { case MissingImport(_, d, origins, _, _) if d.isInstanceOf[DIKey.SetElementKey] => origins }.flatten,
+          brokenBinding,
+          Set.empty,
         ),
         UnsaturatedAxis(DIKey[BadDep], "axis", NonEmptySet(AxisPoint("axis", "a"))),
       )