From ddf9a23e2737884051403473aff32fb9e2c5ff71 Mon Sep 17 00:00:00 2001 From: ghostbuster91 Date: Fri, 25 Jan 2019 11:49:06 +0100 Subject: [PATCH] Check cycles on package level * Include test which proves that force flag does not enforce package level checks --- acyclic/src/acyclic/plugin/Plugin.scala | 6 ++- acyclic/src/acyclic/plugin/PluginPhase.scala | 41 ++++++++++++------- acyclic/test/src/acyclic/CycleTests.scala | 22 ++++++++++ acyclic/test/src/acyclic/TestUtils.scala | 6 ++- .../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 +++ 10 files changed, 88 insertions(+), 17 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/acyclic/src/acyclic/plugin/Plugin.scala b/acyclic/src/acyclic/plugin/Plugin.scala index e5be82a..8f102a1 100644 --- a/acyclic/src/acyclic/plugin/Plugin.scala +++ b/acyclic/src/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/acyclic/plugin/PluginPhase.scala b/acyclic/src/acyclic/plugin/PluginPhase.scala index 76e089c..dc59977 100644 --- a/acyclic/src/acyclic/plugin/PluginPhase.scala +++ b/acyclic/src/acyclic/plugin/PluginPhase.scala @@ -18,6 +18,7 @@ class PluginPhase( val global: Global, cycleReporter: Seq[(Value, SortedSet[Int])] => Unit, force: => Boolean, + forcePkg: => Boolean, fatal: => Boolean ) extends PluginComponent with GraphAnalysis { t => @@ -59,23 +60,35 @@ class PluginPhase( } 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 { - 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 + 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)) => + expr.symbol.toString == "package acyclic" && sel.name.toString == "pkg" + }.exists(x => x) + } yield { + Value.Pkg( + pkgObject.symbol + .enclosingPackageClass + .fullName + .split('.') + .toList + ) + } + + 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 - ) - } - (skipNodePaths, acyclicNodePaths, acyclicPkgNames) + )) } override def newPhase(prev: Phase): Phase = new Phase(prev) { diff --git a/acyclic/test/src/acyclic/CycleTests.scala b/acyclic/test/src/acyclic/CycleTests.scala index 134a4dd..dac55d5 100644 --- a/acyclic/test/src/acyclic/CycleTests.scala +++ b/acyclic/test/src/acyclic/CycleTests.scala @@ -69,6 +69,28 @@ object CycleTests 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/TestUtils.scala b/acyclic/test/src/acyclic/TestUtils.scala index d33bc26..cce5b49 100644 --- a/acyclic/test/src/acyclic/TestUtils.scala +++ b/acyclic/test/src/acyclic/TestUtils.scala @@ -31,6 +31,7 @@ object TestUtils { path: String, extraIncludes: Seq[String] = Seq("acyclic/src/acyclic/package.scala"), force: Boolean = false, + forcePkg: Boolean = false, warn: Boolean = false, collectInfo: Boolean = true ): Seq[(Position, String, String)] = { @@ -52,6 +53,7 @@ object TestUtils { 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) { @@ -83,13 +85,13 @@ object TestUtils { storeReporter.map(_.infos.toSeq.map(i => (i.pos, 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/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 +}