scala / bug

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

Range position incorrect for infix, braced application #8859

Open scabug opened 10 years ago

scabug commented 10 years ago

As discovered and investigated by [~ajozwik]: https://github.com/scala/scala/pull/3991

// test/files/run/parser-binop-braces.scala
object Test extends App {
  import scala.reflect.runtime._
  import scala.reflect.runtime.universe._
  import scala.tools.reflect.ToolBox

  val mirror = universe.runtimeMirror(universe.getClass.getClassLoader)
  val toolbox = mirror.mkToolBox(options = "-Yrangepos")
  def showParsed(code: String) = {
    val parsed = toolbox.parse(code)
    val recovered = code.substring(parsed.pos.start, parsed.pos.end)
    println(s"\n$code\n${show(parsed, printPositions = true)}\n$recovered")
  }
  showParsed("x map f")
  showParsed("x map (f)")
  showParsed("x map ((f))")
  showParsed("x map {f}")
  showParsed("x map {{f}}")
}
scalac-hash v2.11.2 test/files/run/parser-binop-braces.scala && scala-hash v2.11.2 Test

x map f
[0:7][0:5]x.map([6:7]f)
x map f

x map (f)
[0:9][0:5]x.map([7:8]f)
x map (f)

x map ((f))
[0:11][0:5]x.map([8:9]f)
x map ((f))

x map {f}
[0:8][0:5]x.map([7:8]f)
x map {f

x map {{f}}
[0:9][0:5]x.map([8:9]f)
x map {{f
scabug commented 10 years ago

Imported From: https://issues.scala-lang.org/browse/SI-8859?orig=1 Reporter: @retronym Affected Versions: 2.11.2

scabug commented 10 years ago

@retronym said: Here's how I believe we can fix this: https://github.com/retronym/scala/compare/ticket/8859?expand=1

scabug commented 10 years ago

@gkossakowski said: Jason, would be feasible to have JUnit tests for this issue?

scabug commented 10 years ago

@retronym said (edited on Sep 24, 2014 12:59:03 PM UTC): The test in my prototype doesn't depend on any particular features of partest, so that would be possible.

The bigger question in my mind is whether the introduction of the new, transient AST node Braces is the simplest solution to the bug.

scabug commented 10 years ago

@gkossakowski said: I haven't looked at it in detail. I'll try to look into that over the weekend (before that I'm busy preparing for reactive streams workshop I'm giving).

scabug commented 7 years ago

@dwijnand said: If this were fixed it would help sbt's performance, as currently *.sbt files are parsed twice:

https://github.com/sbt/sbt/blob/v0.13.13/main/src/main/scala/sbt/internals/parser/SbtParser.scala#L132

retronym commented 6 years ago

Possibly related:

object Foo {
  def apply(t : Any)(block: => Unit) : Unit = {}
  def foo(t : Any)(block: => Unit) : Unit = {}
}

object Test {
  Foo(1){} //OK
  Foo foo(1) {} //Error: Int(1) does not take parameters
  Foo.foo(1) {} //OK
}

Where the applications are parsed as:

    Foo(1)(());
    Foo.foo(1(()));
    Foo.foo(1)(())
soronpo commented 6 years ago

I created a scastie link with the error and parser printing.

som-snytt commented 6 years ago

I think the error is correct, as it's InfixExpr id SimpleExpr where the simple is Simple Args. The example also looks good this way:

Foo foo ("abc")
{
  1
}
// missing argument list for method foo in object Foo

Are they still making puzzlers?

som-snytt commented 4 years ago

There's a related (edit: or not) position ticket https://github.com/scala/bug/issues/12074

I was just grepping for a good example of a confusing "does not take parameters", because there is a dotty ticket about how helpful an error message should be. -Yreporter:french-waiter.