-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scala 3 support #136
Merged
Merged
Scala 3 support #136
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a30ba0a
Move existing sources to `src-2` directory.
mrdziuban 718865e
Port plugin to Scala 3, get tests passing.
mrdziuban 07310c7
Support Scala 3.4.x and 3.5.0.
mrdziuban e5c63c0
Lowercase severity names.
mrdziuban 9bda417
Depend on acyclic 0.3.13, remove 2.12.20 from unreleased versions.
mrdziuban 545b931
Add `assert` to `force.warn.fail` test, use uppercase strings.
mrdziuban ddddff6
Share `run` logic between scala 2 and 3.
mrdziuban 225e3c1
Share `findAcyclics` logic between scala 2 and 3.
mrdziuban c559f33
Share all `run` logic between scala 2 and 3.
mrdziuban 75bcffb
Merge branch 'main' into scala-3
lihaoyi c032bc7
Prevent duplicate runs.
mrdziuban 23c947d
Use `echo` instead of `inform` -- the latter only appears when `-verb…
mrdziuban File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
pluginClass=acyclic.plugin.RuntimePlugin |
File renamed without changes.
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
100 changes: 100 additions & 0 deletions
100
acyclic/src-3/acyclic/plugin/DependencyExtraction.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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()) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
package acyclic | ||
|
||
object CycleTests extends BaseCycleTests(TestUtils) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how much of the logic in this file can be shared between the Scala 2 and Scala 3 implementations? Like clearly the logic for working with symbols and types and trees is all bespoke, but it seems there's a lot of stuff working with
Value
s andDepNode
s that can be shared if properly separated out from the compiler-specific logicThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, I was able to share pretty much all of it by abstracting over
CompilationUnit
,Tree
, andSymbol