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

Toolbox.parse chokes on package declaration #6657

Closed
scabug opened this issue Nov 12, 2012 · 13 comments
Closed

Toolbox.parse chokes on package declaration #6657

scabug opened this issue Nov 12, 2012 · 13 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Nov 12, 2012

This REPL session show the problem:

import scala.reflect.runtime.{currentMirror => cm}
import scala.tools.reflect.ToolBox
val toolbox = cm.mkToolBox()
toolbox.parse("package Foo; class Bar")

scala.tools.reflect.ToolBoxError: reflective compilation has failed: 

illegal start of definition
	at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$ToolBoxGlobal.throwIfErrors(ToolBoxFactory.scala:318)
	at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$ToolBoxGlobal.parse(ToolBoxFactory.scala:289)
	at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl.parse(ToolBoxFactory.scala:405)

Quick glance over the code:
https://github.com/scala/scala/blob/2.10.x/src/compiler/scala/tools/reflect/ToolBoxFactory.scala#L283

makes it clear why it fails. This should be fixed either by changing wrapper to being a package instead of object or documenting it clearly. I think the former option is a winner.

Eugene, can you comment?

@scabug
Copy link
Author

scabug commented Nov 12, 2012

Imported From: https://issues.scala-lang.org/browse/SI-6657?orig=1
Reporter: @gkossakowski
Affected Versions: 2.10.0-RC2

@scabug
Copy link
Author

scabug commented Nov 12, 2012

@xeno-by said:
I implemented this to be consistent with tb.typeCheck and tb.eval, which also choke on packages. I'll ask Martin why it was done like this in the first place.

@scabug
Copy link
Author

scabug commented Nov 12, 2012

@xeno-by said:
Is this a blocker for any of your scenarios btw?

@scabug
Copy link
Author

scabug commented Nov 12, 2012

@gkossakowski said:
Thanks for response. Actually, I think you need several methods. parse should be a method that does not do any wrapping and then parseDe for parsing src in definition position (as parse does at the moment) and then parseExpr to parse expressions.

This is not a blocker for me. I just noticed this problem while playing with reflection.

@scabug
Copy link
Author

scabug commented Aug 13, 2013

John Eckhart (jeckhart) said:
I encountered this when trying to update the Sonar Scala plugin to use the reflective parser. Is the only workaround to use the tools.nsc compiler api?

@scabug
Copy link
Author

scabug commented Aug 14, 2013

@retronym said:
@Denys / @eugene: Should we add {{parseTopLevel}}? I sort of thought I'd seen this already but I can't find it now.

@scabug
Copy link
Author

scabug commented Aug 14, 2013

@densh said:
Adding another entry point is quick and simple way to fix the problem but I still have the hope that we can get away with just one. We can probably customize rules for parseStats to take packages into account but I've not really looked at it yet. There is a symmetrical quasiquotes issue #6841.

@scabug
Copy link
Author

scabug commented Oct 19, 2013

@densh said:
Fixed in scala/scala#3007

@scabug
Copy link
Author

scabug commented Jun 27, 2014

Johann Egger (lapislazuli) said:
this bug affects me using scala version 2.11.1
testing the following in the REPL

import scala.reflect.runtime.universe._
import scala.tools.reflect.ToolBox

val input = """package foo.bar
class Test {
}"""
val toolbox = runtimeMirror(getClass.getClassLoader).mkToolBox()
toolbox.parse(input)

results in

scala.tools.reflect.ToolBoxError: reflective compilation has failed:

'{' expected but ';' found.
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$ToolBoxGlobal.throwIfErrors(ToolBoxFactory.scala:315)
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$ToolBoxGlobal.parse(ToolBoxFactory.scala:290)
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$$anonfun$parse$1.apply(ToolBoxFactory.scala:416)
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$$anonfun$parse$1.apply(ToolBoxFactory.scala:413)
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$withCompilerApi$.liftedTree2$1(ToolBoxFactory.scala:354)
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl$withCompilerApi$.apply(ToolBoxFactory.scala:354)
  at scala.tools.reflect.ToolBoxFactory$ToolBoxImpl.parse(ToolBoxFactory.scala:413)
  ... 46 elided

my current workaround is surrounding package declarations with curly-brackets.

val input = """
package foo.bar{
  class Test {
  }
}"""

this results in a valid tree. However calling
{code}toolbox.typecheck{code}
on the resulting tree fails throwing:

java.lang.AssertionError: assertion failed: value <local <expression-owner>>
  at scala.reflect.internal.Symbols$Symbol.newPackage(Symbols.scala:309)
  at scala.tools.nsc.typechecker.Namers$Namer.createPackageSymbol(Namers.scala:380)
  at scala.tools.nsc.typechecker.Namers$Namer.createPackageSymbol(Namers.scala:373)
...

@scabug
Copy link
Author

scabug commented Jun 27, 2014

@retronym said:
You need to use the explicit braces to demarcate the package, as per:

cat sandbox/test.scala
import scala.reflect.runtime.universe._
import scala.tools.reflect.ToolBox

object Test extends App {
  val input = """package foo.bar {
  class Test {
  }}"""
  val toolbox = runtimeMirror(getClass.getClassLoader).mkToolBox()
  toolbox.parse(input)
}

The syntax you use is only applicable to entire compilation units, and not to fragments parsed by toolboxes.

@densh If you believe it is feasible to add support top level package clauses, please open a new ticket with Johann's test case.

@scabug scabug closed this as completed Jun 27, 2014
@scabug
Copy link
Author

scabug commented Jun 30, 2014

@densh said:
One can theoretically make header packages work in toolbox.parse and quasiquotes but I wonder if it's really worth it. Currently the syntax is already tweaked a bit as it's a sequence of either template or top-Level statements which doesn't happen naturally in Scala grammar. Adding header packages will only make it worse making it easy to write code that doesn't make any sense.

@scabug
Copy link
Author

scabug commented May 21, 2016

Julian Peeters (julianpeeters) said (edited on May 21, 2016 7:27:51 PM UTC):
Looks like there was a regression when moving from 2.11.7 to 2.11.8. As before, using explicit braces works for me, unitil there is more than one package in a snippet. Oddly, if the packages depend on each other, everything works fine, no braces required:

"""package example
  |     
  |case class Person(vehicle: test.major.Vehicle)
  |     
  |package test.major
  |    
  |case class Vehicle(name: String)""".stripMargin

Let me know if you'd like me to file a new issue.

@scabug
Copy link
Author

scabug commented May 21, 2016

Julian Peeters (julianpeeters) said:
Bah, false alarm. Sorry for the noise.

@scabug scabug added the macros label Apr 7, 2017
@scabug scabug added this to the 2.11.0-M7 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants