From a30ba0a7a3417e81efc95bacc5379d957d867065 Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 15 Sep 2024 13:24:24 -0400 Subject: [PATCH 01/11] Move existing sources to `src-2` directory. --- .../acyclic/plugin/DependencyExtraction.scala | 0 acyclic/{src => src-2}/acyclic/plugin/Plugin.scala | 0 acyclic/{src => src-2}/acyclic/plugin/PluginPhase.scala | 2 +- acyclic/src/acyclic/package.scala | 2 +- acyclic/src/acyclic/plugin/GraphAnalysis.scala | 7 ++----- acyclic/test/src-2/acyclic/CycleTests.scala | 3 +++ .../src/acyclic/{CycleTests.scala => BaseCycleTests.scala} | 6 +++--- acyclic/test/src/acyclic/TestUtils.scala | 2 +- build.sc | 2 +- 9 files changed, 12 insertions(+), 12 deletions(-) rename acyclic/{src => src-2}/acyclic/plugin/DependencyExtraction.scala (100%) rename acyclic/{src => src-2}/acyclic/plugin/Plugin.scala (100%) rename acyclic/{src => src-2}/acyclic/plugin/PluginPhase.scala (99%) create mode 100644 acyclic/test/src-2/acyclic/CycleTests.scala rename acyclic/test/src/acyclic/{CycleTests.scala => BaseCycleTests.scala} (93%) diff --git a/acyclic/src/acyclic/plugin/DependencyExtraction.scala b/acyclic/src-2/acyclic/plugin/DependencyExtraction.scala similarity index 100% rename from acyclic/src/acyclic/plugin/DependencyExtraction.scala rename to acyclic/src-2/acyclic/plugin/DependencyExtraction.scala diff --git a/acyclic/src/acyclic/plugin/Plugin.scala b/acyclic/src-2/acyclic/plugin/Plugin.scala similarity index 100% rename from acyclic/src/acyclic/plugin/Plugin.scala rename to acyclic/src-2/acyclic/plugin/Plugin.scala diff --git a/acyclic/src/acyclic/plugin/PluginPhase.scala b/acyclic/src-2/acyclic/plugin/PluginPhase.scala similarity index 99% rename from acyclic/src/acyclic/plugin/PluginPhase.scala rename to acyclic/src-2/acyclic/plugin/PluginPhase.scala index 76e089c..d67355f 100644 --- a/acyclic/src/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-2/acyclic/plugin/PluginPhase.scala @@ -20,7 +20,7 @@ class PluginPhase( force: => Boolean, fatal: => Boolean ) extends PluginComponent - with GraphAnalysis { t => + with GraphAnalysis[Global#Tree] { t => import global._ diff --git a/acyclic/src/acyclic/package.scala b/acyclic/src/acyclic/package.scala index 43947e6..1cee541 100644 --- a/acyclic/src/acyclic/package.scala +++ b/acyclic/src/acyclic/package.scala @@ -1,4 +1,4 @@ -import scala.reflect.internal.annotations.compileTimeOnly +import scala.annotation.compileTimeOnly package object acyclic { /** diff --git a/acyclic/src/acyclic/plugin/GraphAnalysis.scala b/acyclic/src/acyclic/plugin/GraphAnalysis.scala index 98d3434..8ffcdfb 100644 --- a/acyclic/src/acyclic/plugin/GraphAnalysis.scala +++ b/acyclic/src/acyclic/plugin/GraphAnalysis.scala @@ -1,6 +1,6 @@ package acyclic.plugin + import acyclic.file -import scala.tools.nsc.Global import collection.mutable sealed trait Value { @@ -19,10 +19,7 @@ object Value { } } -trait GraphAnalysis { - val global: Global - import global._ - +trait GraphAnalysis[Tree] { case class Node[+T <: Value](value: T, dependencies: Map[Value, Seq[Tree]]) { override def toString = s"DepNode(\n $value, \n ${dependencies.keys}\n)" } diff --git a/acyclic/test/src-2/acyclic/CycleTests.scala b/acyclic/test/src-2/acyclic/CycleTests.scala new file mode 100644 index 0000000..c60e882 --- /dev/null +++ b/acyclic/test/src-2/acyclic/CycleTests.scala @@ -0,0 +1,3 @@ +package acyclic + +object CycleTests extends BaseCycleTests(new TestUtils("src-2")) diff --git a/acyclic/test/src/acyclic/CycleTests.scala b/acyclic/test/src/acyclic/BaseCycleTests.scala similarity index 93% rename from acyclic/test/src/acyclic/CycleTests.scala rename to acyclic/test/src/acyclic/BaseCycleTests.scala index 134a4dd..30f7162 100644 --- a/acyclic/test/src/acyclic/CycleTests.scala +++ b/acyclic/test/src/acyclic/BaseCycleTests.scala @@ -1,13 +1,13 @@ package acyclic import utest._ -import TestUtils.{make, makeFail} import scala.tools.nsc.util.ScalaClassLoader.URLClassLoader import acyclic.plugin.Value.{Pkg, File} import scala.collection.SortedSet import acyclic.file -object CycleTests extends TestSuite { +class BaseCycleTests(utils: TestUtils) extends TestSuite { + import utils.{make, makeFail, srcDirName} def tests = Tests { test("fail") - { @@ -53,7 +53,7 @@ object CycleTests extends TestSuite { test("innercycle") - make("success/pkg/innercycle") } } - test("self") - make("../../src", extraIncludes = Nil) + test("self") - make(s"../../$srcDirName", extraIncludes = Nil) test("force") - { test("warn") - { test("fail") - { diff --git a/acyclic/test/src/acyclic/TestUtils.scala b/acyclic/test/src/acyclic/TestUtils.scala index d33bc26..e10cc59 100644 --- a/acyclic/test/src/acyclic/TestUtils.scala +++ b/acyclic/test/src/acyclic/TestUtils.scala @@ -16,7 +16,7 @@ import javax.print.attribute.standard.Severity import scala.collection.SortedSet import scala.reflect.api.Position -object TestUtils { +class TestUtils(val srcDirName: String) { def getFilePaths(src: String): List[String] = { val f = new java.io.File(src) if (f.isDirectory) f.list.toList.flatMap(x => getFilePaths(src + "/" + x)) diff --git a/build.sc b/build.sc index 33f6ac9..f3140b4 100644 --- a/build.sc +++ b/build.sc @@ -42,7 +42,7 @@ trait AcyclicModule extends CrossScalaModule with PublishModule { override def scalacPluginIvyDeps = Deps.acyclicAgg(crossScalaVersion) object test extends ScalaTests with TestModule.Utest { - override def sources = T.sources(millSourcePath / "src", millSourcePath / "resources") + override def sources = T.sources(super.sources() :+ PathRef(millSourcePath / "resources")) override def ivyDeps = Agg( Deps.utest, Deps.scalaCompiler(crossScalaVersion) From 718865e0ddc66c87a995fe123978b21978c16a98 Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 15 Sep 2024 21:11:09 -0400 Subject: [PATCH 02/11] Port plugin to Scala 3, get tests passing. --- acyclic/resources/plugin.properties | 1 + .../acyclic/plugin/DependencyExtraction.scala | 100 +++++++++ acyclic/src-3/acyclic/plugin/Plugin.scala | 33 +++ .../src-3/acyclic/plugin/PluginPhase.scala | 205 ++++++++++++++++++ acyclic/test/src-2/acyclic/CycleTests.scala | 2 +- .../{src => src-2}/acyclic/TestUtils.scala | 15 +- acyclic/test/src-3/acyclic/CycleTests.scala | 3 + acyclic/test/src-3/acyclic/TestUtils.scala | 122 +++++++++++ acyclic/test/src/acyclic/BaseCycleTests.scala | 5 +- acyclic/test/src/acyclic/BaseTestUtils.scala | 30 +++ build.sc | 16 +- 11 files changed, 512 insertions(+), 20 deletions(-) create mode 100644 acyclic/resources/plugin.properties create mode 100644 acyclic/src-3/acyclic/plugin/DependencyExtraction.scala create mode 100644 acyclic/src-3/acyclic/plugin/Plugin.scala create mode 100644 acyclic/src-3/acyclic/plugin/PluginPhase.scala rename acyclic/test/{src => src-2}/acyclic/TestUtils.scala (86%) create mode 100644 acyclic/test/src-3/acyclic/CycleTests.scala create mode 100644 acyclic/test/src-3/acyclic/TestUtils.scala create mode 100644 acyclic/test/src/acyclic/BaseTestUtils.scala diff --git a/acyclic/resources/plugin.properties b/acyclic/resources/plugin.properties new file mode 100644 index 0000000..ff75cfc --- /dev/null +++ b/acyclic/resources/plugin.properties @@ -0,0 +1 @@ +pluginClass=acyclic.plugin.RuntimePlugin diff --git a/acyclic/src-3/acyclic/plugin/DependencyExtraction.scala b/acyclic/src-3/acyclic/plugin/DependencyExtraction.scala new file mode 100644 index 0000000..cab9d67 --- /dev/null +++ b/acyclic/src-3/acyclic/plugin/DependencyExtraction.scala @@ -0,0 +1,100 @@ +package acyclic.plugin + +import acyclic.file +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.{CompilationUnit, report} +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Flags +import dotty.tools.dotc.core.Names.Name +import dotty.tools.dotc.core.Symbols.Symbol +import dotty.tools.dotc.core.Types.Type + +object DependencyExtraction { + def apply(unit: CompilationUnit)(using Context): Seq[(Symbol, tpd.Tree)] = { + + class CollectTypeTraverser[T](pf: PartialFunction[Type, T]) extends tpd.TreeAccumulator[List[T]] { + def apply(acc: List[T], tree: tpd.Tree)(using Context) = + foldOver( + if (pf.isDefinedAt(tree.tpe)) pf(tree.tpe) :: acc else acc, + tree + ) + } + + abstract class ExtractDependenciesTraverser extends tpd.TreeTraverser { + protected val depBuf = collection.mutable.ArrayBuffer.empty[(Symbol, tpd.Tree)] + protected def addDependency(sym: Symbol, tree: tpd.Tree): Unit = depBuf += ((sym, tree)) + def dependencies: collection.immutable.Set[(Symbol, tpd.Tree)] = { + // convert to immutable set and remove NoSymbol if we have one + depBuf.toSet + } + + } + + class ExtractDependenciesByMemberRefTraverser extends ExtractDependenciesTraverser { + override def traverse(tree: tpd.Tree)(using Context): Unit = { + tree match { + case i @ tpd.Import(expr, selectors) => + selectors.foreach { s => + def lookupImported(name: Name) = expr.symbol.info.member(name).symbol + + if (s.isWildcard) { + addDependency(lookupImported(s.name.toTermName), tree) + addDependency(lookupImported(s.name.toTypeName), tree) + } + } + case select: tpd.Select => + addDependency(select.symbol, tree) + /* + * Idents are used in number of situations: + * - to refer to local variable + * - to refer to a top-level package (other packages are nested selections) + * - to refer to a term defined in the same package as an enclosing class; + * this looks fishy, see this thread: + * https://groups.google.com/d/topic/scala-internals/Ms9WUAtokLo/discussion + */ + case ident: tpd.Ident => + addDependency(ident.symbol, tree) + case typeTree: tpd.TypeTree => + val typeSymbolCollector = new CollectTypeTraverser({ + case tpe if tpe != null && tpe.typeSymbol != null && !tpe.typeSymbol.is(Flags.Package) => tpe.typeSymbol + }) + val deps = typeSymbolCollector(Nil, typeTree).toSet + deps.foreach(addDependency(_, tree)) + case t: tpd.Template => + traverse(t.body) + case other => () + } + foldOver((), tree) + } + } + + def byMembers(): collection.immutable.Set[(Symbol, tpd.Tree)] = { + val traverser = new ExtractDependenciesByMemberRefTraverser + if (!unit.isJava) + traverser.traverse(unit.tpdTree) + traverser.dependencies + } + + class ExtractDependenciesByInheritanceTraverser extends ExtractDependenciesTraverser { + override def traverse(tree: tpd.Tree)(using Context): Unit = tree match { + case t: tpd.Template => + // we are using typeSymbol and not typeSymbolDirect because we want + // type aliases to be expanded + val parentTypeSymbols = t.parents.map(parent => parent.tpe.typeSymbol).toSet + report.debuglog("Parent type symbols for " + tree.sourcePos.show + ": " + parentTypeSymbols.map(_.fullName)) + parentTypeSymbols.foreach(addDependency(_, tree)) + traverse(t.body) + case tree => foldOver((), tree) + } + } + + def byInheritence(): collection.immutable.Set[(Symbol, tpd.Tree)] = { + val traverser = new ExtractDependenciesByInheritanceTraverser + if (!unit.isJava) + traverser.traverse(unit.tpdTree) + traverser.dependencies + } + + (byMembers() | byInheritence()).toSeq + } +} diff --git a/acyclic/src-3/acyclic/plugin/Plugin.scala b/acyclic/src-3/acyclic/plugin/Plugin.scala new file mode 100644 index 0000000..90fa92c --- /dev/null +++ b/acyclic/src-3/acyclic/plugin/Plugin.scala @@ -0,0 +1,33 @@ +package acyclic.plugin + +import acyclic.file +import dotty.tools.dotc.plugins.{PluginPhase, StandardPlugin} +import scala.collection.SortedSet +import dotty.tools.dotc.core.Contexts.Context + +class RuntimePlugin extends TestPlugin() +class TestPlugin(cycleReporter: Seq[(Value, SortedSet[Int])] => Unit = _ => ()) extends StandardPlugin { + + val name = "acyclic" + val description = "Allows the developer to prohibit inter-file dependencies" + + var force = false + var fatal = true + + private class Phase() extends PluginPhase { + val phaseName = "acyclic" + override val runsBefore = Set("patternMatcher") + + override def run(using Context): Unit = new acyclic.plugin.PluginPhase(cycleReporter, force, fatal).run() + } + + override def init(options: List[String]): List[PluginPhase] = { + if (options.contains("force")) { + force = true + } + if (options.contains("warn")) { + fatal = false + } + List(Phase()) + } +} diff --git a/acyclic/src-3/acyclic/plugin/PluginPhase.scala b/acyclic/src-3/acyclic/plugin/PluginPhase.scala new file mode 100644 index 0000000..aed003a --- /dev/null +++ b/acyclic/src-3/acyclic/plugin/PluginPhase.scala @@ -0,0 +1,205 @@ +package acyclic.plugin + +import acyclic.file +import scala.collection.{SortedSet, mutable} +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.{CompilationUnit, report} +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.core.Symbols.NoSymbol +import dotty.tools.dotc.util.NoSource + +/** + * - Break dependency graph into strongly connected components + * - Turn acyclic packages into virtual "files" in the dependency graph, as + * aggregates of all the files within them + * - Any strongly connected component which includes an acyclic.file or + * acyclic.pkg is a failure + * - Pick an arbitrary cycle and report it + * - Don't report more than one cycle per file/pkg, to avoid excessive spam + */ +class PluginPhase( + cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, + force: => Boolean, + fatal: => Boolean +)(using ctx: Context) extends GraphAnalysis[tpd.Tree] { t => + + private val pkgNameAccumulator = new tpd.TreeAccumulator[List[String]] { + @annotation.tailrec + private def definitivePackageDef(pkg: tpd.PackageDef): tpd.PackageDef = + pkg.stats.collectFirst { case p: tpd.PackageDef => p } match { + case Some(p) => definitivePackageDef(p) + case None => pkg + } + + def apply(acc: List[String], tree: tpd.Tree)(using Context) = tree match { + case p: tpd.PackageDef => definitivePackageDef(p).pid.show :: acc + case _ => foldOver(acc, tree) + } + } + + private def pkgName(unit: CompilationUnit) = + pkgNameAccumulator(Nil, unit.tpdTree).reverse.flatMap(_.split('.')) + + private lazy val units = Option(ctx.run) match { + case Some(run) => run.units.toSeq.sortBy(_.source.content.mkString.hashCode()) + case None => Seq() + } + + private def hasImport(selector: String, tree: tpd.Tree): Boolean = { + val accumulator = new tpd.TreeAccumulator[Boolean] { + def apply(acc: Boolean, tree: tpd.Tree)(using Context): Boolean = tree match { + case tpd.Import(expr, List(sel)) => + acc || (expr.symbol.toString == "object acyclic" && sel.name.show == selector) + case _ => foldOver(acc, tree) + } + } + + accumulator(false, tree) + } + + private val pkgObjectAccumulator = new tpd.TreeAccumulator[List[tpd.Tree]] { + def apply(acc: List[tpd.Tree], tree: tpd.Tree)(using Context): List[tpd.Tree] = + foldOver( + if (tree.symbol.isPackageObject) tree :: acc else acc, + tree + ) + } + + def findAcyclics() = { + val acyclicNodePaths = for { + unit <- units if hasImport("file", unit.tpdTree) + } yield { + Value.File(unit.source.path, pkgName(unit)) + } + val skipNodePaths = for { + unit <- units if hasImport("skipped", unit.tpdTree) + } yield { + Value.File(unit.source.path, pkgName(unit)) + } + + val acyclicPkgNames = for { + unit <- units + pkgObject <- pkgObjectAccumulator(Nil, unit.tpdTree).reverse + if hasImport("pkg", pkgObject) + } yield { + Value.Pkg( + pkgObject.symbol + .enclosingPackageClass + .fullName + .toString + .split('.') + .toList + ) + } + (skipNodePaths, acyclicNodePaths, acyclicPkgNames) + } + + def run(): Unit = { + val unitMap = units.map(u => u.source.path -> u).toMap + val nodes = for (unit <- units) yield { + + val deps = DependencyExtraction(unit) + + val connections = for { + (sym, tree) <- deps + if sym != NoSymbol + if sym.source != null + if sym.source != NoSource + if sym.source.path != unit.source.path + if unitMap.contains(sym.source.path) + } yield (sym.source.path, tree) + + Node[Value.File]( + Value.File(unit.source.path, pkgName(unit)), + connections.groupBy(c => Value.File(c._1, pkgName(unitMap(c._1))): Value) + .mapValues(_.map(_._2)) + .toMap + ) + } + + val nodeMap = nodes.map(n => n.value -> n).toMap + + val (skipNodePaths, acyclicFiles, acyclicPkgs) = findAcyclics() + + val allAcyclics = acyclicFiles ++ acyclicPkgs + + // synthetic nodes for packages, which aggregate the dependencies of + // their contents + val pkgNodes = acyclicPkgs.map { value => + Node( + value, + nodes.filter(_.value.pkg.startsWith(value.pkg)) + .flatMap(_.dependencies.toSeq) + .groupBy(_._1) + .mapValues(_.flatMap(_._2)) + .toMap + ) + } + + val linkedNodes: Seq[DepNode] = (nodes ++ pkgNodes).map { d => + val extraLinks = d.dependencies.flatMap { + case (value: Value.File, pos) => + for { + acyclicPkg <- acyclicPkgs + if nodeMap(value).value.pkg.startsWith(acyclicPkg.pkg) + if !d.value.pkg.startsWith(acyclicPkg.pkg) + } yield (acyclicPkg, pos) + + case (_: Value.Pkg, _) => Nil + } + d.copy(dependencies = d.dependencies ++ extraLinks) + } + + // only care about cycles with size > 1 here + val components = DepNode.stronglyConnectedComponents(linkedNodes) + .filter(_.size > 1) + + val usedNodes = mutable.Set.empty[DepNode] + for { + c <- components + n <- c + if !usedNodes.contains(n) + if (!force && allAcyclics.contains(n.value)) || (force && !skipNodePaths.contains(n.value)) + } { + val cycle = DepNode.smallestCycle(n, c) + val cycleInfo = + (cycle :+ cycle.head).sliding(2) + .map { case Seq(a, b) => (a.value, a.dependencies(b.value)) } + .toSeq + cycleReporter( + cycleInfo.map { case (a, b) => a -> b.map(_.sourcePos.line + 1).to(SortedSet) } + ) + + val msg = "Unwanted cyclic dependency" + if (fatal) { + report.error(msg) + } else { + report.warning(msg) + } + + for (Seq((value, locs), (nextValue, _)) <- (cycleInfo :+ cycleInfo.head).sliding(2)) { + report.inform("") + value match { + case Value.Pkg(pkg) => report.inform(s"package ${pkg.mkString(".")}") + case Value.File(_, _) => + } + + report.echo("", locs.head.srcPos) + + val otherLines = locs.tail + .map(_.sourcePos.line + 1) + .filter(_ != locs.head.sourcePos.line + 1) + + report.inform("symbol: " + locs.head.symbol.toString) + + if (!otherLines.isEmpty) { + report.inform("More dependencies at lines " + otherLines.mkString(" ")) + } + + } + report.inform("") + usedNodes ++= cycle + } + } + +} diff --git a/acyclic/test/src-2/acyclic/CycleTests.scala b/acyclic/test/src-2/acyclic/CycleTests.scala index c60e882..fd85c1f 100644 --- a/acyclic/test/src-2/acyclic/CycleTests.scala +++ b/acyclic/test/src-2/acyclic/CycleTests.scala @@ -1,3 +1,3 @@ package acyclic -object CycleTests extends BaseCycleTests(new TestUtils("src-2")) +object CycleTests extends BaseCycleTests(TestUtils) diff --git a/acyclic/test/src/acyclic/TestUtils.scala b/acyclic/test/src-2/acyclic/TestUtils.scala similarity index 86% rename from acyclic/test/src/acyclic/TestUtils.scala rename to acyclic/test/src-2/acyclic/TestUtils.scala index e10cc59..6ac2787 100644 --- a/acyclic/test/src/acyclic/TestUtils.scala +++ b/acyclic/test/src-2/acyclic/TestUtils.scala @@ -14,14 +14,9 @@ import acyclic.plugin.Value import java.io.OutputStream import javax.print.attribute.standard.Severity import scala.collection.SortedSet -import scala.reflect.api.Position -class TestUtils(val srcDirName: String) { - def getFilePaths(src: String): List[String] = { - val f = new java.io.File(src) - if (f.isDirectory) f.list.toList.flatMap(x => getFilePaths(src + "/" + x)) - else List(src) - } +object TestUtils extends BaseTestUtils { + val srcDirName: String = "src-2" /** * Attempts to compile a resource folder as a compilation run, in order @@ -33,7 +28,7 @@ class TestUtils(val srcDirName: String) { force: Boolean = false, warn: Boolean = false, collectInfo: Boolean = true - ): Seq[(Position, String, String)] = { + ): Seq[(String, String)] = { val src = "acyclic/test/resources/" + path val sources = getFilePaths(src) ++ extraIncludes @@ -80,7 +75,7 @@ class TestUtils(val srcDirName: String) { if (vd.toList.isEmpty) throw CompilationException(cycles.get) - storeReporter.map(_.infos.toSeq.map(i => (i.pos, i.msg, i.severity.toString))).getOrElse(Seq.empty) + storeReporter.map(_.infos.toSeq.map(i => (i.msg, i.severity.toString))).getOrElse(Seq.empty) } def makeFail(path: String, force: Boolean = false)(expected: Seq[(Value, SortedSet[Int])]*) = { @@ -112,6 +107,4 @@ class TestUtils(val srcDirName: String) { assert(fullExpected.forall(cycles.contains)) } - case class CompilationException(cycles: Seq[Seq[(Value, SortedSet[Int])]]) extends Exception - } diff --git a/acyclic/test/src-3/acyclic/CycleTests.scala b/acyclic/test/src-3/acyclic/CycleTests.scala new file mode 100644 index 0000000..fd85c1f --- /dev/null +++ b/acyclic/test/src-3/acyclic/CycleTests.scala @@ -0,0 +1,3 @@ +package acyclic + +object CycleTests extends BaseCycleTests(TestUtils) diff --git a/acyclic/test/src-3/acyclic/TestUtils.scala b/acyclic/test/src-3/acyclic/TestUtils.scala new file mode 100644 index 0000000..2a46e7f --- /dev/null +++ b/acyclic/test/src-3/acyclic/TestUtils.scala @@ -0,0 +1,122 @@ +package acyclic + +import acyclic.plugin.Value +import java.io.OutputStream +import javax.print.attribute.standard.Severity +import scala.collection.SortedSet +import dotty.tools.io.{ClassPath, Path, PlainFile, VirtualDirectory} +import dotty.tools.dotc.Compiler +import dotty.tools.dotc.config.ScalaSettings +import dotty.tools.dotc.core.Contexts.{Context, ContextBase, FreshContext, NoContext} +import dotty.tools.dotc.interfaces.Diagnostic.{ERROR, INFO, WARNING} +import dotty.tools.dotc.plugins.Plugin +import dotty.tools.dotc.reporting.{ConsoleReporter, StoreReporter} +import java.net.URLClassLoader +import java.nio.file.Paths +import utest._ +import utest.asserts._ + +object TestUtils extends BaseTestUtils { + val srcDirName: String = "src-3" + + /** + * Attempts to compile a resource folder as a compilation run, in order + * to test whether it succeeds or fails correctly. + */ + def make( + path: String, + extraIncludes: Seq[String] = Seq("acyclic/src/acyclic/package.scala"), + force: Boolean = false, + warn: Boolean = false, + collectInfo: Boolean = true + ): Seq[(String, String)] = { + val src = "acyclic/test/resources/" + path + val sources = (getFilePaths(src) ++ extraIncludes).map(f => PlainFile(Path(Paths.get(f)))) + val vd = new VirtualDirectory("(memory)", None) + val loader = getClass.getClassLoader.asInstanceOf[URLClassLoader] + val entries = loader.getURLs map (_.getPath) + + val scalaSettings = new ScalaSettings + val settingsState1 = scalaSettings.outputDir.updateIn(scalaSettings.defaultState, vd) + val settingsState2 = scalaSettings.classpath.updateIn(settingsState1, ClassPath.join(entries*)) + + val opts = List( + if (force) Seq("force") else Seq(), + if (warn) Seq("warn") else Seq() + ).flatten + + val settingsState3 = if (opts.nonEmpty) { + val options = opts.map("acyclic:" + _) + println("options: " + options) + scalaSettings.pluginOptions.updateIn(settingsState2, options) + } else { + settingsState2 + } + + var cycles: Option[Seq[Seq[(Value, SortedSet[Int])]]] = None + val storeReporter = if (collectInfo) Some(new StoreReporter()) else None + + val ctxBase = new ContextBase { + override val initialCtx: Context = FreshContext.initial(NoContext.base, settings) + + override protected def loadRoughPluginsList(using Context): List[Plugin] = + List(new plugin.TestPlugin(foundCycles => + cycles = cycles match { + case None => Some(Seq(foundCycles)) + case Some(oldCycles) => Some(oldCycles :+ foundCycles) + } + )) + } + + given ctx: Context = FreshContext.initial(ctxBase, new ScalaSettings { + override val defaultState = settingsState3 + }) + .asInstanceOf[FreshContext] + .setReporter(storeReporter.getOrElse(ConsoleReporter())) + + ctx.initialize() + + val compiler = new Compiler() + val run = compiler.newRun + + run.compile(sources) + + if (vd.toList.isEmpty) throw CompilationException(cycles.get) + + storeReporter.map(_.pendingMessages.toSeq.map(i => (i.msg.message, i.level match { + case ERROR => "ERROR" + case INFO => "INFO" + case WARNING => "WARNING" + }))).getOrElse(Seq.empty) + } + + def makeFail(path: String, force: Boolean = false)(expected: Seq[(Value, SortedSet[Int])]*) = { + def canonicalize(cycle: Seq[(Value, SortedSet[Int])]): Seq[(Value, SortedSet[Int])] = { + val startIndex = cycle.indexOf(cycle.minBy(_._1.toString)) + cycle.toList.drop(startIndex) ++ cycle.toList.take(startIndex) + } + + val ex = intercept[CompilationException] { make(path, force = force, collectInfo = false) } + val cycles = ex.cycles + .map(canonicalize) + .map( + _.map { + case (Value.File(p, pkg), v) => (Value.File(p, Nil), v) + case x => x + } + ) + .toSet + + def expand(v: Value) = v match { + case Value.File(filePath, pkg) => Value.File("acyclic/test/resources/" + path + "/" + filePath, Nil) + case v => v + } + + val fullExpected = expected.map(_.map(x => x.copy(_1 = expand(x._1)))) + .map(canonicalize) + .toSet + + assert(fullExpected.forall(cycles.contains)) + } + +} diff --git a/acyclic/test/src/acyclic/BaseCycleTests.scala b/acyclic/test/src/acyclic/BaseCycleTests.scala index 30f7162..3f3ab73 100644 --- a/acyclic/test/src/acyclic/BaseCycleTests.scala +++ b/acyclic/test/src/acyclic/BaseCycleTests.scala @@ -1,12 +1,11 @@ package acyclic import utest._ -import scala.tools.nsc.util.ScalaClassLoader.URLClassLoader import acyclic.plugin.Value.{Pkg, File} import scala.collection.SortedSet import acyclic.file -class BaseCycleTests(utils: TestUtils) extends TestSuite { +class BaseCycleTests(utils: BaseTestUtils) extends TestSuite { import utils.{make, makeFail, srcDirName} def tests = Tests { @@ -58,7 +57,7 @@ class BaseCycleTests(utils: TestUtils) extends TestSuite { test("warn") - { test("fail") - { make("force/simple", force = true, warn = true).exists { - case (_, "Unwanted cyclic dependency", "warning") => true + case ("Unwanted cyclic dependency", "warning") => true case _ => false } } diff --git a/acyclic/test/src/acyclic/BaseTestUtils.scala b/acyclic/test/src/acyclic/BaseTestUtils.scala new file mode 100644 index 0000000..cab5378 --- /dev/null +++ b/acyclic/test/src/acyclic/BaseTestUtils.scala @@ -0,0 +1,30 @@ +package acyclic + +import acyclic.plugin.Value +import scala.collection.SortedSet + +abstract class BaseTestUtils { + val srcDirName: String + + /** + * Attempts to compile a resource folder as a compilation run, in order + * to test whether it succeeds or fails correctly. + */ + def make( + path: String, + extraIncludes: Seq[String] = Seq("acyclic/src/acyclic/package.scala"), + force: Boolean = false, + warn: Boolean = false, + collectInfo: Boolean = true + ): Seq[(String, String)] + + def makeFail(path: String, force: Boolean = false)(expected: Seq[(Value, SortedSet[Int])]*): Unit + + case class CompilationException(cycles: Seq[Seq[(Value, SortedSet[Int])]]) extends Exception + + final def getFilePaths(src: String): List[String] = { + val f = new java.io.File(src) + if (f.isDirectory) f.list.toList.flatMap(x => getFilePaths(src + "/" + x)) + else List(src) + } +} diff --git a/build.sc b/build.sc index f3140b4..d950df3 100644 --- a/build.sc +++ b/build.sc @@ -4,20 +4,26 @@ import mill._, scalalib._, publish._ import de.tobiasroeser.mill.vcs.version.VcsVersion object Deps { + val unreleased = "2.12.20" +: 0.to(3).map("3.3." + _).toSeq + def acyclicAgg(scalaVersion: String) = { - Agg.when(!Seq("2.12.20").contains(scalaVersion) /* exclude unreleased versions, if any */ )( + Agg.when(!unreleased.contains(scalaVersion) /* exclude unreleased versions, if any */ )( ivy"com.lihaoyi:::acyclic:0.3.12" ) } - def scalaCompiler(scalaVersion: String) = ivy"org.scala-lang:scala-compiler:${scalaVersion}" + def scalaCompiler(scalaVersion: String) = + if (scalaVersion.startsWith("3.")) ivy"org.scala-lang::scala3-compiler:$scalaVersion" + else ivy"org.scala-lang:scala-compiler:$scalaVersion" + val utest = ivy"com.lihaoyi::utest:0.8.2" } val crosses = Seq("2.11.12") ++ 8.to(20).map("2.12." + _) ++ - 0.to(14).map("2.13." + _) + 0.to(14).map("2.13." + _) ++ + 0.to(3).map("3.3." + _) object acyclic extends Cross[AcyclicModule](crosses) trait AcyclicModule extends CrossScalaModule with PublishModule { @@ -36,8 +42,8 @@ trait AcyclicModule extends CrossScalaModule with PublishModule { ) ) override def compileIvyDeps = - Agg(Deps.scalaCompiler(crossScalaVersion)) ++ - Deps.acyclicAgg(crossScalaVersion) + Agg(Deps.scalaCompiler(crossScalaVersion))/* ++ + Deps.acyclicAgg(crossScalaVersion)*/ override def scalacPluginIvyDeps = Deps.acyclicAgg(crossScalaVersion) From 07310c723cb933d1905cc56046981f6e4eaba438 Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 15 Sep 2024 21:40:29 -0400 Subject: [PATCH 03/11] Support Scala 3.4.x and 3.5.0. --- acyclic/test/src-3/acyclic/TestUtils.scala | 2 +- build.sc | 23 +++++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/acyclic/test/src-3/acyclic/TestUtils.scala b/acyclic/test/src-3/acyclic/TestUtils.scala index 2a46e7f..6f3769c 100644 --- a/acyclic/test/src-3/acyclic/TestUtils.scala +++ b/acyclic/test/src-3/acyclic/TestUtils.scala @@ -36,7 +36,7 @@ object TestUtils extends BaseTestUtils { val loader = getClass.getClassLoader.asInstanceOf[URLClassLoader] val entries = loader.getURLs map (_.getPath) - val scalaSettings = new ScalaSettings + val scalaSettings = new ScalaSettings {} val settingsState1 = scalaSettings.outputDir.updateIn(scalaSettings.defaultState, vd) val settingsState2 = scalaSettings.classpath.updateIn(settingsState1, ClassPath.join(entries*)) diff --git a/build.sc b/build.sc index d950df3..2be24f9 100644 --- a/build.sc +++ b/build.sc @@ -4,7 +4,14 @@ import mill._, scalalib._, publish._ import de.tobiasroeser.mill.vcs.version.VcsVersion object Deps { - val unreleased = "2.12.20" +: 0.to(3).map("3.3." + _).toSeq + val scala211 = Seq("2.11.12") + val scala212 = 8.to(20).map("2.12." + _) + val scala213 = 0.to(14).map("2.13." + _) + val scala33 = 0.to(3).map("3.3." + _) + val scala34 = 0.to(3).map("3.4." + _) + val scala35 = 0.to(0).map("3.5." + _) + + val unreleased = Seq("2.12.20") ++ scala33 ++ scala34 ++ scala35 def acyclicAgg(scalaVersion: String) = { Agg.when(!unreleased.contains(scalaVersion) /* exclude unreleased versions, if any */ )( @@ -20,10 +27,12 @@ object Deps { } val crosses = - Seq("2.11.12") ++ - 8.to(20).map("2.12." + _) ++ - 0.to(14).map("2.13." + _) ++ - 0.to(3).map("3.3." + _) + Deps.scala211 ++ + Deps.scala212 ++ + Deps.scala213 ++ + Deps.scala33 ++ + Deps.scala34 ++ + Deps.scala35 object acyclic extends Cross[AcyclicModule](crosses) trait AcyclicModule extends CrossScalaModule with PublishModule { @@ -42,8 +51,8 @@ trait AcyclicModule extends CrossScalaModule with PublishModule { ) ) override def compileIvyDeps = - Agg(Deps.scalaCompiler(crossScalaVersion))/* ++ - Deps.acyclicAgg(crossScalaVersion)*/ + Agg(Deps.scalaCompiler(crossScalaVersion)) ++ + Deps.acyclicAgg(crossScalaVersion) override def scalacPluginIvyDeps = Deps.acyclicAgg(crossScalaVersion) From e5c63c04ef30aa7c9d1e3d8ed28dd32566556cda Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 15 Sep 2024 21:59:31 -0400 Subject: [PATCH 04/11] Lowercase severity names. --- acyclic/test/src-3/acyclic/TestUtils.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acyclic/test/src-3/acyclic/TestUtils.scala b/acyclic/test/src-3/acyclic/TestUtils.scala index 6f3769c..417135c 100644 --- a/acyclic/test/src-3/acyclic/TestUtils.scala +++ b/acyclic/test/src-3/acyclic/TestUtils.scala @@ -84,9 +84,9 @@ object TestUtils extends BaseTestUtils { if (vd.toList.isEmpty) throw CompilationException(cycles.get) storeReporter.map(_.pendingMessages.toSeq.map(i => (i.msg.message, i.level match { - case ERROR => "ERROR" - case INFO => "INFO" - case WARNING => "WARNING" + case ERROR => "error" + case INFO => "info" + case WARNING => "warning" }))).getOrElse(Seq.empty) } From 9bda417e6d49c295b0bbb153e26fa0975cfca1ee Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 15 Sep 2024 21:59:55 -0400 Subject: [PATCH 05/11] Depend on acyclic 0.3.13, remove 2.12.20 from unreleased versions. --- build.sc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.sc b/build.sc index 2be24f9..afed9c4 100644 --- a/build.sc +++ b/build.sc @@ -11,11 +11,11 @@ object Deps { val scala34 = 0.to(3).map("3.4." + _) val scala35 = 0.to(0).map("3.5." + _) - val unreleased = Seq("2.12.20") ++ scala33 ++ scala34 ++ scala35 + val unreleased = scala33 ++ scala34 ++ scala35 def acyclicAgg(scalaVersion: String) = { Agg.when(!unreleased.contains(scalaVersion) /* exclude unreleased versions, if any */ )( - ivy"com.lihaoyi:::acyclic:0.3.12" + ivy"com.lihaoyi:::acyclic:0.3.13" ) } From 545b931d8b5f290c70df2912356eef6445b15bf2 Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 15 Sep 2024 22:37:41 -0400 Subject: [PATCH 06/11] Add `assert` to `force.warn.fail` test, use uppercase strings. --- acyclic/test/src-3/acyclic/TestUtils.scala | 6 +++--- acyclic/test/src/acyclic/BaseCycleTests.scala | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/acyclic/test/src-3/acyclic/TestUtils.scala b/acyclic/test/src-3/acyclic/TestUtils.scala index 417135c..6f3769c 100644 --- a/acyclic/test/src-3/acyclic/TestUtils.scala +++ b/acyclic/test/src-3/acyclic/TestUtils.scala @@ -84,9 +84,9 @@ object TestUtils extends BaseTestUtils { if (vd.toList.isEmpty) throw CompilationException(cycles.get) storeReporter.map(_.pendingMessages.toSeq.map(i => (i.msg.message, i.level match { - case ERROR => "error" - case INFO => "info" - case WARNING => "warning" + case ERROR => "ERROR" + case INFO => "INFO" + case WARNING => "WARNING" }))).getOrElse(Seq.empty) } diff --git a/acyclic/test/src/acyclic/BaseCycleTests.scala b/acyclic/test/src/acyclic/BaseCycleTests.scala index 3f3ab73..1ee2cd3 100644 --- a/acyclic/test/src/acyclic/BaseCycleTests.scala +++ b/acyclic/test/src/acyclic/BaseCycleTests.scala @@ -56,10 +56,10 @@ class BaseCycleTests(utils: BaseTestUtils) extends TestSuite { test("force") - { test("warn") - { test("fail") - { - make("force/simple", force = true, warn = true).exists { - case ("Unwanted cyclic dependency", "warning") => true + assert(make("force/simple", force = true, warn = true).exists { + case ("Unwanted cyclic dependency", "WARNING") => true case _ => false - } + }) } } test("fail") - makeFail("force/simple", force = true)(Seq( From ddddff6522f493ce431e720ee01a01d74066376d Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Mon, 16 Sep 2024 08:25:09 -0400 Subject: [PATCH 07/11] Share `run` logic between scala 2 and 3. --- .../src-2/acyclic/plugin/PluginPhase.scala | 169 ++++++------------ acyclic/src-3/acyclic/plugin/Compat.scala | 5 + .../src-3/acyclic/plugin/PluginPhase.scala | 105 ++--------- .../src/acyclic/plugin/BasePluginPhase.scala | 106 +++++++++++ .../src/acyclic/plugin/GraphAnalysis.scala | 14 +- 5 files changed, 185 insertions(+), 214 deletions(-) create mode 100644 acyclic/src-3/acyclic/plugin/Compat.scala create mode 100644 acyclic/src/acyclic/plugin/BasePluginPhase.scala diff --git a/acyclic/src-2/acyclic/plugin/PluginPhase.scala b/acyclic/src-2/acyclic/plugin/PluginPhase.scala index d67355f..347b49a 100644 --- a/acyclic/src-2/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-2/acyclic/plugin/PluginPhase.scala @@ -19,8 +19,7 @@ class PluginPhase( cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, force: => Boolean, fatal: => Boolean -) extends PluginComponent - with GraphAnalysis[Global#Tree] { t => +) extends PluginComponent { t => import global._ @@ -29,6 +28,7 @@ class PluginPhase( override val runsBefore = List("patmat") val phaseName = "acyclic" + def pkgName(unit: CompilationUnit) = { unit.body .collect { case x: PackageDef => x.pid.toString } @@ -40,42 +40,56 @@ class PluginPhase( .toSeq .sortBy(_.source.content.mkString.hashCode()) - def findAcyclics() = { - val acyclicNodePaths = for { - unit <- units - if unit.body.children.collect { - case Import(expr, List(sel)) => - expr.symbol.toString == "package acyclic" && sel.name.toString == "file" - }.exists(x => x) - } yield { - Value.File(unit.source.path, pkgName(unit)) - } - val skipNodePaths = for { - unit <- units - if unit.body.children.collect { - case Import(expr, List(sel)) => - expr.symbol.toString == "package acyclic" && sel.name.toString == "skipped" - }.exists(x => x) - } yield { - Value.File(unit.source.path, pkgName(unit)) - } + private object base extends BasePluginPhase[Tree] with GraphAnalysis[Tree] { + protected val cycleReporter = t.cycleReporter + protected lazy val force = t.force + protected lazy val fatal = t.fatal + + def treeLine(tree: Tree): Int = tree.pos.line + def treeSymbolString(tree: Tree): String = tree.symbol.toString + + def reportError(msg: String): Unit = global.error(msg) + def reportWarning(msg: String): Unit = global.warning(msg) + def reportInform(msg: String): Unit = global.inform(msg) + def reportEcho(msg: String, tree: Tree): Unit = global.reporter.echo(tree.pos, msg) + + def findAcyclics() = { + val acyclicNodePaths = for { + unit <- units + if unit.body.children.collect { + case Import(expr, List(sel)) => + expr.symbol.toString == "package acyclic" && sel.name.toString == "file" + }.exists(x => x) + } yield { + Value.File(unit.source.path, pkgName(unit)) + } + val skipNodePaths = for { + unit <- units + if unit.body.children.collect { + case Import(expr, List(sel)) => + expr.symbol.toString == "package acyclic" && sel.name.toString == "skipped" + }.exists(x => x) + } yield { + Value.File(unit.source.path, pkgName(unit)) + } - val acyclicPkgNames = for { - unit <- units - pkgObject <- unit.body.collect { case x: ModuleDef if x.name.toString == "package" => x } - if pkgObject.impl.children.collect { case Import(expr, List(sel)) => - expr.symbol.toString == "package acyclic" && sel.name.toString == "pkg" - }.exists(x => x) - } yield { - Value.Pkg( - pkgObject.symbol - .enclosingPackageClass - .fullName - .split('.') - .toList - ) + val acyclicPkgNames = for { + unit <- units + pkgObject <- unit.body.collect { case x: ModuleDef if x.name.toString == "package" => x } + if pkgObject.impl.children.collect { case Import(expr, List(sel)) => + expr.symbol.toString == "package acyclic" && sel.name.toString == "pkg" + }.exists(x => x) + } yield { + Value.Pkg( + pkgObject.symbol + .enclosingPackageClass + .fullName + .split('.') + .toList + ) + } + (skipNodePaths, acyclicNodePaths, acyclicPkgNames) } - (skipNodePaths, acyclicNodePaths, acyclicPkgNames) } override def newPhase(prev: Phase): Phase = new Phase(prev) { @@ -92,7 +106,7 @@ class PluginPhase( if sym.sourceFile.path != unit.source.path } yield (sym.sourceFile.path, tree) - Node[Value.File]( + Node[Value.File, Tree]( Value.File(unit.source.path, pkgName(unit)), connections.groupBy(c => Value.File(c._1, pkgName(unitMap(c._1))): Value) .mapValues(_.map(_._2)) @@ -100,88 +114,9 @@ class PluginPhase( ) } - val nodeMap = nodes.map(n => n.value -> n).toMap - - val (skipNodePaths, acyclicFiles, acyclicPkgs) = findAcyclics() - - val allAcyclics = acyclicFiles ++ acyclicPkgs - - // synthetic nodes for packages, which aggregate the dependencies of - // their contents - val pkgNodes = acyclicPkgs.map { value => - Node( - value, - nodes.filter(_.value.pkg.startsWith(value.pkg)) - .flatMap(_.dependencies.toSeq) - .groupBy(_._1) - .mapValues(_.flatMap(_._2)) - .toMap - ) - } - - val linkedNodes: Seq[DepNode] = (nodes ++ pkgNodes).map { d => - val extraLinks = for { - (value: Value.File, pos) <- d.dependencies - acyclicPkg <- acyclicPkgs - if nodeMap(value).value.pkg.startsWith(acyclicPkg.pkg) - if !d.value.pkg.startsWith(acyclicPkg.pkg) - } yield (acyclicPkg, pos) - d.copy(dependencies = d.dependencies ++ extraLinks) - } - - // only care about cycles with size > 1 here - val components = DepNode.stronglyConnectedComponents(linkedNodes) - .filter(_.size > 1) - - val usedNodes = mutable.Set.empty[DepNode] - for { - c <- components - n <- c - if !usedNodes.contains(n) - if (!force && allAcyclics.contains(n.value)) || (force && !skipNodePaths.contains(n.value)) - } { - val cycle = DepNode.smallestCycle(n, c) - val cycleInfo = - (cycle :+ cycle.head).sliding(2) - .map { case Seq(a, b) => (a.value, a.dependencies(b.value)) } - .toSeq - cycleReporter( - cycleInfo.map { case (a, b) => a -> b.map(_.pos.line).to(SortedSet) } - ) - - val msg = "Unwanted cyclic dependency" - if (fatal) { - global.error(msg) - } else { - global.warning(msg) - } - - for (Seq((value, locs), (nextValue, _)) <- (cycleInfo :+ cycleInfo.head).sliding(2)) { - global.inform("") - value match { - case Value.Pkg(pkg) => global.inform(s"package ${pkg.mkString(".")}") - case Value.File(_, _) => - } - - global.reporter.echo(locs.head.pos, "") - - val otherLines = locs.tail - .map(_.pos.line) - .filter(_ != locs.head.pos.line) - - global.inform("symbol: " + locs.head.symbol.toString) - - if (!otherLines.isEmpty) { - global.inform("More dependencies at lines " + otherLines.mkString(" ")) - } - - } - global.inform("") - usedNodes ++= cycle - } + base.runOnNodes(nodes) } def name: String = "acyclic" } - } diff --git a/acyclic/src-3/acyclic/plugin/Compat.scala b/acyclic/src-3/acyclic/plugin/Compat.scala new file mode 100644 index 0000000..e195a41 --- /dev/null +++ b/acyclic/src-3/acyclic/plugin/Compat.scala @@ -0,0 +1,5 @@ +package acyclic.plugin + +import acyclic.file + +object Compat diff --git a/acyclic/src-3/acyclic/plugin/PluginPhase.scala b/acyclic/src-3/acyclic/plugin/PluginPhase.scala index aed003a..41fb7e9 100644 --- a/acyclic/src-3/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-3/acyclic/plugin/PluginPhase.scala @@ -1,7 +1,7 @@ package acyclic.plugin import acyclic.file -import scala.collection.{SortedSet, mutable} +import scala.collection.SortedSet import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.{CompilationUnit, report} import dotty.tools.dotc.core.Contexts.Context @@ -18,10 +18,18 @@ import dotty.tools.dotc.util.NoSource * - Don't report more than one cycle per file/pkg, to avoid excessive spam */ class PluginPhase( - cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, - force: => Boolean, - fatal: => Boolean -)(using ctx: Context) extends GraphAnalysis[tpd.Tree] { t => + protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, + protected val force: Boolean, + protected val fatal: Boolean +)(using ctx: Context) extends BasePluginPhase[tpd.Tree], GraphAnalysis[tpd.Tree] { + + def treeLine(tree: tpd.Tree): Int = tree.sourcePos.line + 1 + def treeSymbolString(tree: tpd.Tree): String = tree.symbol.toString + + def reportError(msg: String): Unit = report.error(msg) + def reportWarning(msg: String): Unit = report.warning(msg) + def reportInform(msg: String): Unit = report.inform(msg) + def reportEcho(msg: String, tree: tpd.Tree): Unit = report.echo(msg, tree.srcPos) private val pkgNameAccumulator = new tpd.TreeAccumulator[List[String]] { @annotation.tailrec @@ -109,7 +117,7 @@ class PluginPhase( if unitMap.contains(sym.source.path) } yield (sym.source.path, tree) - Node[Value.File]( + Node[Value.File, tpd.Tree]( Value.File(unit.source.path, pkgName(unit)), connections.groupBy(c => Value.File(c._1, pkgName(unitMap(c._1))): Value) .mapValues(_.map(_._2)) @@ -117,89 +125,6 @@ class PluginPhase( ) } - val nodeMap = nodes.map(n => n.value -> n).toMap - - val (skipNodePaths, acyclicFiles, acyclicPkgs) = findAcyclics() - - val allAcyclics = acyclicFiles ++ acyclicPkgs - - // synthetic nodes for packages, which aggregate the dependencies of - // their contents - val pkgNodes = acyclicPkgs.map { value => - Node( - value, - nodes.filter(_.value.pkg.startsWith(value.pkg)) - .flatMap(_.dependencies.toSeq) - .groupBy(_._1) - .mapValues(_.flatMap(_._2)) - .toMap - ) - } - - val linkedNodes: Seq[DepNode] = (nodes ++ pkgNodes).map { d => - val extraLinks = d.dependencies.flatMap { - case (value: Value.File, pos) => - for { - acyclicPkg <- acyclicPkgs - if nodeMap(value).value.pkg.startsWith(acyclicPkg.pkg) - if !d.value.pkg.startsWith(acyclicPkg.pkg) - } yield (acyclicPkg, pos) - - case (_: Value.Pkg, _) => Nil - } - d.copy(dependencies = d.dependencies ++ extraLinks) - } - - // only care about cycles with size > 1 here - val components = DepNode.stronglyConnectedComponents(linkedNodes) - .filter(_.size > 1) - - val usedNodes = mutable.Set.empty[DepNode] - for { - c <- components - n <- c - if !usedNodes.contains(n) - if (!force && allAcyclics.contains(n.value)) || (force && !skipNodePaths.contains(n.value)) - } { - val cycle = DepNode.smallestCycle(n, c) - val cycleInfo = - (cycle :+ cycle.head).sliding(2) - .map { case Seq(a, b) => (a.value, a.dependencies(b.value)) } - .toSeq - cycleReporter( - cycleInfo.map { case (a, b) => a -> b.map(_.sourcePos.line + 1).to(SortedSet) } - ) - - val msg = "Unwanted cyclic dependency" - if (fatal) { - report.error(msg) - } else { - report.warning(msg) - } - - for (Seq((value, locs), (nextValue, _)) <- (cycleInfo :+ cycleInfo.head).sliding(2)) { - report.inform("") - value match { - case Value.Pkg(pkg) => report.inform(s"package ${pkg.mkString(".")}") - case Value.File(_, _) => - } - - report.echo("", locs.head.srcPos) - - val otherLines = locs.tail - .map(_.sourcePos.line + 1) - .filter(_ != locs.head.sourcePos.line + 1) - - report.inform("symbol: " + locs.head.symbol.toString) - - if (!otherLines.isEmpty) { - report.inform("More dependencies at lines " + otherLines.mkString(" ")) - } - - } - report.inform("") - usedNodes ++= cycle - } + runOnNodes(nodes) } - } diff --git a/acyclic/src/acyclic/plugin/BasePluginPhase.scala b/acyclic/src/acyclic/plugin/BasePluginPhase.scala new file mode 100644 index 0000000..668cd7f --- /dev/null +++ b/acyclic/src/acyclic/plugin/BasePluginPhase.scala @@ -0,0 +1,106 @@ +package acyclic.plugin + +import acyclic.plugin.Compat._ +import scala.collection.{mutable, SortedSet} + +trait BasePluginPhase[Tree] { self: GraphAnalysis[Tree] => + protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit + protected def force: Boolean + protected def fatal: Boolean + + def treeLine(tree: Tree): Int + def treeSymbolString(tree: Tree): String + + def reportError(msg: String): Unit + def reportWarning(msg: String): Unit + def reportInform(msg: String): Unit + def reportEcho(msg: String, tree: Tree): Unit + + def findAcyclics(): (Seq[Value.File], Seq[Value.File], Seq[Value.Pkg]) + + final def runOnNodes(nodes: Seq[Node[Value.File, Tree]]): Unit = { + val nodeMap = nodes.map(n => n.value -> n).toMap + + val (skipNodePaths, acyclicFiles, acyclicPkgs) = findAcyclics() + + val allAcyclics = acyclicFiles ++ acyclicPkgs + + // synthetic nodes for packages, which aggregate the dependencies of + // their contents + val pkgNodes = acyclicPkgs.map { value => + Node( + value, + nodes.filter(_.value.pkg.startsWith(value.pkg)) + .flatMap(_.dependencies.toSeq) + .groupBy(_._1) + .mapValues(_.flatMap(_._2)) + .toMap + ) + } + + val linkedNodes: Seq[DepNode] = (nodes ++ pkgNodes).map { d => + val extraLinks = d.dependencies.flatMap { + case (value: Value.File, pos) => + for { + acyclicPkg <- acyclicPkgs + if nodeMap(value).value.pkg.startsWith(acyclicPkg.pkg) + if !d.value.pkg.startsWith(acyclicPkg.pkg) + } yield (acyclicPkg, pos) + + case (_: Value.Pkg, _) => Nil + } + d.copy(dependencies = d.dependencies ++ extraLinks) + } + + // only care about cycles with size > 1 here + val components = DepNode.stronglyConnectedComponents(linkedNodes) + .filter(_.size > 1) + + val usedNodes = mutable.Set.empty[DepNode] + for { + c <- components + n <- c + if !usedNodes.contains(n) + if (!force && allAcyclics.contains(n.value)) || (force && !skipNodePaths.contains(n.value)) + } { + val cycle = DepNode.smallestCycle(n, c) + val cycleInfo = + (cycle :+ cycle.head).sliding(2) + .map { case Seq(a, b) => (a.value, a.dependencies(b.value)) } + .toSeq + cycleReporter( + cycleInfo.map { case (a, b) => a -> b.map(treeLine).to(SortedSet) } + ) + + val msg = "Unwanted cyclic dependency" + if (fatal) { + reportError(msg) + } else { + reportWarning(msg) + } + + for (Seq((value, locs), (nextValue, _)) <- (cycleInfo :+ cycleInfo.head).sliding(2)) { + reportInform("") + value match { + case Value.Pkg(pkg) => reportInform(s"package ${pkg.mkString(".")}") + case Value.File(_, _) => + } + + reportEcho("", locs.head) + + val otherLines = locs.tail + .map(treeLine) + .filter(_ != treeLine(locs.head)) + + reportInform("symbol: " + treeSymbolString(locs.head)) + + if (!otherLines.isEmpty) { + reportInform("More dependencies at lines " + otherLines.mkString(" ")) + } + + } + reportInform("") + usedNodes ++= cycle + } + } +} diff --git a/acyclic/src/acyclic/plugin/GraphAnalysis.scala b/acyclic/src/acyclic/plugin/GraphAnalysis.scala index 8ffcdfb..50b11ad 100644 --- a/acyclic/src/acyclic/plugin/GraphAnalysis.scala +++ b/acyclic/src/acyclic/plugin/GraphAnalysis.scala @@ -19,14 +19,14 @@ object Value { } } -trait GraphAnalysis[Tree] { - case class Node[+T <: Value](value: T, dependencies: Map[Value, Seq[Tree]]) { - override def toString = s"DepNode(\n $value, \n ${dependencies.keys}\n)" - } +case class Node[+T <: Value, Tree](value: T, dependencies: Map[Value, Seq[Tree]]) { + override def toString = s"DepNode(\n $value, \n ${dependencies.keys}\n)" +} - type DepNode = Node[Value] - type FileNode = Node[Value.File] - type PkgNode = Node[Value.Pkg] +trait GraphAnalysis[Tree] { + type DepNode = Node[Value, Tree] + type FileNode = Node[Value.File, Tree] + type PkgNode = Node[Value.Pkg, Tree] object DepNode { From 225e3c15b063ea59027870d020df7a718d203ffe Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Mon, 16 Sep 2024 09:12:20 -0400 Subject: [PATCH 08/11] Share `findAcyclics` logic between scala 2 and 3. --- .../src-2/acyclic/plugin/PluginPhase.scala | 69 +++++-------------- .../src-3/acyclic/plugin/PluginPhase.scala | 67 +++++------------- .../src/acyclic/plugin/BasePluginPhase.scala | 33 +++++++-- 3 files changed, 64 insertions(+), 105 deletions(-) diff --git a/acyclic/src-2/acyclic/plugin/PluginPhase.scala b/acyclic/src-2/acyclic/plugin/PluginPhase.scala index 347b49a..300dc61 100644 --- a/acyclic/src-2/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-2/acyclic/plugin/PluginPhase.scala @@ -29,18 +29,7 @@ class PluginPhase( val phaseName = "acyclic" - def pkgName(unit: CompilationUnit) = { - unit.body - .collect { case x: PackageDef => x.pid.toString } - .flatMap(_.split('.')) - } - - def units = global.currentRun - .units - .toSeq - .sortBy(_.source.content.mkString.hashCode()) - - private object base extends BasePluginPhase[Tree] with GraphAnalysis[Tree] { + private object base extends BasePluginPhase[CompilationUnit, Tree] with GraphAnalysis[Tree] { protected val cycleReporter = t.cycleReporter protected lazy val force = t.force protected lazy val fatal = t.fatal @@ -53,49 +42,23 @@ class PluginPhase( def reportInform(msg: String): Unit = global.inform(msg) def reportEcho(msg: String, tree: Tree): Unit = global.reporter.echo(tree.pos, msg) - def findAcyclics() = { - val acyclicNodePaths = for { - unit <- units - if unit.body.children.collect { - case Import(expr, List(sel)) => - expr.symbol.toString == "package acyclic" && sel.name.toString == "file" - }.exists(x => x) - } yield { - Value.File(unit.source.path, pkgName(unit)) - } - val skipNodePaths = for { - unit <- units - if unit.body.children.collect { - case Import(expr, List(sel)) => - expr.symbol.toString == "package acyclic" && sel.name.toString == "skipped" - }.exists(x => x) - } yield { - Value.File(unit.source.path, pkgName(unit)) - } - - val acyclicPkgNames = for { - unit <- units - pkgObject <- unit.body.collect { case x: ModuleDef if x.name.toString == "package" => x } - if pkgObject.impl.children.collect { case Import(expr, List(sel)) => - expr.symbol.toString == "package acyclic" && sel.name.toString == "pkg" - }.exists(x => x) - } yield { - Value.Pkg( - pkgObject.symbol - .enclosingPackageClass - .fullName - .split('.') - .toList - ) - } - (skipNodePaths, acyclicNodePaths, acyclicPkgNames) - } + def units: Seq[CompilationUnit] = global.currentRun.units.toSeq.sortBy(_.source.content.mkString.hashCode()) + def unitTree(unit: CompilationUnit): Tree = unit.body + def unitPath(unit: CompilationUnit): String = unit.source.path + def unitPkgName(unit: CompilationUnit): List[String] = + unit.body.collect { case x: PackageDef => x.pid.toString }.flatMap(_.split('.')) + def findPkgObjects(tree: Tree): List[Tree] = tree.collect { case x: ModuleDef if x.name.toString == "package" => x } + def pkgObjectName(pkgObject: Tree): String = pkgObject.symbol.enclosingPackageClass.fullName + def hasAcyclicImport(tree: Tree, selector: String): Boolean = + tree.collect { + case Import(expr, List(sel)) => expr.symbol.toString == "package acyclic" && sel.name.toString == selector + }.exists(identity) } override def newPhase(prev: Phase): Phase = new Phase(prev) { override def run() { - val unitMap = units.map(u => u.source.path -> u).toMap - val nodes = for (unit <- units) yield { + val unitMap = base.units.map(u => u.source.path -> u).toMap + val nodes = for (unit <- base.units) yield { val deps = DependencyExtraction(t.global)(unit) @@ -107,8 +70,8 @@ class PluginPhase( } yield (sym.sourceFile.path, tree) Node[Value.File, Tree]( - Value.File(unit.source.path, pkgName(unit)), - connections.groupBy(c => Value.File(c._1, pkgName(unitMap(c._1))): Value) + Value.File(unit.source.path, base.unitPkgName(unit)), + connections.groupBy(c => Value.File(c._1, base.unitPkgName(unitMap(c._1))): Value) .mapValues(_.map(_._2)) .toMap ) diff --git a/acyclic/src-3/acyclic/plugin/PluginPhase.scala b/acyclic/src-3/acyclic/plugin/PluginPhase.scala index 41fb7e9..4a8bbe0 100644 --- a/acyclic/src-3/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-3/acyclic/plugin/PluginPhase.scala @@ -21,7 +21,7 @@ class PluginPhase( protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, protected val force: Boolean, protected val fatal: Boolean -)(using ctx: Context) extends BasePluginPhase[tpd.Tree], GraphAnalysis[tpd.Tree] { +)(using ctx: Context) extends BasePluginPhase[CompilationUnit, tpd.Tree], GraphAnalysis[tpd.Tree] { def treeLine(tree: tpd.Tree): Int = tree.sourcePos.line + 1 def treeSymbolString(tree: tpd.Tree): String = tree.symbol.toString @@ -45,26 +45,6 @@ class PluginPhase( } } - private def pkgName(unit: CompilationUnit) = - pkgNameAccumulator(Nil, unit.tpdTree).reverse.flatMap(_.split('.')) - - private lazy val units = Option(ctx.run) match { - case Some(run) => run.units.toSeq.sortBy(_.source.content.mkString.hashCode()) - case None => Seq() - } - - private def hasImport(selector: String, tree: tpd.Tree): Boolean = { - val accumulator = new tpd.TreeAccumulator[Boolean] { - def apply(acc: Boolean, tree: tpd.Tree)(using Context): Boolean = tree match { - case tpd.Import(expr, List(sel)) => - acc || (expr.symbol.toString == "object acyclic" && sel.name.show == selector) - case _ => foldOver(acc, tree) - } - } - - accumulator(false, tree) - } - private val pkgObjectAccumulator = new tpd.TreeAccumulator[List[tpd.Tree]] { def apply(acc: List[tpd.Tree], tree: tpd.Tree)(using Context): List[tpd.Tree] = foldOver( @@ -73,35 +53,26 @@ class PluginPhase( ) } - def findAcyclics() = { - val acyclicNodePaths = for { - unit <- units if hasImport("file", unit.tpdTree) - } yield { - Value.File(unit.source.path, pkgName(unit)) - } - val skipNodePaths = for { - unit <- units if hasImport("skipped", unit.tpdTree) - } yield { - Value.File(unit.source.path, pkgName(unit)) + private def hasAcyclicImportAccumulator(selector: String) = new tpd.TreeAccumulator[Boolean] { + def apply(acc: Boolean, tree: tpd.Tree)(using Context): Boolean = tree match { + case tpd.Import(expr, List(sel)) => + acc || (expr.symbol.toString == "object acyclic" && sel.name.show == selector) + case _ => foldOver(acc, tree) } + } - val acyclicPkgNames = for { - unit <- units - pkgObject <- pkgObjectAccumulator(Nil, unit.tpdTree).reverse - if hasImport("pkg", pkgObject) - } yield { - Value.Pkg( - pkgObject.symbol - .enclosingPackageClass - .fullName - .toString - .split('.') - .toList - ) - } - (skipNodePaths, acyclicNodePaths, acyclicPkgNames) + lazy val units = Option(ctx.run) match { + case Some(run) => run.units.toSeq.sortBy(_.source.content.mkString.hashCode()) + case None => Seq() } + def unitTree(unit: CompilationUnit): tpd.Tree = unit.tpdTree + def unitPath(unit: CompilationUnit): String = unit.source.path + def unitPkgName(unit: CompilationUnit): List[String] = pkgNameAccumulator(Nil, unit.tpdTree).reverse.flatMap(_.split('.')) + def findPkgObjects(tree: tpd.Tree): List[tpd.Tree] = pkgObjectAccumulator(Nil, tree).reverse + def pkgObjectName(pkgObject: tpd.Tree): String = pkgObject.symbol.enclosingPackageClass.fullName.toString + def hasAcyclicImport(tree: tpd.Tree, selector: String): Boolean = hasAcyclicImportAccumulator(selector)(false, tree) + def run(): Unit = { val unitMap = units.map(u => u.source.path -> u).toMap val nodes = for (unit <- units) yield { @@ -118,8 +89,8 @@ class PluginPhase( } yield (sym.source.path, tree) Node[Value.File, tpd.Tree]( - Value.File(unit.source.path, pkgName(unit)), - connections.groupBy(c => Value.File(c._1, pkgName(unitMap(c._1))): Value) + Value.File(unit.source.path, unitPkgName(unit)), + connections.groupBy(c => Value.File(c._1, unitPkgName(unitMap(c._1))): Value) .mapValues(_.map(_._2)) .toMap ) diff --git a/acyclic/src/acyclic/plugin/BasePluginPhase.scala b/acyclic/src/acyclic/plugin/BasePluginPhase.scala index 668cd7f..a4172e6 100644 --- a/acyclic/src/acyclic/plugin/BasePluginPhase.scala +++ b/acyclic/src/acyclic/plugin/BasePluginPhase.scala @@ -3,7 +3,7 @@ package acyclic.plugin import acyclic.plugin.Compat._ import scala.collection.{mutable, SortedSet} -trait BasePluginPhase[Tree] { self: GraphAnalysis[Tree] => +trait BasePluginPhase[CompilationUnit, Tree] { self: GraphAnalysis[Tree] => protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit protected def force: Boolean protected def fatal: Boolean @@ -16,7 +16,33 @@ trait BasePluginPhase[Tree] { self: GraphAnalysis[Tree] => def reportInform(msg: String): Unit def reportEcho(msg: String, tree: Tree): Unit - def findAcyclics(): (Seq[Value.File], Seq[Value.File], Seq[Value.Pkg]) + def units: Seq[CompilationUnit] + def unitTree(unit: CompilationUnit): Tree + def unitPath(unit: CompilationUnit): String + def unitPkgName(unit: CompilationUnit): List[String] + def findPkgObjects(tree: Tree): List[Tree] + def pkgObjectName(pkgObject: Tree): String + def hasAcyclicImport(tree: Tree, selector: String): Boolean + + final def findAcyclics(): (Seq[Value.File], Seq[Value.File], Seq[Value.Pkg]) = { + val acyclicNodePaths = for { + unit <- units if hasAcyclicImport(unitTree(unit), "file") + } yield { + Value.File(unitPath(unit), unitPkgName(unit)) + } + val skipNodePaths = for { + unit <- units if hasAcyclicImport(unitTree(unit), "skipped") + } yield { + Value.File(unitPath(unit), unitPkgName(unit)) + } + + val acyclicPkgNames = for { + unit <- units + pkgObject <- findPkgObjects(unitTree(unit)) + if hasAcyclicImport(pkgObject, "pkg") + } yield Value.Pkg(pkgObjectName(pkgObject).split('.').toList) + (skipNodePaths, acyclicNodePaths, acyclicPkgNames) + } final def runOnNodes(nodes: Seq[Node[Value.File, Tree]]): Unit = { val nodeMap = nodes.map(n => n.value -> n).toMap @@ -53,8 +79,7 @@ trait BasePluginPhase[Tree] { self: GraphAnalysis[Tree] => } // only care about cycles with size > 1 here - val components = DepNode.stronglyConnectedComponents(linkedNodes) - .filter(_.size > 1) + val components = DepNode.stronglyConnectedComponents(linkedNodes).filter(_.size > 1) val usedNodes = mutable.Set.empty[DepNode] for { From c559f33ca2a1cff3e0af11bb8b3ef7aa01a15352 Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Mon, 16 Sep 2024 09:24:25 -0400 Subject: [PATCH 09/11] Share all `run` logic between scala 2 and 3. --- .../src-2/acyclic/plugin/PluginPhase.scala | 31 ++++-------------- .../src-3/acyclic/plugin/PluginPhase.scala | 32 ++++--------------- .../src/acyclic/plugin/BasePluginPhase.scala | 27 ++++++++++++++-- 3 files changed, 38 insertions(+), 52 deletions(-) diff --git a/acyclic/src-2/acyclic/plugin/PluginPhase.scala b/acyclic/src-2/acyclic/plugin/PluginPhase.scala index 300dc61..ac58e9c 100644 --- a/acyclic/src-2/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-2/acyclic/plugin/PluginPhase.scala @@ -1,4 +1,5 @@ package acyclic.plugin + import acyclic.file import acyclic.plugin.Compat._ import scala.collection.{SortedSet, mutable} @@ -29,7 +30,7 @@ class PluginPhase( val phaseName = "acyclic" - private object base extends BasePluginPhase[CompilationUnit, Tree] with GraphAnalysis[Tree] { + private object base extends BasePluginPhase[CompilationUnit, Tree, Symbol] with GraphAnalysis[Tree] { protected val cycleReporter = t.cycleReporter protected lazy val force = t.force protected lazy val fatal = t.fatal @@ -53,32 +54,14 @@ class PluginPhase( tree.collect { case Import(expr, List(sel)) => expr.symbol.toString == "package acyclic" && sel.name.toString == selector }.exists(identity) + + def extractDependencies(unit: CompilationUnit): Seq[(Symbol, Tree)] = DependencyExtraction(global)(unit) + def symbolPath(sym: Symbol): String = sym.sourceFile.path + def isValidSymbol(sym: Symbol): Boolean = sym != NoSymbol && sym.sourceFile != null } override def newPhase(prev: Phase): Phase = new Phase(prev) { - override def run() { - val unitMap = base.units.map(u => u.source.path -> u).toMap - val nodes = for (unit <- base.units) yield { - - val deps = DependencyExtraction(t.global)(unit) - - val connections = for { - (sym, tree) <- deps - if sym != NoSymbol - if sym.sourceFile != null - if sym.sourceFile.path != unit.source.path - } yield (sym.sourceFile.path, tree) - - Node[Value.File, Tree]( - Value.File(unit.source.path, base.unitPkgName(unit)), - connections.groupBy(c => Value.File(c._1, base.unitPkgName(unitMap(c._1))): Value) - .mapValues(_.map(_._2)) - .toMap - ) - } - - base.runOnNodes(nodes) - } + override def run(): Unit = base.runAllUnits() def name: String = "acyclic" } diff --git a/acyclic/src-3/acyclic/plugin/PluginPhase.scala b/acyclic/src-3/acyclic/plugin/PluginPhase.scala index 4a8bbe0..13de5e8 100644 --- a/acyclic/src-3/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-3/acyclic/plugin/PluginPhase.scala @@ -5,7 +5,7 @@ import scala.collection.SortedSet import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.{CompilationUnit, report} import dotty.tools.dotc.core.Contexts.Context -import dotty.tools.dotc.core.Symbols.NoSymbol +import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol} import dotty.tools.dotc.util.NoSource /** @@ -21,7 +21,7 @@ class PluginPhase( protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, protected val force: Boolean, protected val fatal: Boolean -)(using ctx: Context) extends BasePluginPhase[CompilationUnit, tpd.Tree], GraphAnalysis[tpd.Tree] { +)(using ctx: Context) extends BasePluginPhase[CompilationUnit, tpd.Tree, Symbol], GraphAnalysis[tpd.Tree] { def treeLine(tree: tpd.Tree): Int = tree.sourcePos.line + 1 def treeSymbolString(tree: tpd.Tree): String = tree.symbol.toString @@ -73,29 +73,9 @@ class PluginPhase( def pkgObjectName(pkgObject: tpd.Tree): String = pkgObject.symbol.enclosingPackageClass.fullName.toString def hasAcyclicImport(tree: tpd.Tree, selector: String): Boolean = hasAcyclicImportAccumulator(selector)(false, tree) - def run(): Unit = { - val unitMap = units.map(u => u.source.path -> u).toMap - val nodes = for (unit <- units) yield { + def extractDependencies(unit: CompilationUnit): Seq[(Symbol, tpd.Tree)] = DependencyExtraction(unit) + def symbolPath(sym: Symbol): String = sym.source.path + def isValidSymbol(sym: Symbol): Boolean = sym != NoSymbol && sym.source != null && sym.source != NoSource - val deps = DependencyExtraction(unit) - - val connections = for { - (sym, tree) <- deps - if sym != NoSymbol - if sym.source != null - if sym.source != NoSource - if sym.source.path != unit.source.path - if unitMap.contains(sym.source.path) - } yield (sym.source.path, tree) - - Node[Value.File, tpd.Tree]( - Value.File(unit.source.path, unitPkgName(unit)), - connections.groupBy(c => Value.File(c._1, unitPkgName(unitMap(c._1))): Value) - .mapValues(_.map(_._2)) - .toMap - ) - } - - runOnNodes(nodes) - } + def run(): Unit = runAllUnits() } diff --git a/acyclic/src/acyclic/plugin/BasePluginPhase.scala b/acyclic/src/acyclic/plugin/BasePluginPhase.scala index a4172e6..71fdd91 100644 --- a/acyclic/src/acyclic/plugin/BasePluginPhase.scala +++ b/acyclic/src/acyclic/plugin/BasePluginPhase.scala @@ -3,7 +3,7 @@ package acyclic.plugin import acyclic.plugin.Compat._ import scala.collection.{mutable, SortedSet} -trait BasePluginPhase[CompilationUnit, Tree] { self: GraphAnalysis[Tree] => +trait BasePluginPhase[CompilationUnit, Tree, Symbol] { self: GraphAnalysis[Tree] => protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit protected def force: Boolean protected def fatal: Boolean @@ -24,6 +24,10 @@ trait BasePluginPhase[CompilationUnit, Tree] { self: GraphAnalysis[Tree] => def pkgObjectName(pkgObject: Tree): String def hasAcyclicImport(tree: Tree, selector: String): Boolean + def extractDependencies(unit: CompilationUnit): Seq[(Symbol, Tree)] + def symbolPath(sym: Symbol): String + def isValidSymbol(sym: Symbol): Boolean + final def findAcyclics(): (Seq[Value.File], Seq[Value.File], Seq[Value.Pkg]) = { val acyclicNodePaths = for { unit <- units if hasAcyclicImport(unitTree(unit), "file") @@ -44,7 +48,26 @@ trait BasePluginPhase[CompilationUnit, Tree] { self: GraphAnalysis[Tree] => (skipNodePaths, acyclicNodePaths, acyclicPkgNames) } - final def runOnNodes(nodes: Seq[Node[Value.File, Tree]]): Unit = { + final def runAllUnits(): Unit = { + val unitMap = units.map(u => unitPath(u) -> u).toMap + val nodes = for (unit <- units) yield { + val deps = extractDependencies(unit) + + val connections = for { + (sym, tree) <- deps + if isValidSymbol(sym) + if symbolPath(sym) != unitPath(unit) + if unitMap.contains(symbolPath(sym)) + } yield (symbolPath(sym), tree) + + Node[Value.File, Tree]( + Value.File(unitPath(unit), unitPkgName(unit)), + connections.groupBy(c => Value.File(c._1, unitPkgName(unitMap(c._1))): Value) + .mapValues(_.map(_._2)) + .toMap + ) + } + val nodeMap = nodes.map(n => n.value -> n).toMap val (skipNodePaths, acyclicFiles, acyclicPkgs) = findAcyclics() From c032bc725a8acd0668a1a32b38021382b39b734e Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 22 Sep 2024 22:19:50 -0400 Subject: [PATCH 10/11] Prevent duplicate runs. --- acyclic/src-3/acyclic/plugin/Plugin.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/acyclic/src-3/acyclic/plugin/Plugin.scala b/acyclic/src-3/acyclic/plugin/Plugin.scala index 90fa92c..9833f8b 100644 --- a/acyclic/src-3/acyclic/plugin/Plugin.scala +++ b/acyclic/src-3/acyclic/plugin/Plugin.scala @@ -13,12 +13,18 @@ class TestPlugin(cycleReporter: Seq[(Value, SortedSet[Int])] => Unit = _ => ()) var force = false var fatal = true + var alreadyRun = false private class Phase() extends PluginPhase { val phaseName = "acyclic" override val runsBefore = Set("patternMatcher") - override def run(using Context): Unit = new acyclic.plugin.PluginPhase(cycleReporter, force, fatal).run() + override def run(using Context): Unit = { + if (!alreadyRun) { + alreadyRun = true + new acyclic.plugin.PluginPhase(cycleReporter, force, fatal).run() + } + } } override def init(options: List[String]): List[PluginPhase] = { From 23c947d38d89091fc4173034a0fa00bfe7c36e78 Mon Sep 17 00:00:00 2001 From: Matt Dziuban Date: Sun, 22 Sep 2024 22:20:12 -0400 Subject: [PATCH 11/11] Use `echo` instead of `inform` -- the latter only appears when `-verbose` is set. --- acyclic/src-3/acyclic/plugin/PluginPhase.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acyclic/src-3/acyclic/plugin/PluginPhase.scala b/acyclic/src-3/acyclic/plugin/PluginPhase.scala index 13de5e8..8c59d22 100644 --- a/acyclic/src-3/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-3/acyclic/plugin/PluginPhase.scala @@ -28,7 +28,7 @@ class PluginPhase( def reportError(msg: String): Unit = report.error(msg) def reportWarning(msg: String): Unit = report.warning(msg) - def reportInform(msg: String): Unit = report.inform(msg) + def reportInform(msg: String): Unit = report.echo(msg) def reportEcho(msg: String, tree: tpd.Tree): Unit = report.echo(msg, tree.srcPos) private val pkgNameAccumulator = new tpd.TreeAccumulator[List[String]] {