search

scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Scaladoc crash on invalid table syntax #11986

Closed dragos closed 2 years ago

dragos commented 4 years ago

reproduction steps

/**
 * +--------------------------------------------------------------------------------+
 */
class Foo

problem

There are a lot of sys.exit calls in the table parser, I think none of them should crash the scaladoc tool. Also, I couldn't find any mention of scaladoc table syntax. :)

model contains 2 documentable templates
scaladoc.scala:1: warning: Fixing missing delimiter row
/**
^
scaladoc.scala:1: warning: unexpected table row markdown
/**
^
peek: parseCell0: '
|              V                       |                   '
scaladoc.scala:1: warning: Dropping 1 excess table data cells from row.
/**
^
scaladoc.scala:1: warning: Fixing invalid column alignment: (DataFrame)
/**
^
scaladoc.scala:1: warning: Fixing invalid column alignment: (data.frame)
/**
^
peek: expected-newline-missing: '
|              V                       |                   '
5 warnings found
Exception in thread "main" java.lang.RuntimeException: table parsing left buffer in unexpected state
    at scala.sys.package$.error(package.scala:30)
    at scala.tools.nsc.doc.base.CommentFactoryBase$WikiParser.table(CommentFactoryBase.scala:821)
    at scala.tools.nsc.doc.base.CommentFactoryBase$WikiParser.block(CommentFactoryBase.scala:461)
    at scala.tools.nsc.doc.base.CommentFactoryBase$WikiParser.document(CommentFactoryBase.scala:444)
    at scala.tools.nsc.doc.base.CommentFactoryBase.parseWikiAtSymbol(CommentFactoryBase.scala:428)
    at scala.tools.nsc.doc.base.CommentFactoryBase.parseWikiAtSymbol$(CommentFactoryBase.scala:428)
    at scala.tools.nsc.doc.DocFactory$$anon$2.parseWikiAtSymbol(DocFactory.scala:75)
    at scala.tools.nsc.doc.base.CommentFactoryBase.parse0$1(CommentFactoryBase.scala:387)
    at scala.tools.nsc.doc.base.CommentFactoryBase.parseAtSymbol(CommentFactoryBase.scala:418)
    at scala.tools.nsc.doc.base.CommentFactoryBase.parseAtSymbol$(CommentFactoryBase.scala:209)
    at scala.tools.nsc.doc.DocFactory$$anon$2.parseAtSymbol(DocFactory.scala:75)
    at scala.tools.nsc.doc.model.CommentFactory.parse(CommentFactory.scala:92)
    at scala.tools.nsc.doc.model.CommentFactory.parse$(CommentFactory.scala:90)
    at scala.tools.nsc.doc.DocFactory$$anon$2.parse(DocFactory.scala:75)
    at scala.tools.nsc.doc.model.CommentFactory.defineComment(CommentFactory.scala:82)
    at scala.tools.nsc.doc.model.CommentFactory.defineComment$(CommentFactory.scala:45)
    at scala.tools.nsc.doc.DocFactory$$anon$2.defineComment(DocFactory.scala:75)
    at scala.tools.nsc.doc.model.CommentFactory.$anonfun$comment$1(CommentFactory.scala:38)
    at scala.collection.mutable.HashMap.getOrElseUpdate(HashMap.scala:86)
    at scala.tools.nsc.doc.model.CommentFactory.comment(CommentFactory.scala:38)
    at scala.tools.nsc.doc.model.CommentFactory.comment$(CommentFactory.scala:36)
    at scala.tools.nsc.doc.DocFactory$$anon$2.comment(DocFactory.scala:75)
    at scala.tools.nsc.doc.model.ModelFactory$MemberImpl.comment$lzycompute(ModelFactory.scala:118)
    at scala.tools.nsc.doc.model.ModelFactory$MemberImpl.comment(ModelFactory.scala:118)
    at scala.tools.nsc.doc.html.page.EntityPage.memberToCommentBodyHtml(Entity.scala:491)
    at scala.tools.nsc.doc.html.page.EntityPage.memberToCommentBodyHtml$(Entity.scala:487)
    at scala.tools.nsc.doc.html.page.EntityPage$$anon$1.memberToCommentBodyHtml(Entity.scala:1128)
    at scala.tools.nsc.doc.html.page.EntityPage.memberToCommentHtml(Entity.scala:451)
    at scala.tools.nsc.doc.html.page.EntityPage.memberToCommentHtml$(Entity.scala:442)
    at scala.tools.nsc.doc.html.page.EntityPage$$anon$1.memberToCommentHtml(Entity.scala:1128)
    at scala.tools.nsc.doc.html.page.EntityPage.memberToHtml(Entity.scala:430)
    at scala.tools.nsc.doc.html.page.EntityPage.memberToHtml$(Entity.scala:417)
    at scala.tools.nsc.doc.html.page.EntityPage$$anon$1.memberToHtml(Entity.scala:1128)
    at scala.tools.nsc.doc.html.page.EntityPage.$anonfun$content$7(Entity.scala:324)
    at scala.tools.nsc.doc.html.page.EntityPage.$init$(Entity.scala:324)
    at scala.tools.nsc.doc.html.page.EntityPage$$anon$1.<init>(Entity.scala:1128)
    at scala.tools.nsc.doc.html.page.EntityPage$.apply(Entity.scala:1128)
    at scala.tools.nsc.doc.html.HtmlFactory.writeTemplates(HtmlFactory.scala:168)
    at scala.tools.nsc.doc.html.HtmlFactory.generate(HtmlFactory.scala:155)
    at scala.tools.nsc.doc.html.Doclet.generateImpl(Doclet.scala:30)
    at scala.tools.nsc.doc.doclet.Generator.generate(Generator.scala:35)
    at scala.tools.nsc.doc.DocFactory.generate$1(DocFactory.scala:135)
    at scala.tools.nsc.doc.DocFactory.document(DocFactory.scala:138)
    at scala.tools.nsc.ScalaDoc.process(ScalaDoc.scala:47)
    at scala.tools.nsc.ScalaDoc$.main(ScalaDoc.scala:98)
    at scala.tools.nsc.ScalaDoc.main(ScalaDoc.scala)
SethTisue commented 4 years ago

@janekdb interested in looking at this...?

janekdb commented 3 years ago

I will try to take a look. One benefit of the sys.exit calls is in the increased chance of generating a bug report :)

danicheg commented 2 years ago

I can't reproduce the problem on Scala 2.12.16, 2.13.8, and 3.1.3. @SethTisue could you also check this and close if don't reproduce it?

SethTisue commented 2 years ago

The current sources for table-sourcing have a number of calls to sys.error, but none to sys.exit (or System.exit).

And when I look at the original PR where table parsing first landed (https://github.com/scala/scala/pull/6043), I don't see any exit calls in the diffs there, either. So perhaps Iulian meant sys.error.

User error (such as invalid Scaladoc syntax) should never result in sys.error being called. So if any of the sys.error calls are actually triggerable by user input, we still have bugs here, because user error should result in a proper error, not just a stack trace.

My ambition level on this doesn't extend to auditing the sys.error calls and trying to determine what kind of user input might trigger them. As you saw, Iulian's original test case no longer produces an error.

So yeah, let's close this. Yes, there might still be other ways to make Scaladoc crash. If someone finds one (either accidentally, or by auditing the sites where sys.error appears), we can make a new ticket for that.

som-snytt commented 2 years ago

The OP means sys.error.

I, too, am unable to reproduce with any obsolete version.

The unhelpful code comment is that the second warning is unreachable: https://github.com/scala/scala/blob/v2.13.8/src/scaladoc/scala/tools/nsc/doc/base/CommentFactoryBase.scala#L718-L720

som-snytt commented 2 years ago

I see Seth is awake. I tried fileformat=mac (which TIL) but also doesn't crash FSR. The code is checking for \n which must be wrong. The compiler reader is more forgiving about line endings.

som-snytt commented 2 years ago

The blog is at https://scala-lang.org/blog/2018/10/04/scaladoc-tables.html

dragos commented 2 years ago

Sorry about the confusion, indeed I should have written sys.error. Glad to hear it's not longer a problem, regardless of who fixed it and how