Skip to content

Commit

Permalink
Fix scaladoc TastyInspector regressions (#21716)
Browse files Browse the repository at this point in the history
Adds an `inspectAllTastyFilesInContext` method in scaladoc-only version
of TastyInspector in an effort to fix regressions introduced by
the switch to the new TastyInspector.

Notably, those issues are not being fixed in the stable TastyInspector -
this is scaladoc only.

Also the scaladoc copy of the TastyInspector had to be renamed, as
otherwise we run into classpath issues with the stable tasty inspector
taking precedence.

I really wanted to include the test for both issues, but both required
non-standard specification, so I could not use `scaladoc-testcases` and
ended up having to use sbt scripted tests unfortunately.

---------

Co-authored-by: Mikołaj Fornal <[email protected]>
  • Loading branch information
jchyb and Florian3k authored Oct 8, 2024
1 parent a3015f4 commit 21a7cbd
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 14 deletions.
9 changes: 9 additions & 0 deletions sbt-test/sbt-dotty/scaladoc-regressions/build.sbt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
ThisBuild / scalaVersion := sys.props("plugin.scalaVersion")

lazy val i20476 = project
.in(file("i20476"))
.enablePlugins(ScalaJSPlugin)

lazy val i18231 = project
.in(file("i18231"))
.settings(scalacOptions += "-release:8")
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object Foo {
@Deprecated
def foo(): Unit = ???
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package demo

import scala.scalajs.js

def bar: js.Promise[Int] = js.Promise.resolve(()).`then`(_ => 1)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
addSbtPlugin("org.scala-js" % "sbt-scalajs" % sys.props("plugin.scalaJSVersion"))
2 changes: 2 additions & 0 deletions sbt-test/sbt-dotty/scaladoc-regressions/test
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
> i18231/doc
> i20476/doc
4 changes: 2 additions & 2 deletions scaladoc/src/dotty/tools/scaladoc/tasty/TastyParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package tasty
import java.util.regex.Pattern

import scala.util.{Try, Success, Failure}
import scala.tasty.inspector.{TastyInspector, Inspector, Tasty}
import scala.tasty.inspector.{ScaladocInternalTastyInspector, Inspector, Tasty}
import scala.quoted._

import dotty.tools.dotc
Expand Down Expand Up @@ -160,7 +160,7 @@ object ScaladocTastyInspector:
report.error("File extension is not `tasty` or `jar`: " + invalidPath)

if tastyPaths.nonEmpty then
TastyInspector.inspectAllTastyFiles(tastyPaths, jarPaths, classpath)(inspector)
ScaladocInternalTastyInspector.inspectAllTastyFilesInContext(tastyPaths, jarPaths, classpath)(inspector)(using ctx.compilerContext)

val all = inspector.topLevels.result()
all.groupBy(_._1).map { case (pckName, members) =>
Expand Down
43 changes: 32 additions & 11 deletions scaladoc/src/scala/tasty/inspector/TastyInspector.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copy of tasty-inspector/src/scala/tasty/inspector/TastyInspector.scala
// Renamed copy of tasty-inspector/src/scala/tasty/inspector/TastyInspector.scala
// FIXME remove this copy of the file
// Since copying, an inspectAllTastyFilesInContext method was added for scaladoc only
// to fix regressions introduced by the switch from old to a new TastyInspector

package scala.tasty.inspector

Expand All @@ -21,7 +23,7 @@ import dotty.tools.dotc.report

import java.io.File.pathSeparator

object TastyInspector:
object ScaladocInternalTastyInspector:

/** Load and process TASTy files using TASTy reflect
*
Expand All @@ -41,6 +43,32 @@ object TastyInspector:
def inspectTastyFilesInJar(jar: String)(inspector: Inspector): Boolean =
inspectAllTastyFiles(Nil, List(jar), Nil)(inspector)

private def checkFiles(tastyFiles: List[String], jars: List[String]): Unit =
def checkFile(fileName: String, ext: String): Unit =
val file = dotty.tools.io.Path(fileName)
if !file.ext.toLowerCase.equalsIgnoreCase(ext) then
throw new IllegalArgumentException(s"File extension is not `.$ext`: $file")
else if !file.exists then
throw new IllegalArgumentException(s"File not found: ${file.toAbsolute}")
tastyFiles.foreach(checkFile(_, "tasty"))
jars.foreach(checkFile(_, "jar"))

/**
* Added for Scaladoc-only.
* Meant to fix regressions introduces by the switch from old to new TastyInspector:
* https://github.com/scala/scala3/issues/18231
* https://github.com/scala/scala3/issues/20476
* Stable TastyInspector API does not support passing compiler context.
*/
def inspectAllTastyFilesInContext(tastyFiles: List[String], jars: List[String], dependenciesClasspath: List[String])(inspector: Inspector)(using Context): Boolean =
checkFiles(tastyFiles, jars)
val classes = tastyFiles ::: jars
classes match
case Nil => true
case _ =>
val reporter = inspectorDriver(inspector).process(inspectorArgs(dependenciesClasspath, classes), summon[Context])
!reporter.hasErrors

/** Load and process TASTy files using TASTy reflect
*
* @param tastyFiles List of paths of `.tasty` files
Expand All @@ -50,14 +78,7 @@ object TastyInspector:
* @return boolean value indicating whether the process succeeded
*/
def inspectAllTastyFiles(tastyFiles: List[String], jars: List[String], dependenciesClasspath: List[String])(inspector: Inspector): Boolean =
def checkFile(fileName: String, ext: String): Unit =
val file = dotty.tools.io.Path(fileName)
if !file.ext.toLowerCase.equalsIgnoreCase(ext) then
throw new IllegalArgumentException(s"File extension is not `.$ext`: $file")
else if !file.exists then
throw new IllegalArgumentException(s"File not found: ${file.toAbsolute}")
tastyFiles.foreach(checkFile(_, "tasty"))
jars.foreach(checkFile(_, "jar"))
checkFiles(tastyFiles, jars)
val files = tastyFiles ::: jars
inspectFiles(dependenciesClasspath, files)(inspector)

Expand Down Expand Up @@ -124,4 +145,4 @@ object TastyInspector:
end inspectFiles


end TastyInspector
end ScaladocInternalTastyInspector
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class LinkWarningsTest extends ScaladocTest("noLinkWarnings"):

override def runTest = afterRendering {
val diagnostics = summon[DocContext].compilerContext.reportedDiagnostics
assertEquals("There should be exactly one warning", 1, diagnostics.warningMsgs.size)
val filteredWarnings = diagnostics.warningMsgs.filter(_ != "1 warning found")
assertEquals("There should be exactly one warning", 1, filteredWarnings.size)
assertNoErrors(diagnostics)
}

0 comments on commit 21a7cbd

Please sign in to comment.