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

Range position incorrect for infix, braced application #8859

Open
scabug opened this issue Sep 22, 2014 · 10 comments
Open

Range position incorrect for infix, braced application #8859

scabug opened this issue Sep 22, 2014 · 10 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Sep 22, 2014

As discovered and investigated by [~ajozwik]: scala/scala#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
Copy link
Author

scabug commented Sep 22, 2014

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

@scabug
Copy link
Author

scabug commented Sep 22, 2014

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

@scabug
Copy link
Author

scabug commented Sep 24, 2014

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

@scabug
Copy link
Author

scabug commented Sep 24, 2014

@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
Copy link
Author

scabug commented Sep 24, 2014

@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
Copy link
Author

scabug commented Nov 10, 2016

@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

@scabug scabug added this to the Backlog milestone Apr 7, 2017
@retronym
Copy link
Member

retronym commented Jan 3, 2018

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
Copy link

soronpo commented Jan 3, 2018

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

@som-snytt
Copy link

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
Copy link

som-snytt commented Jul 27, 2020

There's a related (edit: or not) position ticket #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants