Skip to content

Commit

Permalink
Check cycles on package level
Browse files Browse the repository at this point in the history
  • Loading branch information
debs-sifive committed Oct 31, 2024
1 parent fbb60d1 commit 6722d01
Show file tree
Hide file tree
Showing 15 changed files with 104 additions and 13 deletions.
6 changes: 5 additions & 1 deletion acyclic/src-2/acyclic/plugin/Plugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,24 @@ 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
override def processOptions(options: List[String], error: String => Unit): Unit = {
if (options.contains("force")) {
force = true
}
if (options.contains("forcePkg")) {
forcePkg = true
}
if (options.contains("warn")) {
fatal = false
}
}
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)
)
}
4 changes: 4 additions & 0 deletions acyclic/src-2/acyclic/plugin/PluginPhase.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class PluginPhase(
val global: Global,
cycleReporter: Seq[(Value, SortedSet[Int])] => Unit,
force: => Boolean,
forcePkg: => Boolean,
fatal: => Boolean
) extends PluginComponent { t =>

Expand All @@ -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
Expand All @@ -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 != "<empty>" => 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
Expand Down
6 changes: 5 additions & 1 deletion acyclic/src-3/acyclic/plugin/Plugin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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()
}
}
}
Expand All @@ -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
}
Expand Down
12 changes: 12 additions & 0 deletions acyclic/src-3/acyclic/plugin/PluginPhase.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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] {

Expand Down Expand Up @@ -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 != "<empty>") 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)) =>
Expand All @@ -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)
Expand Down
22 changes: 16 additions & 6 deletions acyclic/src/acyclic/plugin/BasePluginPhase.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)]
Expand All @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions acyclic/test/resources/forcepkg/cyclicpackage/a/A1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package forcepkg.cyclicpackage
package a

class A1 extends b.B1{

}
5 changes: 5 additions & 0 deletions acyclic/test/resources/forcepkg/cyclicpackage/a/A2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package forcepkg.cyclicpackage.a

class A2 {

}
3 changes: 3 additions & 0 deletions acyclic/test/resources/forcepkg/cyclicpackage/b/B1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package forcepkg.cyclicpackage.b

class B1
4 changes: 4 additions & 0 deletions acyclic/test/resources/forcepkg/cyclicpackage/b/B2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package forcepkg.cyclicpackage
package b

class B2 extends a.A2
6 changes: 6 additions & 0 deletions acyclic/test/resources/forcepkg/simple/A.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package forcepkg.simple


class A {
val b: B = null
}
6 changes: 6 additions & 0 deletions acyclic/test/resources/forcepkg/simple/B.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package forcepkg.simple

class B {
val a1: A = new A
val a2: A = new A
}
6 changes: 4 additions & 2 deletions acyclic/test/src-2/acyclic/TestUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)] = {
Expand All @@ -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) {
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 4 additions & 2 deletions acyclic/test/src-3/acyclic/TestUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)] = {
Expand All @@ -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

Expand Down Expand Up @@ -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(
Expand Down
22 changes: 22 additions & 0 deletions acyclic/test/src/acyclic/BaseCycleTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
)
}
}
}
}
3 changes: 2 additions & 1 deletion acyclic/test/src/acyclic/BaseTestUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 6722d01

Please sign in to comment.