Skip to content

quoted, jdk (6): Add Scaladoc comments for undocumented entities - #26903

Draft
bvenners wants to merge 1 commit into
scala:mainfrom
artimahub:scaladoc-missing-docs-quoted-jdk
Draft

bvenners wants to merge 1 commit into
scala:mainfrom
artimahub:scaladoc-missing-docs-quoted-jdk

Conversation

@bvenners

Copy link
Copy Markdown
Contributor

This PR fills in a main doc comment plus @PARAM, @tparam, and @return tags for scala.jdk, scala.quoted and scala.quoted.runtime APIs that are completely missing any Scaladoc documentation. Most of it is quoted/Quotes.scala, which alone accounts for 285 newly documented declarations across the reflection API; the rest covers the scala.jdk accumulators and converters, Expr, ExprMap, FromExpr and Type. I'm submitting it as a draft PR so that I can get the CI to run on it, to see if it breaks anything, and to start getting feedback. We automated the generation of these changes and have not reviewed all of them yet. We will review them all before making the PR non-draft. Please let me know whether you think this is going in the right direction in general, and anything specific that you notice that could be improved.

@bvenners
bvenners force-pushed the scaladoc-missing-docs-quoted-jdk branch from 22bc2a1 to 17ea7d3 Compare September 9, 2026 05:27

@cheeseng cheeseng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

First batch of human review.


/** Methods of the module object `val ExprMatch` */
trait ExprMatchModule { self: ExprMatch.type =>
/** Pattern matches an the scrutineeExpr against the patternExpr and returns a tuple

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not in our change set, but I think this should be 'scrutinee' and 'pattern' instead of 'scrutineeExpr' and 'patternExpr'.

@@ -3850,8 +4780,19 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching =>
/** Methods of the module object `val MatchCase`. */
trait MatchCaseModule { this: MatchCase.type =>
/* Create match type case `case <pattern> => <rhs>` */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's old comment here, it didn't get removed after adding the new comment.

* @param pattern the pattern type `P`
* @param rhs the result type `R`
* @return a new `MatchCase`
*/
def apply(pattern: TypeRepr, rhs: TypeRepr): MatchCase
/* Matches a match type case `case <pattern> => <rhs>` */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's old comment here, it didn't get removed after adding the new comment.

/** Returns the number of accumulated elements as an `Int`.
*
* @throws IllegalArgumentException if this accumulator holds `Int.MaxValue` or more elements, in
* which case [[sizeLong]] has to be used instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the second part does not sound right, may be "else return sizeLong.toInt" instead?

@cheeseng cheeseng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Second batchers of human review comments.

class RichBiConsumerAsFunction2[T, U](private val underlying: java.util.function.BiConsumer[T, U]) extends AnyVal {
/** Returns a Scala `Function2` that calls `underlying`, or, if `underlying` is an `AsJavaBiConsumer`, the Scala function that wrapper holds. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For this part:

..., the Scala function that wrapper holds.

I think it should be:

..., the Scala function that the wrapper holds.

or

..., the Scala function that this wrapper holds.

This unfortunately has many occurences in this PR, may use replace tool to replace others perhaps.


/** Provides the lowest-priority implicit conversion from a Scala function to a Java functional
* interface wrapper. It applies only when no conversion declared in
* [[Priority2FunctionExtensions]] or one of its subtraits does, so that a Scala function is

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The 'subtraits does' is a bit awkward, may be replaced with:

"It applies only when no conversion defined in [[Priority2FunctionExtensions]] or its subtraits is applicable, so a Scala function is enriched with the most specific Java function type that fits it."

if (sizeLong < Int.MaxValue) sizeLong.toInt
else throw new IllegalArgumentException(s"Size too large for an Int: $sizeLong")

/** Returns the number of accumulated elements, or `-1` once this accumulator holds

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

May be more concise if we follow the length doc:

"Returns the number of accumulated elements, or -1 if the accumulator holds Int.MaxValue or more of them, in which case [sizeLong] has to be used instead."

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.

2 participants