From 667a8255d994b334dfc87bd89a970855748752fe Mon Sep 17 00:00:00 2001 From: InversionSpaces Date: Tue, 4 Jul 2023 12:07:22 +0200 Subject: [PATCH] feat(compiler): Find and display link cycles (#787) Find and display dependency cycles --- .../scala/aqua/compiler/AquaCompiler.scala | 6 +- .../main/scala/aqua/compiler/AquaError.scala | 2 +- io/src/main/scala/aqua/ErrorRendering.scala | 11 +- .../src/main/scala/aqua/linker/Linker.scala | 100 +++++++++++++++--- .../test/scala/aqua/linker/LinkerSpec.scala | 12 +-- 5 files changed, 105 insertions(+), 26 deletions(-) diff --git a/compiler/src/main/scala/aqua/compiler/AquaCompiler.scala b/compiler/src/main/scala/aqua/compiler/AquaCompiler.scala index 34ac6703..5be419d6 100644 --- a/compiler/src/main/scala/aqua/compiler/AquaCompiler.scala +++ b/compiler/src/main/scala/aqua/compiler/AquaCompiler.scala @@ -11,7 +11,7 @@ import aqua.res.AquaRes import aqua.semantics.{CompilerState, Semantics} import aqua.semantics.header.{HeaderHandler, HeaderSem, Picker} import cats.data.* -import cats.data.Validated.{Invalid, Valid, validNec} +import cats.data.Validated.{validNec, Invalid, Valid} import cats.parse.Parser0 import cats.syntax.applicative.* import cats.syntax.flatMap.* @@ -19,7 +19,7 @@ import cats.syntax.functor.* import cats.syntax.monoid.* import cats.syntax.traverse.* import cats.syntax.semigroup.* -import cats.{Comonad, Functor, Monad, Monoid, Order, ~>} +import cats.{~>, Comonad, Functor, Monad, Monoid, Order} import scribe.Logging class AquaCompiler[F[_]: Monad, E, I: Order, S[_]: Comonad, C: Monoid: Picker]( @@ -39,7 +39,7 @@ class AquaCompiler[F[_]: Monad, E, I: Order, S[_]: Comonad, C: Monoid: Picker]( Err, ValidatedCtxT ], - cycleError: List[AquaModule[I, Err, ValidatedCtxT]] => Err + cycleError: Linker.DepCycle[AquaModule[I, Err, ValidatedCtxT]] => Err ): ValidatedNec[Err, Map[I, ValidatedCtx]] = { logger.trace("linking modules...") diff --git a/compiler/src/main/scala/aqua/compiler/AquaError.scala b/compiler/src/main/scala/aqua/compiler/AquaError.scala index 77c787cf..bd15d066 100644 --- a/compiler/src/main/scala/aqua/compiler/AquaError.scala +++ b/compiler/src/main/scala/aqua/compiler/AquaError.scala @@ -13,7 +13,7 @@ case class ResolveImportsErr[I, E, S[_]](fromFile: I, token: Token[S], err: E) extends AquaError[I, E, S] case class ImportErr[I, E, S[_]](token: Token[S]) extends AquaError[I, E, S] -case class CycleError[I, E, S[_]](modules: List[I]) extends AquaError[I, E, S] +case class CycleError[I, E, S[_]](modules: NonEmptyChain[I]) extends AquaError[I, E, S] case class CompileError[I, E, S[_]](err: SemanticError[S]) extends AquaError[I, E, S] case class OutputError[I, E, S[_]](compiled: AquaCompiled[I], err: E) extends AquaError[I, E, S] diff --git a/io/src/main/scala/aqua/ErrorRendering.scala b/io/src/main/scala/aqua/ErrorRendering.scala index e348d8eb..6950c177 100644 --- a/io/src/main/scala/aqua/ErrorRendering.scala +++ b/io/src/main/scala/aqua/ErrorRendering.scala @@ -75,7 +75,16 @@ object ErrorRendering { val span = token.unit._1 showForConsole("Cannot resolve import", span, "Cannot resolve import" :: Nil) case CycleError(modules) => - s"Cycle loops detected in imports: ${modules.map(_.file.fileName)}" + val cycleFileNames = ( + modules.toChain.toList :+ modules.head + ).map(_.file.fileName) + val message = cycleFileNames + .sliding(2) + .collect { case prev :: next :: Nil => + s"$prev imports $next" + } + .mkString(", ") + s"Cycle loops detected in imports: $message" case CompileError(err) => err match { case RulesViolated(token, messages) => diff --git a/linker/src/main/scala/aqua/linker/Linker.scala b/linker/src/main/scala/aqua/linker/Linker.scala index 8171db06..5afdbd25 100644 --- a/linker/src/main/scala/aqua/linker/Linker.scala +++ b/linker/src/main/scala/aqua/linker/Linker.scala @@ -2,22 +2,91 @@ package aqua.linker import cats.data.{NonEmptyChain, Validated, ValidatedNec} import cats.kernel.{Monoid, Semigroup} -import cats.syntax.semigroup._ +import cats.syntax.semigroup.* +import cats.syntax.validated.* +import cats.syntax.functor.* +import cats.instances.list.* import scribe.Logging import scala.annotation.tailrec object Linker extends Logging { + // Dependency Cycle, prev element import next + // and last imports head + type DepCycle[I] = NonEmptyChain[I] + + /** + * Find dependecy cycles in modules + * + * @param mods Modules + * @return [[List]] of dependecy cycles found + */ + private def findDepCycles[I, E, T]( + mods: List[AquaModule[I, E, T => T]] + ): List[DepCycle[AquaModule[I, E, T => T]]] = { + val modsIds = mods.map(_.id).toSet + // Limit search to only passed modules (there maybe dependencies not from `mods`) + val deps = mods.map(m => m.id -> m.dependsOn.keySet.intersect(modsIds)).toMap + + // DFS traversal of dependency graph + @tailrec + def findCycles( + paths: List[NonEmptyChain[I]], + visited: Set[I], + result: List[DepCycle[I]] + ): List[DepCycle[I]] = paths match { + case Nil => result + case path :: otherPaths => + val pathDeps = deps.get(path.last).toList.flatten + val cycles = pathDeps.flatMap(dep => + NonEmptyChain + .fromChain( + // This is slow + path.toChain.dropWhile(_ != dep) + ) + .toList + ) + val newPaths = pathDeps + .filterNot(visited.contains) + .map(path :+ _) ++ otherPaths + + findCycles( + paths = newPaths, + visited = visited ++ pathDeps, + result = cycles ++ result + ) + } + + val cycles = mods + .flatMap(m => + findCycles( + paths = NonEmptyChain.one(m.id) :: Nil, + visited = Set.empty, + result = List.empty + ) + ) + .distinctBy( + // This is really slow, but there + // should not be a lot of cycles + _.toChain.toList.toSet + ) + + val modsById = mods.fproductLeft(_.id).toMap + + // This should be safe + cycles.map(_.map(modsById)) + } + @tailrec def iter[I, E, T: Semigroup]( mods: List[AquaModule[I, E, T => T]], proc: Map[I, T => T], - cycleError: List[AquaModule[I, E, T => T]] => E - ): Either[E, Map[I, T => T]] = + cycleError: DepCycle[AquaModule[I, E, T => T]] => E + ): ValidatedNec[E, Map[I, T => T]] = mods match { case Nil => - Right(proc) + proc.valid case _ => val (canHandle, postpone) = mods.partition(_.dependsOn.keySet.forall(proc.contains)) logger.debug("ITERATE, can handle: " + canHandle.map(_.id)) @@ -25,9 +94,15 @@ object Linker extends Logging { logger.debug(s"postpone = ${postpone.map(_.id)}") logger.debug(s"proc = ${proc.keySet}") - if (canHandle.isEmpty && postpone.nonEmpty) - Left(cycleError(postpone)) - else { + if (canHandle.isEmpty && postpone.nonEmpty) { + findDepCycles(postpone) + .map(cycleError) + .invalid + .leftMap( + // This should be safe as cycles should exist at this moment + errs => NonEmptyChain.fromSeq(errs).get + ) + } else { val folded = canHandle.foldLeft(proc) { case (acc, m) => val importKeys = m.dependsOn.keySet logger.debug(s"${m.id} dependsOn $importKeys") @@ -52,19 +127,14 @@ object Linker extends Logging { def link[I, E, T: Semigroup]( modules: Modules[I, E, T => T], - cycleError: List[AquaModule[I, E, T => T]] => E, + cycleError: DepCycle[AquaModule[I, E, T => T]] => E, empty: I => T ): ValidatedNec[E, Map[I, T]] = if (modules.dependsOn.nonEmpty) Validated.invalid(modules.dependsOn.values.reduce(_ ++ _)) else { - val result = iter(modules.loaded.values.toList, Map.empty[I, T => T], cycleError) + val result = iter(modules.loaded.values.toList, Map.empty, cycleError) - Validated.fromEither( - result - .map(_.collect { case (i, f) if modules.exports(i) => i -> f(empty(i)) }) - .left - .map(NonEmptyChain.one) - ) + result.map(_.collect { case (i, f) if modules.exports(i) => i -> f(empty(i)) }) } } diff --git a/linker/src/test/scala/aqua/linker/LinkerSpec.scala b/linker/src/test/scala/aqua/linker/LinkerSpec.scala index 959f226c..065a5ee5 100644 --- a/linker/src/test/scala/aqua/linker/LinkerSpec.scala +++ b/linker/src/test/scala/aqua/linker/LinkerSpec.scala @@ -14,10 +14,10 @@ class LinkerSpec extends AnyFlatSpec with Matchers { empty .add( AquaModule[String, String, String => String]( - "mod1", - Map.empty, - Map("mod2" -> "unresolved mod2 in mod1"), - _ ++ " | mod1" + id = "mod1", + imports = Map.empty, + dependsOn = Map("mod2" -> "unresolved mod2 in mod1"), + body = _ ++ " | mod1" ), toExport = true ) @@ -25,7 +25,7 @@ class LinkerSpec extends AnyFlatSpec with Matchers { Linker.link[String, String, String]( withMod1, - cycle => cycle.map(_.id).mkString(" -> "), + cycle => cycle.map(_.id).toChain.toList.mkString(" -> "), _ => "" ) should be(Validated.invalidNec("unresolved mod2 in mod1")) @@ -36,7 +36,7 @@ class LinkerSpec extends AnyFlatSpec with Matchers { Linker.link[String, String, String]( withMod2, - cycle => cycle.map(_.id + "?").mkString(" -> "), + cycle => cycle.map(_.id + "?").toChain.toList.mkString(" -> "), _ => "" ) should be(Validated.validNec(Map("mod1" -> " | mod2 | mod1"))) }