Skip to content

Fixing a bunch of scaladoc issues#925

Merged
adriaanm merged 21 commits intoscala:2.10.xfrom
VladUreche:issue/scaladoc
Jul 20, 2012
Merged

Fixing a bunch of scaladoc issues#925
adriaanm merged 21 commits intoscala:2.10.xfrom
VladUreche:issue/scaladoc

Conversation

@VladUreche
Copy link
Contributor

closes SI-3314, SI-4888, SI-5235, SI-5558, SI-4324, SI-5780, SI-4887, SI-3695, SI-4224, SI-4497, SI-5079, SI-6073, SI-5533, SI-5784

and prepares scaladoc to properly handle reflection. Btw, this patch introduces good support for scaladoc links, so we shouldn't miss it for 2.10 ;)

More info: http://www.slideshare.net/VladUreche/scaladoc-reflection

Review by @heathermiller and @cvogt

The bug is related to a couple of other annoyances, also fixed:
 - usecases without type params were crashing scaladoc due to a change
in the PolyTypes class (not allowing empty tparams list)
 - properly getting rid of backticks (even if the link is not valid)
 - correct linking for usecases with $Coll = `immutable.Seq`
(the symbol searching algorithm was too of restrictive, now we search
the entire ownerchain - and the empty package at the end)
 - give a warning if the type lookup fails
 - finally, added a $Coll variable to List, for some reason it wasn't
there and we were getting immutable.Seq as the result of use cases.
And adds support for linking to class members, only usable from the
model factory now, so no links to members from the doc comment yet,
sorry. But it fixes the Enumeration problem once and for all!

Also corrected the inTpl for members obtained by implicit conversions,
so they're in the correct template and the comment variable expansion
is done from the correct (but different) template.

Review by @kzys.
Related to SI-3314, where we started showing inherited templates
that were normally not documented. This patch corrects a problem
in parentTypes that was preventing inherited templates from being
displayed in diagrams.

Also renamed:
PackageDiagram => ContentDiagram
ClassDiagram => InheritanceDiagram
which should have been done much earlier
This bug was fixed by the model upgrade in c11427c. Test case confirmation.
This bug was fixed by the model upgrade in c11427c. Test case confirmation.
case class C(i: Int)(b: Boolean) would appear uncurried in scaladoc:
case class C(i: Int, b: Boolean)
SI-3448 partial fix:

This enables a workaround for the fact that Map takes two type params
while $Coll takes only one. But it doesn't fix the problem though,
as there's one more piece missing from the puzzle - we need to adjust
the `Coll`s in {immutable, mutable, concurrent}.Map to something that
makes sense for the usecase. And that's not possible. But I'm
committing this nevertheless, maybe other projects can benefit from it.

And for SI-3448, the solution lies in automatic usecase generation,
whenever that will be ready.
This was a long-standing issue in scaladoc: It was unable to
disambiguate between entries with the same name. One example is:
immutable.Seq:

trait Seq[+A] extends Iterable[A] with Seq[A] ...

What's that? Seq extends Seq? No, immutable.Seq extends collection.Seq,
but scaladoc was unable to show that. Now it does, depending on the
template you're in. Prefixes are relative and can go back:
-scala.collection.Seq has subclasses *immutable.Seq* and *mutable.Seq*
-scala.immutable.Seq extends *collection.Seq*

Unfortunately the price we pay for this is high, a 20% slowdown in
scaladoc. This is why there is a new flag called -no-prefixes that
disables the prefixes in front of types.

Btw, it also fixes the notorious "booleanValue: This member is added by
an implicit conversion from Boolean to Boolean ...". That's now
java.lang.Boolean, so it becomes clear.

Conflicts:

	src/compiler/scala/tools/nsc/doc/model/diagram/DiagramFactory.scala
As suggested by Heather in pull request scala#816.
Based on an inital patch by Nada Amin. Rewrote some of the code to
compress it a bit.

Also fixed a problem that was affecting the prefix printing for
typerefs: type parameter and existentials won't get prefixes. While
I can imagine cases where you'd want to see where they come from,
you can always hover over them and see their origin.

Also added support for pretty-printing ThisTypes, SuperTypes and
SingleTypes (with links)
Adds the ability to link to members, classes and objects in scaladoc.
The links can now be either qualified names or relative names, they
both work. See the test/scaladoc/resources/links.scala for a usage
example. Also introduced -no-link-warnings scaladoc flag, in case the
build output gets swamped with link warnings.
 - diagrams are not stored because they create many TypeEntities, nodes
and edges -- now they are created on demand, so make sure you don't
demand them twice!
 - eliminated the type entity cache, which was nearly useless (6s gain)
