Skip to content
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

Avoid NoPosition.start in Scaladoc warning #10721

Merged
merged 1 commit into from Apr 6, 2024

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Mar 21, 2024

Some docs for synthetic symbols derived from library-aux sources have no position (for some reason).

Also for some reason, operations on position are brittle instead of safe. I'd expect to map a position such that the degenerate position remains safely degenerate instead of throwing (which helps no one). After solving NoSymbol has no owner, they ought to have tackled NoPosition has no start.

This commit just patches the warning not to do position arithmetic on NoPosition.

It requires restarr to work for the OP.

Edit: this commit also doesn't address "doc variable resolution in code block".

Edit: Fixes scala/bug#12964

@scala-jenkins scala-jenkins added this to the 2.13.15 milestone Mar 21, 2024
@som-snytt som-snytt modified the milestones: 2.13.15, 2.13.14 Mar 21, 2024
@SethTisue SethTisue added the prio:hi high priority (used only by core team, only near release time) label Mar 21, 2024
@SethTisue
Copy link
Member

It requires restarr to work for the OP.

And that's why it makes sense to squeeze this into 2.13.14?

@som-snytt
Copy link
Contributor Author

This is a blocker for work on the PR for documentation of Any#asInstanceOf.

At least, it's a mental blocker.

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

degenerate position remains safely degenerate instead of throwing

Not sure for start / point / end that return Int. What should they do? pos.withPoint(pos.start + vstart + 1) giving an arbitrary position is certainly harder to debug than crashing.

This PR LGTM.

@som-snytt
Copy link
Contributor Author

Tested manually with restarr, by adding a doc var in the code block in the doc for Any.

--- a/src/library-aux/scala/Any.scala
+++ b/src/library-aux/scala/Any.scala
@@ -29,6 +29,7 @@ package scala
  *
  *     val w = new Wrapper(3)
  *     w.print()
+ *     val x = s"Sample bad doc macro in code block: $x"
  * }}}
  *
  * See the [[https://docs.scala-lang.org/overviews/core/value-classes.html Value Classes and Universal Traits]] for more

then library/doc shows a weirdly unpositioned message (but doesn't crash):

[warn] .../scala/src/library/scala/collection/LinearSeq.scala:48:7: The comment for method head contains @inheritdoc, but no parent comment is available to inherit from.
[warn]   def head: A
[warn]       ^
[warn] Variable x undefined in comment for class Any in class Any
[warn] .../scala/src/library/scala/math/Ordering.scala:548:10: Variable doubleOrdering undefined in comment for trait IeeeOrdering in trait IeeeOrdering
[warn]       * $doubleOrdering
[warn]          ^

I confirmed that the diff crashes as reported with untweaked 2.13.x. What starr is that.

Just remembered "What star is that?" was my first astronomy guide.
https://www.abebooks.com/book-search/title/what-star-is-that/author/peter-lancaster-brown/

@som-snytt som-snytt merged commit 1120346 into scala:2.13.x Apr 6, 2024
3 checks passed
@som-snytt som-snytt deleted the issue/12964-doc-asInstanceOf branch April 6, 2024 19:30
@SethTisue SethTisue removed the prio:hi high priority (used only by core team, only near release time) label Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants