Skip to content

Commit c410b57

Browse files
committed
Diagram tweaks #2
- fixed the AnyRef linking (SI-5780) - added tooltips to implicit conversions in diagrams - fixed the intermittent dot error where node images would be left out (dot is not reliable at all -- with all the mechanisms in place to fail gracefully, we still get dot errors crawling their way into diagrams - and that usually means no diagram generated, which is the most appropriate way to fail, I think...)
1 parent f8cb1ae commit c410b57

File tree

12 files changed

+419
-59
lines changed

12 files changed

+419
-59
lines changed

src/compiler/scala/tools/nsc/doc/html/page/diagram/DiagramStats.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ object DiagramStats {
3838
private[this] val dotGenTrack = new TimeTracker("dot diagram generation")
3939
private[this] val dotRunTrack = new TimeTracker("dot process runnning")
4040
private[this] val svgTrack = new TimeTracker("svg processing")
41+
private[this] var brokenImages = 0
42+
private[this] var fixedImages = 0
4143

4244
def printStats(settings: Settings) = {
4345
if (settings.docDiagramsDebug.value) {
@@ -47,6 +49,9 @@ object DiagramStats {
4749
dotGenTrack.printStats(settings.printMsg)
4850
dotRunTrack.printStats(settings.printMsg)
4951
svgTrack.printStats(settings.printMsg)
52+
println(" Broken images: " + brokenImages)
53+
println(" Fixed images: " + fixedImages)
54+
println("")
5055
}
5156
}
5257

@@ -55,4 +60,7 @@ object DiagramStats {
5560
def addDotGenerationTime(ms: Long) = dotGenTrack.addTime(ms)
5661
def addDotRunningTime(ms: Long) = dotRunTrack.addTime(ms)
5762
def addSvgTime(ms: Long) = svgTrack.addTime(ms)
63+
64+
def addBrokenImage(): Unit = brokenImages += 1
65+
def addFixedImage(): Unit = fixedImages += 1
5866
}

src/compiler/scala/tools/nsc/doc/html/page/diagram/DotDiagramGenerator.scala

Lines changed: 61 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
6666
var superClasses = List[Node]()
6767
var incomingImplicits = List[Node]()
6868
var outgoingImplicits = List[Node]()
69-
var subClassesTooltip: Option[String] = None
70-
var superClassesTooltip: Option[String] = None
71-
var incomingImplicitsTooltip: Option[String] = None
72-
var outgoingImplicitsTooltip: Option[String] = None
7369
isClassDiagram = false
7470

7571
d match {
@@ -89,23 +85,23 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
8985
// if there are too many super / sub / implicit nodes, represent
9086
// them by on node with a corresponding tooltip
9187
superClasses = if (_superClasses.length > settings.docDiagramsMaxNormalClasses.value) {
92-
superClassesTooltip = Some(limitSize(_superClasses.map(_.tpe.name).mkString(", ")))
93-
List(NormalNode(textTypeEntity(_superClasses.length + MultiSuffix), None))
88+
val superClassesTooltip = Some(limitSize(_superClasses.map(_.tpe.name).mkString(", ")))
89+
List(NormalNode(textTypeEntity(_superClasses.length + MultiSuffix), None, superClassesTooltip))
9490
} else _superClasses
9591

9692
subClasses = if (_subClasses.length > settings.docDiagramsMaxNormalClasses.value) {
97-
subClassesTooltip = Some(limitSize(_subClasses.map(_.tpe.name).mkString(", ")))
98-
List(NormalNode(textTypeEntity(_subClasses.length + MultiSuffix), None))
93+
val subClassesTooltip = Some(limitSize(_subClasses.map(_.tpe.name).mkString(", ")))
94+
List(NormalNode(textTypeEntity(_subClasses.length + MultiSuffix), None, subClassesTooltip))
9995
} else _subClasses
10096

10197
incomingImplicits = if (_incomingImplicits.length > settings.docDiagramsMaxImplicitClasses.value) {
102-
incomingImplicitsTooltip = Some(limitSize(_incomingImplicits.map(_.tpe.name).mkString(", ")))
103-
List(ImplicitNode(textTypeEntity(_incomingImplicits.length + MultiSuffix), None))
98+
val incomingImplicitsTooltip = Some(limitSize(_incomingImplicits.map(_.tpe.name).mkString(", ")))
99+
List(ImplicitNode(textTypeEntity(_incomingImplicits.length + MultiSuffix), None, incomingImplicitsTooltip))
104100
} else _incomingImplicits
105101

106102
outgoingImplicits = if (_outgoingImplicits.length > settings.docDiagramsMaxImplicitClasses.value) {
107-
outgoingImplicitsTooltip = Some(limitSize(_outgoingImplicits.map(_.tpe.name).mkString(", ")))
108-
List(ImplicitNode(textTypeEntity(_outgoingImplicits.length + MultiSuffix), None))
103+
val outgoingImplicitsTooltip = Some(limitSize(_outgoingImplicits.map(_.tpe.name).mkString(", ")))
104+
List(ImplicitNode(textTypeEntity(_outgoingImplicits.length + MultiSuffix), None, outgoingImplicitsTooltip))
109105
} else _outgoingImplicits
110106

111107
thisNode = _thisNode
@@ -128,14 +124,14 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
128124
// dot cluster containing thisNode
129125
val thisCluster = "subgraph clusterThis {\n" +
130126
"style=\"invis\"\n" +
131-
node2Dot(thisNode, None) +
127+
node2Dot(thisNode) +
132128
"}"
133129
// dot cluster containing incoming implicit nodes, if any
134130
val incomingCluster = {
135131
if(incomingImplicits.isEmpty) ""
136132
else "subgraph clusterIncoming {\n" +
137133
"style=\"invis\"\n" +
138-
incomingImplicits.reverse.map(n => node2Dot(n, incomingImplicitsTooltip)).mkString +
134+
incomingImplicits.reverse.map(n => node2Dot(n)).mkString +
139135
(if (incomingImplicits.size > 1)
140136
incomingImplicits.map(n => "node" + node2Index(n)).mkString(" -> ") +
141137
" [constraint=\"false\", style=\"invis\", minlen=\"0.0\"];\n"
@@ -147,7 +143,7 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
147143
if(outgoingImplicits.isEmpty) ""
148144
else "subgraph clusterOutgoing {\n" +
149145
"style=\"invis\"\n" +
150-
outgoingImplicits.reverse.map(n => node2Dot(n, outgoingImplicitsTooltip)).mkString +
146+
outgoingImplicits.reverse.map(n => node2Dot(n)).mkString +
151147
(if (outgoingImplicits.size > 1)
152148
outgoingImplicits.map(n => "node" + node2Index(n)).mkString(" -> ") +
153149
" [constraint=\"false\", style=\"invis\", minlen=\"0.0\"];\n"
@@ -189,9 +185,9 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
189185
"edge [" + edgeAttributesStr + "];\n" +
190186
implicitsDot + "\n" +
191187
// inheritance nodes
192-
nodes.map(n => node2Dot(n, None)).mkString +
193-
subClasses.map(n => node2Dot(n, subClassesTooltip)).mkString +
194-
superClasses.map(n => node2Dot(n, superClassesTooltip)).mkString +
188+
nodes.map(n => node2Dot(n)).mkString +
189+
subClasses.map(n => node2Dot(n)).mkString +
190+
superClasses.map(n => node2Dot(n)).mkString +
195191
// inheritance edges
196192
edges.map{ case (from, tos) => tos.map(to => {
197193
val id = "graph" + counter + "_" + node2Index(to) + "_" + node2Index(from)
@@ -213,7 +209,7 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
213209
/**
214210
* Generates the dot string of a given node.
215211
*/
216-
private def node2Dot(node: Node, tooltip: Option[String]) = {
212+
private def node2Dot(node: Node) = {
217213

218214
// escape HTML characters in node names
219215
def escape(name: String) = name.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;");
@@ -228,7 +224,7 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
228224
}
229225

230226
// tooltip
231-
tooltip match {
227+
node.tooltip match {
232228
case Some(text) => attr += "tooltip" -> text
233229
// show full name where available (instead of TraversableOps[A] show scala.collection.parallel.TraversableOps[A])
234230
case None if node.tpl.isDefined => attr += "tooltip" -> node.tpl.get.qualifiedName
@@ -294,25 +290,23 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
294290
*/
295291
private def cssClass(node: Node): String =
296292
if (node.isImplicitNode && incomingImplicitNodes.contains(node))
297-
"implicit-incoming"
293+
"implicit-incoming" + cssBaseClass(node, "", " ")
298294
else if (node.isImplicitNode)
299-
"implicit-outgoing"
300-
else if (node.isObjectNode)
301-
"object"
295+
"implicit-outgoing" + cssBaseClass(node, "", " ")
302296
else if (node.isThisNode)
303-
"this" + cssBaseClass(node, "")
297+
"this" + cssBaseClass(node, "", " ")
304298
else if (node.isOutsideNode)
305-
"outside" + cssBaseClass(node, "")
299+
"outside" + cssBaseClass(node, "", " ")
306300
else
307-
cssBaseClass(node, "default")
301+
cssBaseClass(node, "default", "")
308302

309-
private def cssBaseClass(node: Node, default: String) =
303+
private def cssBaseClass(node: Node, default: String, space: String) =
310304
if (node.isClassNode)
311-
" class"
305+
space + "class"
312306
else if (node.isTraitNode)
313-
" trait"
307+
space + "trait"
314308
else if (node.isObjectNode)
315-
" trait"
309+
space + "object"
316310
else
317311
default
318312

@@ -381,10 +375,31 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
381375
// assign id and class attributes to edges and nodes:
382376
// the id attribute generated by dot has the format: "{class}|{id}"
383377
case g @ Elem(prefix, "g", attribs, scope, children @ _*) if (List("edge", "node").contains((g \ "@class").toString)) => {
384-
val res = new Elem(prefix, "g", attribs, scope, (children map(x => transform(x))): _*)
378+
var res = new Elem(prefix, "g", attribs, scope, (children map(x => transform(x))): _*)
385379
val dotId = (g \ "@id").toString
386380
if (dotId.count(_ == '|') == 1) {
387381
val Array(klass, id) = dotId.toString.split("\\|")
382+
/* Sometimes dot "forgets" to add the image -- that's very annoying, but it seems pretty random, and simple
383+
* tests like excute 20K times and diff the output don't trigger the bug -- so it's up to us to place the image
384+
* back in the node */
385+
val kind = getKind(klass)
386+
if (kind != "")
387+
if (((g \ "a" \ "image").isEmpty)) {
388+
DiagramStats.addBrokenImage()
389+
val xposition = getPosition(g, "x", -22)
390+
val yposition = getPosition(g, "y", -11.3334)
391+
if (xposition.isDefined && yposition.isDefined) {
392+
val imageNode = <image xmlns:xlink="http://www.w3.org/1999/xlink" xlink:href={ ("./lib/" + kind + "_diagram.png") } width="16px" height="16px" preserveAspectRatio="xMinYMin meet" x={ xposition.get.toString } y={ yposition.get.toString }/>
393+
val anchorNode = (g \ "a") match {
394+
case Seq(Elem(prefix, "a", attribs, scope, children @ _*)) =>
395+
transform(new Elem(prefix, "a", attribs, scope, (children ++ imageNode): _*))
396+
case _ =>
397+
g \ "a"
398+
}
399+
res = new Elem(prefix, "g", attribs, scope, anchorNode: _*)
400+
DiagramStats.addFixedImage()
401+
}
402+
}
388403
res % new UnprefixedAttribute("id", id, Null) %
389404
new UnprefixedAttribute("class", (g \ "@class").toString + " " + klass, Null)
390405
}
@@ -399,6 +414,20 @@ class DotDiagramGenerator(settings: doc.Settings) extends DiagramGenerator {
399414
case x => x
400415
}
401416

417+
def getKind(klass: String): String =
418+
if (klass.contains("class")) "class"
419+
else if (klass.contains("trait")) "trait"
420+
else if (klass.contains("object")) "object"
421+
else ""
422+
423+
def getPosition(g: xml.Node, axis: String, offset: Double): Option[Double] = {
424+
val node = g \ "a" \ "text" \ ("@" + axis)
425+
if (node.isEmpty)
426+
None
427+
else
428+
Some(node.toString.toDouble + offset)
429+
}
430+
402431
/* graph / node / edge attributes */
403432

404433
private val graphAttributes: Map[String, String] = Map(

src/compiler/scala/tools/nsc/doc/model/Entity.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,11 @@ trait DocTemplateEntity extends TemplateEntity with MemberEntity {
278278
def implicitsShadowing: Map[MemberEntity, ImplicitMemberShadowing]
279279

280280
/** Classes that can be implcitly converted to this class */
281-
def incomingImplicitlyConvertedClasses: List[DocTemplateEntity]
281+
def incomingImplicitlyConvertedClasses: List[(DocTemplateEntity, ImplicitConversion)]
282282

283283
/** Classes to which this class can be implicitly converted to
284284
NOTE: Some classes might not be included in the scaladoc run so they will be NoDocTemplateEntities */
285-
def outgoingImplicitlyConvertedClasses: List[(TemplateEntity, TypeEntity)]
285+
def outgoingImplicitlyConvertedClasses: List[(TemplateEntity, TypeEntity, ImplicitConversion)]
286286

287287
/** If this template takes place in inheritance and implicit conversion relations, it will be shown in this diagram */
288288
def inheritanceDiagram: Option[Diagram]

src/compiler/scala/tools/nsc/doc/model/ModelFactory.scala

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -299,14 +299,14 @@ class ModelFactory(val global: Global, val settings: doc.Settings) {
299299
def directSubClasses = allSubClasses.filter(_.parentTypes.map(_._1).contains(this))
300300

301301
/* Implcitly convertible class cache */
302-
private var implicitlyConvertibleClassesCache: mutable.ListBuffer[DocTemplateEntity] = null
303-
def registerImplicitlyConvertibleClass(sc: DocTemplateEntity): Unit = {
302+
private var implicitlyConvertibleClassesCache: mutable.ListBuffer[(DocTemplateEntity, ImplicitConversionImpl)] = null
303+
def registerImplicitlyConvertibleClass(dtpl: DocTemplateEntity, conv: ImplicitConversionImpl): Unit = {
304304
if (implicitlyConvertibleClassesCache == null)
305-
implicitlyConvertibleClassesCache = mutable.ListBuffer[DocTemplateEntity]()
306-
implicitlyConvertibleClassesCache += sc
305+
implicitlyConvertibleClassesCache = mutable.ListBuffer[(DocTemplateEntity, ImplicitConversionImpl)]()
306+
implicitlyConvertibleClassesCache += ((dtpl, conv))
307307
}
308308

309-
def incomingImplicitlyConvertedClasses: List[DocTemplateEntity] =
309+
def incomingImplicitlyConvertedClasses: List[(DocTemplateEntity, ImplicitConversionImpl)] =
310310
if (implicitlyConvertibleClassesCache == null)
311311
List()
312312
else
@@ -363,18 +363,19 @@ class ModelFactory(val global: Global, val settings: doc.Settings) {
363363

364364
var implicitsShadowing = Map[MemberEntity, ImplicitMemberShadowing]()
365365

366-
lazy val outgoingImplicitlyConvertedClasses: List[(TemplateEntity, TypeEntity)] = conversions flatMap (conv =>
367-
if (!implicitExcluded(conv.conversionQualifiedName))
368-
conv.targetTypeComponents map {
369-
case pair@(template, tpe) =>
370-
template match {
371-
case d: DocTemplateImpl => d.registerImplicitlyConvertibleClass(this)
372-
case _ => // nothing
373-
}
374-
pair
375-
}
376-
else List()
377-
)
366+
lazy val outgoingImplicitlyConvertedClasses: List[(TemplateEntity, TypeEntity, ImplicitConversionImpl)] =
367+
conversions flatMap (conv =>
368+
if (!implicitExcluded(conv.conversionQualifiedName))
369+
conv.targetTypeComponents map {
370+
case pair@(template, tpe) =>
371+
template match {
372+
case d: DocTemplateImpl => d.registerImplicitlyConvertibleClass(this, conv)
373+
case _ => // nothing
374+
}
375+
(pair._1, pair._2, conv)
376+
}
377+
else List()
378+
)
378379

379380
override def isTemplate = true
380381
lazy val definitionName = optimize(inDefinitionTemplates.head.qualifiedName + "." + name)
@@ -862,7 +863,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) {
862863
// nameBuffer append stripPrefixes.foldLeft(pre.prefixString)(_ stripPrefix _)
863864
// }
864865
val bSym = normalizeTemplate(aSym)
865-
if (bSym.isNonClassType) {
866+
if (bSym.isNonClassType && bSym != AnyRefClass) {
866867
nameBuffer append bSym.decodedName
867868
} else {
868869
val tpl = makeTemplate(bSym)

src/compiler/scala/tools/nsc/doc/model/diagram/Diagram.scala

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ abstract class Node {
7373
def isOtherNode = !(isClassNode || isTraitNode || isObjectNode)
7474
def isImplicitNode = false
7575
def isOutsideNode = false
76+
def tooltip: Option[String]
7677
}
7778

7879
// different matchers, allowing you to use the pattern matcher against any node
@@ -97,20 +98,20 @@ object OtherNode { def unapply(n: Node): Option[(TypeEntity, Option[TemplateEn
9798

9899

99100
/** The node for the current class */
100-
case class ThisNode(tpe: TypeEntity, tpl: Option[TemplateEntity]) extends Node { override def isThisNode = true }
101+
case class ThisNode(tpe: TypeEntity, tpl: Option[TemplateEntity], tooltip: Option[String] = None) extends Node { override def isThisNode = true }
101102

102103
/** The usual node */
103-
case class NormalNode(tpe: TypeEntity, tpl: Option[TemplateEntity]) extends Node { override def isNormalNode = true }
104+
case class NormalNode(tpe: TypeEntity, tpl: Option[TemplateEntity], tooltip: Option[String] = None) extends Node { override def isNormalNode = true }
104105

105106
/** A class or trait the thisnode can be converted to by an implicit conversion
106107
* TODO: I think it makes more sense to use the tpe links to templates instead of the TemplateEntity for implicit nodes
107108
* since some implicit conversions convert the class to complex types that cannot be represented as a single tmeplate
108109
*/
109-
case class ImplicitNode(tpe: TypeEntity, tpl: Option[TemplateEntity]) extends Node { override def isImplicitNode = true }
110+
case class ImplicitNode(tpe: TypeEntity, tpl: Option[TemplateEntity], tooltip: Option[String] = None) extends Node { override def isImplicitNode = true }
110111

111112
/** An outside node is shown in packages when a class from a different package makes it to the package diagram due to
112113
* its relation to a class in the template (see @contentDiagram hideInheritedNodes annotation) */
113-
case class OutsideNode(tpe: TypeEntity, tpl: Option[TemplateEntity]) extends Node { override def isOutsideNode = true }
114+
case class OutsideNode(tpe: TypeEntity, tpl: Option[TemplateEntity], tooltip: Option[String] = None) extends Node { override def isOutsideNode = true }
114115

115116

116117
// Computing and offering node depth information

src/compiler/scala/tools/nsc/doc/model/diagram/DiagramFactory.scala

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,16 @@ trait DiagramFactory extends DiagramDirectiveParser {
4343
// the diagram filter
4444
val diagramFilter = makeInheritanceDiagramFilter(tpl)
4545

46+
def implicitTooltip(from: DocTemplateEntity, to: TemplateEntity, conv: ImplicitConversion) =
47+
Some(from.qualifiedName + " can be implicitly converted to " + conv.targetType + " by the implicit method "
48+
+ conv.conversionShortName + " in " + conv.convertorOwner.kind + " " + conv.convertorOwner.qualifiedName)
49+
4650
val result =
4751
if (diagramFilter == NoDiagramAtAll)
4852
None
4953
else {
5054
// the main node
51-
val thisNode = ThisNode(tpl.ownType, Some(tpl))
55+
val thisNode = ThisNode(tpl.ownType, Some(tpl), Some(tpl.qualifiedName + " (this " + tpl.kind + ")"))
5256

5357
// superclasses
5458
var superclasses: List[Node] =
@@ -57,7 +61,10 @@ trait DiagramFactory extends DiagramDirectiveParser {
5761
}.reverse
5862

5963
// incoming implcit conversions
60-
lazy val incomingImplicitNodes = tpl.incomingImplicitlyConvertedClasses.map(tpl => ImplicitNode(tpl.ownType, Some(tpl)))
64+
lazy val incomingImplicitNodes = tpl.incomingImplicitlyConvertedClasses.map {
65+
case (incomingTpl, conv) =>
66+
ImplicitNode(incomingTpl.ownType, Some(incomingTpl), implicitTooltip(from=incomingTpl, to=tpl, conv=conv))
67+
}
6168

6269
// subclasses
6370
var subclasses: List[Node] =
@@ -67,7 +74,10 @@ trait DiagramFactory extends DiagramDirectiveParser {
6774
}.sortBy(_.tpl.get.name)(implicitly[Ordering[String]].reverse)
6875

6976
// outgoing implicit coversions
70-
lazy val outgoingImplicitNodes = tpl.outgoingImplicitlyConvertedClasses.map(pair => ImplicitNode(pair._2, Some(pair._1)))
77+
lazy val outgoingImplicitNodes = tpl.outgoingImplicitlyConvertedClasses.map {
78+
case (outgoingTpl, outgoingType, conv) =>
79+
ImplicitNode(outgoingType, Some(outgoingTpl), implicitTooltip(from=tpl, to=tpl, conv=conv))
80+
}
7181

7282
// TODO: Everyone should be able to use the @{inherit,content}Diagram annotation to change the diagrams.
7383
// Currently, it's possible to leave nodes and edges out, but there's no way to create new nodes and edges

0 commit comments

Comments
 (0)