but was preventing the GC from eliminating TypeEntities
 - an unsuccessful attempt at reducing the tons of :: garbage we're
generating - there's 200MB-250MB of ::s during a typical 'ant docs.lib'
 - got rid of references to WikiParser when creating EntityLinks
The known type class descriptions give you a humanly-understandable
explanation for [T: TypeClass] in the implicit conversions. For
example [T: Integral] will tell you that T must be an integral type
such as Integer and co.

Now that the reflection dust settled, I can actually add the test to
make sure the well-known type classes are intercepted and explanations
are given instead of the usual "the type T is context-bounded by
TypeClass"
for SI-5784. This commit has been checked with tools/scaladoc-compare
and the only difference is that the containing entities in the index
are not duplicate anymore, which solves yet another bug we did not
know about. :)
Normally scaladoc won't generate template pages for anything other than
packages, classes, traits and objects. But using the @template
annotation on {abstract,alias} types, they get their own page and take
part as full members in the diagrams. Furthermore, when looking for the
companion object, if a value of type T is in scope, T will be taken as
the companion object (even though it might be a class)

All templates, including types are listed on the left navigation pane,
so now adding @template to String can get scaladoc to generate (a
no-comments) page for java.lang.String.

The {abstract, alias} type icons need to be updated -- I just took the
class icons and added a small x to them -- but they shoud be something
else (maybe an underscore?)i

TO USE THIS PATCH:
<pre>
/** @contentDiagram */ // tells scaladoc to create a diagram of the
                       // templates contained in trait Base
trait Base {
  /** @template */ // tells scaladoc to create a page for Foo
  type T < Foo
  trait Foo { def foo: Int }
}

/** @contentDiagram */
trait Api extends Base {
  /** @template */
  override type T <: FooApi
  trait FooApi extends Foo { def bar: String }
}
</pre>
@ghost ghost assigned heathermiller Jul 17, 2012
@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/564/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/564/

@xeno-by
Copy link
Contributor

xeno-by commented Jul 18, 2012

Wow what a sick amount of work!

@adriaanm
Copy link
Contributor

yes, great job, @VladUreche!

@VladUreche
Copy link
Contributor Author

@xeno-by Look who's talking, mr. 15KLOC pull request ;)
@adriaanm Have you had a chance to check the change to typers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better to simply replace typeFun by genPolyType, which accepts empty type params

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks!

@SethTisue
Copy link
Member

EPIC

as requested by @adriaanm on pull request scala#925
@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/564/

@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/564/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a missing (2) on the diagram?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry, it's not very clear: (2) is implied by the inclusion of one box in the other (ownership relation)
I'll make it clear in the next pull request.

Group class members based on their semantic relationship. To do this:
 - @group on members, only need to do it for the non-overridden members
 - -groups flag passes to scaladoc, groups="on" in ant
 - @groupdesc Group Group Description to add descriptions
 - @groupName Group New name for group
 - @groupprio Group <int> (lower is better)
See test/scaladoc/run/groups.scala for a top-to-bottom example
@VladUreche
Copy link
Contributor Author

Added two commits as discussed at the reflection meeting: groups and @documentable.

@scala-jenkins
Copy link

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/606/

And relaxed the groups name/description/priority search algorithm to
work for types.
@scala-jenkins
Copy link

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/606/

@retronym
Copy link
Member

Can you please update the Scaladoc documentation after this is released?

https://wiki.scala-lang.org/display/SW/Tags+and+Annotations

The IntelliJ parser will need to be updated to know about the new tags and the expanded link syntax; it would be really useful if you can also document the grammar of the links.

@VladUreche
Copy link
Contributor Author

Hi Jason,
Yes, we're planning on a new release of the scaladoc documentation, it should be out a couple of weeks before the release. Have you seen the @template/@documentable annotation? It's exactly the thing you described in the scalaz type Id example.

@heathermiller
Copy link
Member

Note to @adriaanm-- I've been intermittently reviewing this, as far as I can see, LGTM
I might only have a few trivial suggestions (mostly on documentation and tidying) that could come as a later clean up, but nothing that should prevent this from going in :-)

@adriaanm
Copy link
Contributor

ok, just label this as reviewed when you're ready to sign off -- it'd be great if that can be by EOB today (Scala Standard Time)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(PR already marked as reviewed, these are just "clean-up" related questions we might want to consider before 2.10-FINAL)

(I know this line is not the best one to drop this comment on, but...) What do you think about hiding the group/section on shadowed or ambiguous implicits? I was thinking that maybe it'd be best even under a flag than even hidden under the toggle menu.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about hiding them completely -- they can still be called, so it might be best to use a toggle menu that is closed by default.

@adriaanm
Copy link
Contributor

great, merging, then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants