Skip to content

Scala3doc: Add missing givens, create pages for givens, minor bugfixes #10306

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 16, 2020
Merged

Scala3doc: Add missing givens, create pages for givens, minor bugfixes #10306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 38 additions & 15 deletions scala3doc/src/dotty/dokka/tasty/ClassLikeSupport.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ trait ClassLikeSupport:

val graph = HierarchyGraph.withEdges(getSupertypesGraph(classDef, LinkToType(selfSiangture, classDef.symbol.dri, kindForClasslike(classDef.symbol))))
val baseExtra = PropertyContainer.Companion.empty()
.plus(ClasslikeExtension(classDef.getConstructorMethod, classDef.getCompanion))
.plus(ClasslikeExtension(classDef.getConstructorMethod(), classDef.getCompanion))
.plus(MemberExtension(
classDef.symbol.getVisibility(),
modifiers,
Expand Down Expand Up @@ -111,25 +111,55 @@ trait ClassLikeSupport:
val target = ExtensionTarget(extSym.symbol.name, extSym.tpt.dokkaType.asSignature, extSym.tpt.symbol.dri)
parseMethod(dd.symbol, kind = Kind.Extension(target))
}

// TODO check given methods?
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can check given methods as part of this PR?

case dd: DefDef if !dd.symbol.isHiddenByVisibility && dd.symbol.isGiven =>
Some(parseMethod(dd.symbol, kind = Kind.Given(getGivenInstance(dd).map(_.asSignature), None))) // TODO check given methods?
Some(dd.symbol.owner.typeMember(dd.name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can dd.symbol.owner.typeMember(dd.name) be a null? If not then simple if will be nicer to read:

if dd.symbol.owner.typeMember(dd.name).exisits then None
else Some(parseMethod(dd.symbol, kind = Kind.Given(getGivenInstance(dd).map(_.asSignature), None)))

.filterNot(_.exists)
.map { _ =>
parseMethod(dd.symbol, kind = Kind.Given(getGivenInstance(dd).map(_.asSignature), None))
}

case dd: DefDef if !dd.symbol.isHiddenByVisibility && !dd.symbol.isGiven && !dd.symbol.isSyntheticFunc && !dd.symbol.isExtensionMethod =>
Some(parseMethod(dd.symbol))

case td: TypeDef if !td.symbol.flags.is(Flags.Synthetic) && (!td.symbol.flags.is(Flags.Case) || !td.symbol.flags.is(Flags.Enum)) =>
Some(parseTypeDef(td))

case vd: ValDef if !isSyntheticField(vd.symbol)
&& (!vd.symbol.flags.is(Flags.Case) || !vd.symbol.flags.is(Flags.Enum))
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we can exctract all this conditions to dedicated method?

&& vd.symbol.isGiven =>
val classDef = Some(vd.tpt.tpe).flatMap(_.classSymbol.map(_.tree.asInstanceOf[ClassDef]))
Some(classDef.filter(_.symbol.flags.is(Flags.ModuleClass)).fold[Member](parseValDef(vd))(parseGivenClasslike(_)))

case vd: ValDef if !isSyntheticField(vd.symbol) && (!vd.symbol.flags.is(Flags.Case) || !vd.symbol.flags.is(Flags.Enum)) =>
Some(parseValDef(vd))

case c: ClassDef if c.symbol.owner.method(c.name).exists(_.flags.is(Flags.Given)) =>
Some(parseGivenClasslike(c))

case c: ClassDef if c.symbol.shouldDocumentClasslike && !c.symbol.isGiven =>
Some(parseClasslike(c))

case _ => None
)

private def parseGivenClasslike(c: ClassDef): Member = {
val parsedClasslike = parseClasslike(c)
val parentTpe = c.parents(0) match {
case t: TypeTree => Some(t.tpe)
case _ => None
}
val modifiedClasslikeExtension = ClasslikeExtension.getFrom(parsedClasslike).map(_.copy(
Copy link
Contributor

Choose a reason for hiding this comment

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

if you first call get and then copy you can get rid of map

constructor = c.getConstructorMethod(Some(_ => "using "))
)
).get
parsedClasslike.withNewExtras(
parsedClasslike.getExtra.plus(modifiedClasslikeExtension)
).withKind(
Kind.Given(parsedClasslike.directParents.headOption, parentTpe.flatMap(extractImplicitConversion))
)
}

private def parseInheritedMember(s: Tree): Option[Member] = processTreeOpt(s)(s match
case c: ClassDef if c.symbol.shouldDocumentClasslike && !c.symbol.isGiven => Some(parseClasslike(c, signatureOnly = true))
case other => parseMember(other)
Expand Down Expand Up @@ -178,9 +208,9 @@ trait ClassLikeSupport:
.filterNot(_.isHiddenByVisibility)
.map(_.dri)

def getConstructorMethod: Option[DFunction] =
def getConstructorMethod(paramModifierFunc: Option[Symbol => String] = None): Option[DFunction] =
Some(c.constructor.symbol).filter(_.exists).filterNot(_.isHiddenByVisibility).map( d =>
parseMethod(d, constructorWithoutParamLists(c), s => c.getParameterModifier(s))
parseMethod(d, constructorWithoutParamLists(c), paramModifierFunc.getOrElse(s => c.getParameterModifier(s)))
)

def parseClasslike(classDef: ClassDef, signatureOnly: Boolean = false)(using ctx: Context): DClass = classDef match
Expand Down Expand Up @@ -238,6 +268,7 @@ trait ClassLikeSupport:
val name = methodKind match
case Kind.Constructor => "this"
case Kind.Given(_, _) => methodSymbol.name.stripPrefix("given_")
case Kind.Extension(_) => methodSymbol.name.stripPrefix("extension_")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should no longer be necessary, the encoding of extension methods has changed and their names AFAIU are no longer prefixed with extension_.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the version of master that this PR was based on, there were still extension_ prefixes before every extension.

case _ => methodSymbol.name

new DFunction(
Expand Down Expand Up @@ -333,18 +364,10 @@ trait ClassLikeSupport:
)

def parseValDef(valDef: ValDef): DProperty =
def givenInstance = Some(valDef.symbol.moduleClass)
.filter(_.exists)
.map(_.tree.asInstanceOf[ClassDef])
.flatMap(_.getParents.headOption)
.map(_.dokkaType.asSignature)

def defaultKind = if valDef.symbol.flags.is(Flags.Mutable) then Kind.Var else Kind.Val
val kind =
if valDef.symbol.isGiven then Kind.Given(givenInstance, extractImplicitConversion(valDef.tpt.tpe))
else if valDef.symbol.flags.is(Flags.Implicit) then
val kind = if valDef.symbol.flags.is(Flags.Implicit) then
Kind.Implicit(Kind.Val, extractImplicitConversion(valDef.tpt.tpe))
else defaultKind
else defaultKind
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the indent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without indent there, I've got:

[error] 373 |      else defaultKind
[error]     |      ^
[error]     |The start of this line does not match any of the previous indentation widths.


new DProperty(
valDef.symbol.dri,
Expand Down
4 changes: 2 additions & 2 deletions scala3doc/src/dotty/dokka/tasty/SyntheticSupport.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ trait SyntheticsSupport:
def getAllMembers: List[Symbol] = hackGetAllMembers(qctx.reflect)(s)

def isSyntheticField(c: Symbol) =
c.flags.is(Flags.CaseAccessor) || c.flags.is(Flags.Object)
c.flags.is(Flags.CaseAccessor) || (c.flags.is(Flags.Object) && !c.flags.is(Flags.Given))

def isValidPos(pos: Position) =
pos.exists && pos.start != pos.end
Expand Down Expand Up @@ -69,7 +69,7 @@ trait SyntheticsSupport:
sym.typeRef.appliedTo(sym.typeParams.map(_.typeRef)).allMembers.iterator.map(_.symbol)
.collect {
case sym if
!sym.is(dotc.core.Flags.ModuleVal) &&
(!sym.is(dotc.core.Flags.ModuleVal) || sym.is(dotc.core.Flags.Given)) &&
!sym.flags.isAllOf(dotc.core.Flags.Enum | dotc.core.Flags.Case | dotc.core.Flags.JavaStatic) =>
sym.asInstanceOf[r.Symbol]
}.toList
Expand Down
4 changes: 2 additions & 2 deletions scala3doc/src/dotty/dokka/transformers/ImplicitMembersExtensionTransformer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ class ImplicitMembersExtensionTransformer(ctx: DokkaContext) extends Documentabl

def expandMember(outerMembers: Seq[Member])(c: Member): Member =
val companion = c match
case classlike: DClass => ClasslikeExtension.getFrom(classlike).flatMap(_.companion).map(classlikeMap)
case classlike: DClass => ClasslikeExtension.getFrom(classlike).flatMap(_.companion).flatMap(classlikeMap.get)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it expected that classlikeMap would miss some classlikes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh, actually I changed it on the beginning of implementing this PR because it was throwing exceptions, but I've checked and this change is unnecessary

case _ => None

val allParents = c.parents.flatMap(p => classlikeMap.get(p.dri))

val parentCompanions = allParents.flatMap {
case cls: DClasslike => ClasslikeExtension.getFrom(cls).flatMap(_.companion).map(classlikeMap)
case cls: DClasslike => ClasslikeExtension.getFrom(cls).flatMap(_.companion).flatMap(classlikeMap.get)
case _ => None
}

Expand Down
18 changes: 18 additions & 0 deletions scala3doc/src/dotty/dokka/translators/ScalaSignatureProvider.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ object ScalaSignatureProvider:
methodSignature(method, builder)
case enumEntry: DClass if enumEntry.kind == Kind.EnumCase =>
enumEntrySignature(enumEntry, builder)
case givenClazz: DClass if givenClazz.kind.isInstanceOf[Kind.Given] =>
givenClassSignature(givenClazz, builder)
case clazz: DClass =>
classSignature(clazz, builder)
case enumProperty: DProperty if enumProperty.kind == Kind.EnumCase =>
Expand Down Expand Up @@ -98,6 +100,22 @@ object ScalaSignatureProvider:
val extendPart = builder.text(" extends ").signature(extendType)
withTypes.foldLeft(extendPart)((bdr, tpe) => bdr.text(" with ").signature(tpe))

private def givenClassSignature(clazz: DClass, builder: SignatureBuilder): SignatureBuilder =
val ext = clazz.get(ClasslikeExtension)
val prefixes = builder
.modifiersAndVisibility(clazz, "given")
.name(clazz.getName, clazz.getDri)
.generics(clazz)

val withGenerics = ext.constructor.toSeq.foldLeft(prefixes){ (bdr, elem) =>
bdr.functionParameters(elem)
}
clazz.kind match
case Kind.Given(Some(instance), _) => withGenerics
.text(" as ")
.signature(instance)
case _ => withGenerics
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we have tests for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually tested by givenSignatures as we had these signatures documented, but in different way (we were generating signature from DFunction instead of DClass).


private def classSignature(clazz: DClass, builder: SignatureBuilder): SignatureBuilder =
val ext = clazz.get(ClasslikeExtension)
val prefixes = builder
Expand Down