-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Cleaning up tests in scala3doc #10170
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
Changes from all commits
9a8f8b6
a8cff71
a3e53f6
13550b1
a5c9ce9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package tests | ||
|
||
package complexNames | ||
|
||
abstract class A: | ||
def ++(other: A): A | ||
def +:(other: Int): A | ||
def :+(other: Int): A | ||
|
||
// scala3doc has problems with names in backticks | ||
// def `multi word name`: Int | ||
// def `*** name with arbitrary chars ^%`: Int | ||
// def `mischievous(param:Int)`(otherParam: Int): String | ||
// def withMischievousParams(`param: String, param2`: String): String | ||
|
||
def complexName_^*(param: String): A | ||
|
||
def `completelyUnnecessaryBackticks`: Int //expected: def completelyUnnecessaryBackticks: Int | ||
def `+++:`(other: Int): A //expected: def +++:(other: Int): A | ||
def `:+++`(other: Int): A //expected: def :+++(other: Int): A | ||
|
||
def `abc_^^_&&`: A //expected: def abc_^^_&&: A | ||
def `abc_def`: A //expected: def abc_def: A | ||
def `abc_def_++`: A //expected: def abc_def_++: A | ||
// def `++_abc`: A | ||
// def `abc_++_--`: A |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
package dotty.dokka | ||
|
||
import org.jetbrains.dokka.plugability.DokkaContext | ||
import org.jetbrains.dokka.model.{DModule, WithChildren} | ||
import org.jetbrains.dokka.pages.RootPageNode | ||
import org.jetbrains.dokka.testApi.testRunner.{DokkaTestGenerator, TestMethods} | ||
import org.jetbrains.dokka.testApi.logger.TestLogger | ||
import org.jetbrains.dokka.utilities.DokkaConsoleLogger | ||
import org.jetbrains.dokka.DokkaConfiguration | ||
import scala.jdk.CollectionConverters.{ListHasAsScala, SeqHasAsJava} | ||
import org.junit.{Test, Rule} | ||
import org.junit.rules.{TemporaryFolder, ErrorCollector} | ||
import java.io.File | ||
|
||
abstract class ScaladocTest(val name: String): | ||
def assertions: Seq[Assertion] | ||
|
||
private def getTempDir() : TemporaryFolder = | ||
val folder = new TemporaryFolder() | ||
folder.create() | ||
folder | ||
|
||
private def args = Args( | ||
name = "test", | ||
tastyRoots = Nil , | ||
classpath = System.getProperty("java.class.path"), | ||
None, | ||
output = getTempDir().getRoot, | ||
projectVersion = "1.0", | ||
projectTitle = None, | ||
projectLogo = None, | ||
defaultSyntax = None, | ||
sourceLinks = Nil | ||
) | ||
|
||
private def tastyFiles = | ||
def listFilesSafe(dir: File) = Option(dir.listFiles).getOrElse { | ||
throw AssertionError(s"$dir not found. The test name is incorrect or scala3doc-testcases were not recompiled.") | ||
} | ||
def collectFiles(dir: File): List[String] = listFilesSafe(dir).toList.flatMap { | ||
case f if f.isDirectory => collectFiles(f) | ||
case f if f.getName endsWith ".tasty" => f.getAbsolutePath :: Nil | ||
case _ => Nil | ||
} | ||
collectFiles(File(s"${BuildInfo.test_testcasesOutputDir}/tests/$name")) | ||
|
||
@Rule | ||
def collector = _collector | ||
private val _collector = new ErrorCollector(); | ||
def reportError(msg: String) = collector.addError(new AssertionError(msg)) | ||
|
||
@Test | ||
def executeTest = | ||
DokkaTestGenerator( | ||
DottyDokkaConfig(DocConfiguration.Standalone(args, tastyFiles, Nil)), | ||
TestLogger(DokkaConsoleLogger.INSTANCE), | ||
assertions.asTestMethods, | ||
Nil.asJava | ||
).generate() | ||
|
||
end ScaladocTest | ||
|
||
type Validator = () => Unit | ||
|
||
/** | ||
* Those assertions map 1-1 to their dokka counterparts. Some of them may be irrelevant in scala3doc. | ||
*/ | ||
enum Assertion: | ||
case AfterPluginSetup(fn: DokkaContext => Unit) | ||
case DuringValidation(fn: Validator => Unit) | ||
case AfterDocumentablesCreation(fn: Seq[DModule] => Unit) | ||
case AfterPreMergeDocumentablesTransformation(fn: Seq[DModule] => Unit) | ||
case AfterDocumentablesMerge(fn: DModule => Unit) | ||
case AfterDocumentablesTransformation(fn: DModule => Unit) | ||
case AfterPagesGeneration(fn: RootPageNode => Unit) | ||
case AfterPagesTransformation(fn: RootPageNode => Unit) | ||
case AfterRendering(fn: (RootPageNode, DokkaContext) => Unit) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should turn |
||
|
||
extension (s: Seq[Assertion]): | ||
def asTestMethods: TestMethods = | ||
import Assertion._ | ||
TestMethods( | ||
(context => s.collect { case AfterPluginSetup(fn) => fn(context) }.kUnit), | ||
(validator => s.collect { case DuringValidation(fn) => fn(() => validator.invoke()) }.kUnit), | ||
(modules => s.collect { case AfterDocumentablesCreation(fn) => fn(modules.asScala.toSeq) }.kUnit), | ||
(modules => s.collect { case AfterPreMergeDocumentablesTransformation(fn) => fn(modules.asScala.toSeq) }.kUnit), | ||
(module => s.collect { case AfterDocumentablesMerge(fn) => fn(module)}.kUnit), | ||
(module => s.collect { case AfterDocumentablesTransformation(fn) => fn(module) }.kUnit), | ||
(root => s.collect { case AfterPagesGeneration(fn) => fn(root) }.kUnit), | ||
(root => s.collect { case AfterPagesTransformation(fn) => fn(root) }.kUnit), | ||
((root, context) => s.collect { case AfterRendering(fn) => fn(root, context)}.kUnit) | ||
) | ||
|
||
extension [T] (s: T): | ||
private def kUnit = kotlin.Unit.INSTANCE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice one, probably we can unified the |
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.
We probably need a issue for it
cc @nicolasstucki
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.
what are we missing here?
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.
Print a name that needs backticks in backticks, I believe.
This makes sense to include in Tasty-Reflect, since the general concept is being able to print a name in a form that corresponds to how it was entered in source. I'm not certain what's the best way to do that, though.
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.
We will need to introduce a
Name
abstraction in the reflection API. A String loses too much information about the name. We cannot distinguish semantic names from simple names and we cannot know if they were backticked. That is on my TODO list.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.
Probably for now we can just fix that in scala3doc: lampepfl/scala3doc#235 (comment)
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.
@Kordyjan I'd rather avoid the code duplication, but I suppose that the regex for Scala identifiers that don't need a backtick is unlikely to change anytime soon. Can you wrap the code for detecting whether an identifier needs a backtick in a function whose name starts with
hack
, so we can grep for it easily? We have a bunch of functions like that in the doctool already.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.
Actually, we won't have that info. The scaladoc will need to figure it out.
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.
@abgruszecki: #10199 is ready.