From 6722d013f920d51db5c7e84b2b57c7d7f7a113ab Mon Sep 17 00:00:00 2001 From: Deborah Soung Date: Thu, 31 Oct 2024 13:31:45 -0700 Subject: [PATCH] Check cycles on package level --- acyclic/src-2/acyclic/plugin/Plugin.scala | 6 ++++- .../src-2/acyclic/plugin/PluginPhase.scala | 4 ++++ acyclic/src-3/acyclic/plugin/Plugin.scala | 6 ++++- .../src-3/acyclic/plugin/PluginPhase.scala | 12 ++++++++++ .../src/acyclic/plugin/BasePluginPhase.scala | 22 ++++++++++++++----- .../forcepkg/cyclicpackage/a/A1.scala | 6 +++++ .../forcepkg/cyclicpackage/a/A2.scala | 5 +++++ .../forcepkg/cyclicpackage/b/B1.scala | 3 +++ .../forcepkg/cyclicpackage/b/B2.scala | 4 ++++ .../test/resources/forcepkg/simple/A.scala | 6 +++++ .../test/resources/forcepkg/simple/B.scala | 6 +++++ acyclic/test/src-2/acyclic/TestUtils.scala | 6 +++-- acyclic/test/src-3/acyclic/TestUtils.scala | 6 +++-- acyclic/test/src/acyclic/BaseCycleTests.scala | 22 +++++++++++++++++++ acyclic/test/src/acyclic/BaseTestUtils.scala | 3 ++- 15 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 acyclic/test/resources/forcepkg/cyclicpackage/a/A1.scala create mode 100644 acyclic/test/resources/forcepkg/cyclicpackage/a/A2.scala create mode 100644 acyclic/test/resources/forcepkg/cyclicpackage/b/B1.scala create mode 100644 acyclic/test/resources/forcepkg/cyclicpackage/b/B2.scala create mode 100644 acyclic/test/resources/forcepkg/simple/A.scala create mode 100644 acyclic/test/resources/forcepkg/simple/B.scala diff --git a/acyclic/src-2/acyclic/plugin/Plugin.scala b/acyclic/src-2/acyclic/plugin/Plugin.scala index e5be82a..8f102a1 100644 --- a/acyclic/src-2/acyclic/plugin/Plugin.scala +++ b/acyclic/src-2/acyclic/plugin/Plugin.scala @@ -10,6 +10,7 @@ class TestPlugin(val global: Global, cycleReporter: Seq[(Value, SortedSet[Int])] val name = "acyclic" var force = false + var forcePkg = false var fatal = true // Yeah processOptions is deprecated but keep using it anyway for 2.10.x compatibility @@ -17,6 +18,9 @@ class TestPlugin(val global: Global, cycleReporter: Seq[(Value, SortedSet[Int])] if (options.contains("force")) { force = true } + if (options.contains("forcePkg")) { + forcePkg = true + } if (options.contains("warn")) { fatal = false } @@ -24,6 +28,6 @@ class TestPlugin(val global: Global, cycleReporter: Seq[(Value, SortedSet[Int])] val description = "Allows the developer to prohibit inter-file dependencies" val components = List[tools.nsc.plugins.PluginComponent]( - new PluginPhase(this.global, cycleReporter, force, fatal) + new PluginPhase(this.global, cycleReporter, force, forcePkg, fatal) ) } diff --git a/acyclic/src-2/acyclic/plugin/PluginPhase.scala b/acyclic/src-2/acyclic/plugin/PluginPhase.scala index ac58e9c..8847050 100644 --- a/acyclic/src-2/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-2/acyclic/plugin/PluginPhase.scala @@ -19,6 +19,7 @@ class PluginPhase( val global: Global, cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, force: => Boolean, + forcePkg: => Boolean, fatal: => Boolean ) extends PluginComponent { t => @@ -33,6 +34,7 @@ class PluginPhase( 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 forcePkg = t.forcePkg protected lazy val fatal = t.fatal def treeLine(tree: Tree): Int = tree.pos.line @@ -50,6 +52,8 @@ class PluginPhase( 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 findPkgs(tree: Tree): List[Tree] = tree.collect { case x: PackageDef if x.symbol.fullName != "" => x } + def pkgName(pkg: Tree): String = pkg.symbol.fullName def hasAcyclicImport(tree: Tree, selector: String): Boolean = tree.collect { case Import(expr, List(sel)) => expr.symbol.toString == "package acyclic" && sel.name.toString == selector diff --git a/acyclic/src-3/acyclic/plugin/Plugin.scala b/acyclic/src-3/acyclic/plugin/Plugin.scala index 9833f8b..584d344 100644 --- a/acyclic/src-3/acyclic/plugin/Plugin.scala +++ b/acyclic/src-3/acyclic/plugin/Plugin.scala @@ -12,6 +12,7 @@ class TestPlugin(cycleReporter: Seq[(Value, SortedSet[Int])] => Unit = _ => ()) val description = "Allows the developer to prohibit inter-file dependencies" var force = false + var forcePkg = false var fatal = true var alreadyRun = false @@ -22,7 +23,7 @@ class TestPlugin(cycleReporter: Seq[(Value, SortedSet[Int])] => Unit = _ => ()) override def run(using Context): Unit = { if (!alreadyRun) { alreadyRun = true - new acyclic.plugin.PluginPhase(cycleReporter, force, fatal).run() + new acyclic.plugin.PluginPhase(cycleReporter, force, forcePkg, fatal).run() } } } @@ -31,6 +32,9 @@ class TestPlugin(cycleReporter: Seq[(Value, SortedSet[Int])] => Unit = _ => ()) if (options.contains("force")) { force = true } + if (options.contains("forcePkg")) { + forcePkg = true + } if (options.contains("warn")) { fatal = false } diff --git a/acyclic/src-3/acyclic/plugin/PluginPhase.scala b/acyclic/src-3/acyclic/plugin/PluginPhase.scala index 8c59d22..3675a2f 100644 --- a/acyclic/src-3/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src-3/acyclic/plugin/PluginPhase.scala @@ -7,6 +7,7 @@ import dotty.tools.dotc.{CompilationUnit, report} import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Symbols.{NoSymbol, Symbol} import dotty.tools.dotc.util.NoSource +import dotty.tools.dotc.core.Flags.Package /** * - Break dependency graph into strongly connected components @@ -20,6 +21,7 @@ import dotty.tools.dotc.util.NoSource class PluginPhase( protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, protected val force: Boolean, + protected val forcePkg: Boolean, protected val fatal: Boolean )(using ctx: Context) extends BasePluginPhase[CompilationUnit, tpd.Tree, Symbol], GraphAnalysis[tpd.Tree] { @@ -53,6 +55,14 @@ class PluginPhase( ) } + private val pkgAccumulator = new tpd.TreeAccumulator[List[tpd.Tree]] { + def apply(acc: List[tpd.Tree], tree: tpd.Tree)(using Context): List[tpd.Tree] = + foldOver( + if (tree.symbol.is(Package) && tree.symbol.fullName.toString != "") tree :: acc else acc, + tree + ) + } + 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)) => @@ -71,6 +81,8 @@ class PluginPhase( 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 findPkgs(tree: tpd.Tree): List[tpd.Tree] = pkgAccumulator(Nil, tree).reverse + def pkgName(pkg: tpd.Tree): String = pkg.symbol.fullName.toString def hasAcyclicImport(tree: tpd.Tree, selector: String): Boolean = hasAcyclicImportAccumulator(selector)(false, tree) def extractDependencies(unit: CompilationUnit): Seq[(Symbol, tpd.Tree)] = DependencyExtraction(unit) diff --git a/acyclic/src/acyclic/plugin/BasePluginPhase.scala b/acyclic/src/acyclic/plugin/BasePluginPhase.scala index 71fdd91..898dd1e 100644 --- a/acyclic/src/acyclic/plugin/BasePluginPhase.scala +++ b/acyclic/src/acyclic/plugin/BasePluginPhase.scala @@ -6,6 +6,7 @@ import scala.collection.{mutable, SortedSet} trait BasePluginPhase[CompilationUnit, Tree, Symbol] { self: GraphAnalysis[Tree] => protected val cycleReporter: Seq[(Value, SortedSet[Int])] => Unit protected def force: Boolean + protected def forcePkg: Boolean protected def fatal: Boolean def treeLine(tree: Tree): Int @@ -22,6 +23,8 @@ trait BasePluginPhase[CompilationUnit, Tree, Symbol] { self: GraphAnalysis[Tree] def unitPkgName(unit: CompilationUnit): List[String] def findPkgObjects(tree: Tree): List[Tree] def pkgObjectName(pkgObject: Tree): String + def findPkgs(tree: Tree): List[Tree] + def pkgName(tree: Tree): String def hasAcyclicImport(tree: Tree, selector: String): Boolean def extractDependencies(unit: CompilationUnit): Seq[(Symbol, Tree)] @@ -39,15 +42,22 @@ trait BasePluginPhase[CompilationUnit, Tree, Symbol] { self: GraphAnalysis[Tree] } 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) + val stdPackages = if (forcePkg) packages else Seq.empty[Value.Pkg] + val acyclicPkgNames = packageObjects ++ stdPackages (skipNodePaths, acyclicNodePaths, acyclicPkgNames) } + private def packageObjects = for { + unit <- units + pkgObject <- findPkgObjects(unitTree(unit)) + if hasAcyclicImport(pkgObject, "pkg") + } yield Value.Pkg(pkgObjectName(pkgObject).split('.').toList) + + private def packages: Seq[Value.Pkg] = for { + unit <- units + pkgs <- findPkgs(unitTree(unit)) + } yield Value.Pkg(pkgName(pkgs).split('.').toList) + final def runAllUnits(): Unit = { val unitMap = units.map(u => unitPath(u) -> u).toMap val nodes = for (unit <- units) yield { diff --git a/acyclic/test/resources/forcepkg/cyclicpackage/a/A1.scala b/acyclic/test/resources/forcepkg/cyclicpackage/a/A1.scala new file mode 100644 index 0000000..596f67e --- /dev/null +++ b/acyclic/test/resources/forcepkg/cyclicpackage/a/A1.scala @@ -0,0 +1,6 @@ +package forcepkg.cyclicpackage +package a + +class A1 extends b.B1{ + +} diff --git a/acyclic/test/resources/forcepkg/cyclicpackage/a/A2.scala b/acyclic/test/resources/forcepkg/cyclicpackage/a/A2.scala new file mode 100644 index 0000000..e0d01cc --- /dev/null +++ b/acyclic/test/resources/forcepkg/cyclicpackage/a/A2.scala @@ -0,0 +1,5 @@ +package forcepkg.cyclicpackage.a + +class A2 { + +} diff --git a/acyclic/test/resources/forcepkg/cyclicpackage/b/B1.scala b/acyclic/test/resources/forcepkg/cyclicpackage/b/B1.scala new file mode 100644 index 0000000..036d33c --- /dev/null +++ b/acyclic/test/resources/forcepkg/cyclicpackage/b/B1.scala @@ -0,0 +1,3 @@ +package forcepkg.cyclicpackage.b + +class B1 diff --git a/acyclic/test/resources/forcepkg/cyclicpackage/b/B2.scala b/acyclic/test/resources/forcepkg/cyclicpackage/b/B2.scala new file mode 100644 index 0000000..d8d3445 --- /dev/null +++ b/acyclic/test/resources/forcepkg/cyclicpackage/b/B2.scala @@ -0,0 +1,4 @@ +package forcepkg.cyclicpackage +package b + +class B2 extends a.A2 diff --git a/acyclic/test/resources/forcepkg/simple/A.scala b/acyclic/test/resources/forcepkg/simple/A.scala new file mode 100644 index 0000000..2f0cf63 --- /dev/null +++ b/acyclic/test/resources/forcepkg/simple/A.scala @@ -0,0 +1,6 @@ +package forcepkg.simple + + +class A { + val b: B = null +} diff --git a/acyclic/test/resources/forcepkg/simple/B.scala b/acyclic/test/resources/forcepkg/simple/B.scala new file mode 100644 index 0000000..bb4c00c --- /dev/null +++ b/acyclic/test/resources/forcepkg/simple/B.scala @@ -0,0 +1,6 @@ +package forcepkg.simple + +class B { + val a1: A = new A + val a2: A = new A +} diff --git a/acyclic/test/src-2/acyclic/TestUtils.scala b/acyclic/test/src-2/acyclic/TestUtils.scala index 6ac2787..67edce5 100644 --- a/acyclic/test/src-2/acyclic/TestUtils.scala +++ b/acyclic/test/src-2/acyclic/TestUtils.scala @@ -26,6 +26,7 @@ object TestUtils extends BaseTestUtils { path: String, extraIncludes: Seq[String] = Seq("acyclic/src/acyclic/package.scala"), force: Boolean = false, + forcePkg: Boolean = false, warn: Boolean = false, collectInfo: Boolean = true ): Seq[(String, String)] = { @@ -47,6 +48,7 @@ object TestUtils extends BaseTestUtils { val opts = List( if (force) Seq("force") else Seq(), + if (forcePkg) Seq("forcePkg") else Seq(), if (warn) Seq("warn") else Seq() ).flatten if (opts.nonEmpty) { @@ -78,13 +80,13 @@ object TestUtils extends BaseTestUtils { 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])]*) = { + def makeFail(path: String, force: Boolean = false, forcePkg: 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 ex = intercept[CompilationException] { make(path, force = force, forcePkg = forcePkg,collectInfo = false) } val cycles = ex.cycles .map(canonicalize) .map( diff --git a/acyclic/test/src-3/acyclic/TestUtils.scala b/acyclic/test/src-3/acyclic/TestUtils.scala index 6f3769c..db46fbe 100644 --- a/acyclic/test/src-3/acyclic/TestUtils.scala +++ b/acyclic/test/src-3/acyclic/TestUtils.scala @@ -27,6 +27,7 @@ object TestUtils extends BaseTestUtils { path: String, extraIncludes: Seq[String] = Seq("acyclic/src/acyclic/package.scala"), force: Boolean = false, + forcePkg: Boolean = false, warn: Boolean = false, collectInfo: Boolean = true ): Seq[(String, String)] = { @@ -42,6 +43,7 @@ object TestUtils extends BaseTestUtils { val opts = List( if (force) Seq("force") else Seq(), + if (forcePkg) Seq("forcePkg") else Seq(), if (warn) Seq("warn") else Seq() ).flatten @@ -90,13 +92,13 @@ object TestUtils extends BaseTestUtils { }))).getOrElse(Seq.empty) } - def makeFail(path: String, force: Boolean = false)(expected: Seq[(Value, SortedSet[Int])]*) = { + def makeFail(path: String, force: Boolean = false, forcePkg: 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 ex = intercept[CompilationException] { make(path, force = force, forcePkg = forcePkg, collectInfo = false) } val cycles = ex.cycles .map(canonicalize) .map( diff --git a/acyclic/test/src/acyclic/BaseCycleTests.scala b/acyclic/test/src/acyclic/BaseCycleTests.scala index 1ee2cd3..cef962c 100644 --- a/acyclic/test/src/acyclic/BaseCycleTests.scala +++ b/acyclic/test/src/acyclic/BaseCycleTests.scala @@ -68,6 +68,28 @@ class BaseCycleTests(utils: BaseTestUtils) extends TestSuite { )) test("pass") - make("force/simple") test("skip") - make("force/skip", force = true) + test("mutualcyclic") - make("success/pkg/mutualcyclic", force = true) + } + test("forcepkg") - { + test("fail") - { + makeFail("forcepkg/cyclicpackage", force = false, forcePkg = true)( + Seq( + Pkg("forcepkg.cyclicpackage.b") -> SortedSet(4), + Pkg("forcepkg.cyclicpackage.a") -> SortedSet(4) + ) + ) + } + test("success") - { + make("forcepkg/simple", force = false, forcePkg = true) + } + test("fail") - { + makeFail("forcepkg/simple", force = true, forcePkg = true)( + Seq( + File("B.scala") -> SortedSet(4, 5), + File("A.scala") -> SortedSet(5) + ) + ) + } } } } diff --git a/acyclic/test/src/acyclic/BaseTestUtils.scala b/acyclic/test/src/acyclic/BaseTestUtils.scala index cab5378..cb86f13 100644 --- a/acyclic/test/src/acyclic/BaseTestUtils.scala +++ b/acyclic/test/src/acyclic/BaseTestUtils.scala @@ -14,11 +14,12 @@ abstract class BaseTestUtils { path: String, extraIncludes: Seq[String] = Seq("acyclic/src/acyclic/package.scala"), force: Boolean = false, + forcePkg: Boolean = false, warn: Boolean = false, collectInfo: Boolean = true ): Seq[(String, String)] - def makeFail(path: String, force: Boolean = false)(expected: Seq[(Value, SortedSet[Int])]*): Unit + def makeFail(path: String, force: Boolean = false, forcePkg: Boolean = false)(expected: Seq[(Value, SortedSet[Int])]*): Unit case class CompilationException(cycles: Seq[Seq[(Value, SortedSet[Int])]]) extends Exception