From 07bb2246a50d23248682c98ce3d2ca7f7d30ec30 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Fri, 25 Jan 2019 11:49:06 +0100 Subject: [PATCH 1/2] Check cycles on package level --- src/main/scala/acyclic/plugin/Plugin.scala | 6 ++++- .../scala/acyclic/plugin/PluginPhase.scala | 25 +++++++++++++++---- .../forcepkg/cyclicpackage/a/A1.scala | 6 +++++ .../forcepkg/cyclicpackage/a/A2.scala | 5 ++++ .../forcepkg/cyclicpackage/b/B1.scala | 3 +++ .../forcepkg/cyclicpackage/b/B2.scala | 4 +++ src/test/resources/forcepkg/simple/A.scala | 6 +++++ src/test/resources/forcepkg/simple/B.scala | 6 +++++ src/test/scala/acyclic/CycleTests.scala | 16 +++++++++++- src/test/scala/acyclic/TestUtils.scala | 10 +++++--- 10 files changed, 76 insertions(+), 11 deletions(-) create mode 100644 src/test/resources/forcepkg/cyclicpackage/a/A1.scala create mode 100644 src/test/resources/forcepkg/cyclicpackage/a/A2.scala create mode 100644 src/test/resources/forcepkg/cyclicpackage/b/B1.scala create mode 100644 src/test/resources/forcepkg/cyclicpackage/b/B2.scala create mode 100644 src/test/resources/forcepkg/simple/A.scala create mode 100644 src/test/resources/forcepkg/simple/B.scala diff --git a/src/main/scala/acyclic/plugin/Plugin.scala b/src/main/scala/acyclic/plugin/Plugin.scala index 257894c..dcd1676 100644 --- a/src/main/scala/acyclic/plugin/Plugin.scala +++ b/src/main/scala/acyclic/plugin/Plugin.scala @@ -11,16 +11,20 @@ class TestPlugin(val global: Global, val name = "acyclic" var force = false + var forcePkg = false // 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 + } } val description = "Allows the developer to prohibit inter-file dependencies" val components = List[tools.nsc.plugins.PluginComponent]( - new PluginPhase(this.global, cycleReporter, force) + new PluginPhase(this.global, cycleReporter, force, forcePkg) ) } diff --git a/src/main/scala/acyclic/plugin/PluginPhase.scala b/src/main/scala/acyclic/plugin/PluginPhase.scala index 4e8adbc..6ad43f4 100644 --- a/src/main/scala/acyclic/plugin/PluginPhase.scala +++ b/src/main/scala/acyclic/plugin/PluginPhase.scala @@ -17,7 +17,8 @@ import tools.nsc.plugins.PluginComponent */ class PluginPhase(val global: Global, cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, - force: => Boolean) + force: => Boolean, + forcePkg: => Boolean) extends PluginComponent with GraphAnalysis { t => @@ -58,11 +59,16 @@ class PluginPhase(val global: Global, } yield { Value.File(unit.source.path, pkgName(unit)) } + val stdPackages = if(forcePkg) packages else Seq.empty[Value.Pkg] + val acyclicPkgNames = packageObjects ++ stdPackages + (skipNodePaths, acyclicNodePaths, acyclicPkgNames) + } - val acyclicPkgNames = for { + private def packageObjects = { + 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)) => + 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 { @@ -74,7 +80,16 @@ class PluginPhase(val global: Global, .toList ) } - (skipNodePaths, acyclicNodePaths, acyclicPkgNames) + } + + private def packages: Seq[Value.Pkg] = { + units.map(x => x.body).collect { case y: PackageDef => y }.map(z => z.symbol.fullName).filter(x => x != "") + .distinct + .map(c => Value.Pkg( + c + .split('.') + .toList + )) } override def newPhase(prev: Phase): Phase = new Phase(prev) { diff --git a/src/test/resources/forcepkg/cyclicpackage/a/A1.scala b/src/test/resources/forcepkg/cyclicpackage/a/A1.scala new file mode 100644 index 0000000..596f67e --- /dev/null +++ b/src/test/resources/forcepkg/cyclicpackage/a/A1.scala @@ -0,0 +1,6 @@ +package forcepkg.cyclicpackage +package a + +class A1 extends b.B1{ + +} diff --git a/src/test/resources/forcepkg/cyclicpackage/a/A2.scala b/src/test/resources/forcepkg/cyclicpackage/a/A2.scala new file mode 100644 index 0000000..e0d01cc --- /dev/null +++ b/src/test/resources/forcepkg/cyclicpackage/a/A2.scala @@ -0,0 +1,5 @@ +package forcepkg.cyclicpackage.a + +class A2 { + +} diff --git a/src/test/resources/forcepkg/cyclicpackage/b/B1.scala b/src/test/resources/forcepkg/cyclicpackage/b/B1.scala new file mode 100644 index 0000000..eeb5b12 --- /dev/null +++ b/src/test/resources/forcepkg/cyclicpackage/b/B1.scala @@ -0,0 +1,3 @@ +package forcepkg.cyclicpackage.b + +class B1 \ No newline at end of file diff --git a/src/test/resources/forcepkg/cyclicpackage/b/B2.scala b/src/test/resources/forcepkg/cyclicpackage/b/B2.scala new file mode 100644 index 0000000..58a34da --- /dev/null +++ b/src/test/resources/forcepkg/cyclicpackage/b/B2.scala @@ -0,0 +1,4 @@ +package forcepkg.cyclicpackage +package b + +class B2 extends a.A2 \ No newline at end of file diff --git a/src/test/resources/forcepkg/simple/A.scala b/src/test/resources/forcepkg/simple/A.scala new file mode 100644 index 0000000..2f0cf63 --- /dev/null +++ b/src/test/resources/forcepkg/simple/A.scala @@ -0,0 +1,6 @@ +package forcepkg.simple + + +class A { + val b: B = null +} diff --git a/src/test/resources/forcepkg/simple/B.scala b/src/test/resources/forcepkg/simple/B.scala new file mode 100644 index 0000000..bb4c00c --- /dev/null +++ b/src/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/src/test/scala/acyclic/CycleTests.scala b/src/test/scala/acyclic/CycleTests.scala index ff831aa..1c80918 100644 --- a/src/test/scala/acyclic/CycleTests.scala +++ b/src/test/scala/acyclic/CycleTests.scala @@ -2,7 +2,6 @@ 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 @@ -62,6 +61,21 @@ object CycleTests extends TestSuite{ 'pass-make("force/simple") 'skip-make("force/skip", force = true) } + 'forcepkg{ + 'fail-makeFail("forcepkg/cyclicpackage", force = false, forcePkg = true)( + Seq( + Pkg("forcepkg.cyclicpackage.b") -> SortedSet(4), + Pkg("forcepkg.cyclicpackage.a") -> SortedSet(4) + ) + ) + 'success-make("forcepkg/simple", force = false, forcePkg = true) + 'fail-makeFail("forcepkg/simple", force = true, forcePkg = true)( + Seq( + File("B.scala") -> SortedSet(4, 5), + File("A.scala") -> SortedSet(5) + ) + ) + } } } diff --git a/src/test/scala/acyclic/TestUtils.scala b/src/test/scala/acyclic/TestUtils.scala index 7bff824..099d26b 100644 --- a/src/test/scala/acyclic/TestUtils.scala +++ b/src/test/scala/acyclic/TestUtils.scala @@ -24,7 +24,8 @@ object TestUtils { */ def make(path: String, extraIncludes: Seq[String] = Seq("src/main/scala/acyclic/package.scala"), - force: Boolean = false) = { + force: Boolean = false, + forcePkg: Boolean = false) = { val src = "src/test/resources/" + path val sources = getFilePaths(src) ++ extraIncludes @@ -41,7 +42,8 @@ object TestUtils { settings.classpath.value = ClassPath.join(entries ++ sclpath : _*) - if (force) settings.pluginOptions.value = List("acyclic:force") + if (force) settings.pluginOptions.value ++= List("acyclic:force") + if (forcePkg) settings.pluginOptions.value ++= List("acyclic:forcePkg") var cycles: Option[Seq[Seq[(acyclic.plugin.Value, SortedSet[Int])]]] = None lazy val compiler = new Global(settings, new ConsoleReporter(settings)){ @@ -58,13 +60,13 @@ object TestUtils { if (vd.toList.isEmpty) throw CompilationException(cycles.get) } - 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) } + val ex = intercept[CompilationException]{ make(path, force = force, forcePkg = forcePkg) } val cycles = ex.cycles .map(canonicalize) .map( From e27ea0e2bb8cac1e4d0a33023d0a44fde04559d6 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Fri, 25 Jan 2019 12:00:00 +0100 Subject: [PATCH 2/2] Add test which proves that force flag does not enforce package level checks --- src/test/scala/acyclic/CycleTests.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/scala/acyclic/CycleTests.scala b/src/test/scala/acyclic/CycleTests.scala index 1c80918..50187b0 100644 --- a/src/test/scala/acyclic/CycleTests.scala +++ b/src/test/scala/acyclic/CycleTests.scala @@ -60,6 +60,7 @@ object CycleTests extends TestSuite{ )) 'pass-make("force/simple") 'skip-make("force/skip", force = true) + "mutualcyclic"-make("success/pkg/mutualcyclic", force = true) } 'forcepkg{ 'fail-makeFail("forcepkg/cyclicpackage", force = false, forcePkg = true)(