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

BatchSourceFile throws an exception when there is an empty line at the end of a file #9885

Closed
scabug opened this Issue Aug 11, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Aug 11, 2016

    val file = new BatchSourceFile(
      "bug.scala",
      """package com.example
        |object Test {
        |}
        |
        |""".stripMargin
    )

    val offset = file.lineToOffset(4)
    file.offsetToLine(offset)

Fails with:

java.lang.ArrayIndexOutOfBoundsException: 6
at scala.reflect.internal.util.BatchSourceFile.findLine$1(SourceFile.scala:178)
at scala.reflect.internal.util.BatchSourceFile.offsetToLine(SourceFile.scala:181)
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 11, 2016

Imported From: https://issues.scala-lang.org/browse/SI-9885?orig=1
Reporter: Eugene Apollonsky (chessman)
Affected Versions: 2.11.8

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 11, 2016

@som-snytt said:
Line numbers start at zero, so lineToOffset(4) should throw.

scala> val offset = file.lineToOffset(4)
offset: Int = 37

scala> file.content.length
res2: Int = 37
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 12, 2016

Eugene Apollonsky (chessman) said:
There are 5 lines:

0: package com.example
1: object Test {
2: }
3:
4:
@ val a = """package com.example
      |object Test {
      |}
      |
      |""".stripMargin    
a: String = """
package com.example
object Test {
}


"""
@ a.split("\n", -1) 
res17: Array[String] = Array("package com.example", "object Test {", "}", "", "")
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 12, 2016

Eugene Apollonsky (chessman) said:
This issue affected me when I was using ENSIME:

java.lang.ArrayIndexOutOfBoundsException: 31
	at scala.reflect.internal.util.BatchSourceFile.findLine$1(SourceFile.scala:178)
	at scala.reflect.internal.util.BatchSourceFile.offsetToLine(SourceFile.scala:181)
	at scala.reflect.internal.util.InternalPositionImpl$class.line(Position.scala:166)
	at scala.reflect.internal.util.Position.line(Position.scala:12)
	at scala.reflect.internal.util.DefinedPosition.toString(Position.scala:84)
	at java.lang.String.valueOf(String.java:2994)
	at scala.collection.mutable.StringBuilder.append(StringBuilder.scala:200)
	at scala.tools.nsc.interactive.Global.typedTreeAt(Global.scala:803)
	at scala.tools.nsc.interactive.Global$$anonfun$getTypedTreeAt$1.apply(Global.scala:842)
	at scala.tools.nsc.interactive.Global$$anonfun$getTypedTreeAt$1.apply(Global.scala:842)
	at scala.tools.nsc.interactive.Global$$anonfun$respond$1.apply(Global.scala:701)
	at scala.tools.nsc.interactive.Global$$anonfun$respond$1.apply(Global.scala:701)
	at scala.tools.nsc.interactive.Global.respondGradually(Global.scala:708)
	at scala.tools.nsc.interactive.Global.respond(Global.scala:701)
	at scala.tools.nsc.interactive.Global.getTypedTreeAt(Global.scala:842)
	at scala.tools.nsc.interactive.CompilerControl$AskTypeAtItem.apply$mcV$sp(CompilerControl.scala:333)
	at scala.tools.nsc.interactive.CompilerControl$AskTypeAtItem.apply(CompilerControl.scala:333)
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 12, 2016

@som-snytt said (edited on Aug 13, 2016 12:00:09 AM UTC):
There are four lines, see below, but the more important use case is ensime. The stack trace doesn't show lineToOffset, only offsetToLine. I'll take a look at that, too.

$ scala
Welcome to Scala 2.11.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_60).
Type in expressions for evaluation. Or try :help.

scala> val text =   """package com.example
     |     |object Test {
     |     |}
     |     |
     |     |""".stripMargin
text: String =
"package com.example
object Test {
}

"

scala> text.lines.length
res0: Int = 4

scala> text.split("\n")
res1: Array[String] = Array(package com.example, object Test {, })

scala> text.split("\n", -1)
res2: Array[String] = Array(package com.example, object Test {, }, "", "")

scala> "a\nb\nc\n".split("\n", -1)
res3: Array[String] = Array(a, b, c, "")

Position.toString wants its line number, which is based on its point, so probably the error is due to a position past EOF. (here)

Position has 1-based line number (here), so that may be confusing. It's not obvious that you couldn't create a Position by converting a valid position's line number to an offset, without subtracting one first.

Position.offset(f, f.lineToOffset(p.line - 1))    // start of line
@scabug

This comment has been minimized.

Copy link
Author

scabug commented Aug 13, 2016

@scabug scabug closed this Nov 18, 2016

@scabug

This comment has been minimized.

Copy link
Author

scabug commented Jan 21, 2017

@som-snytt said:
The PR didn't address the two ways of counting lines, namely whether a terminal NL introduces an empty line.

Is this another of those hardest problems in computer science?

BatchSourceFile introduces a NL into an empty file (though possibly it should only do that, for internal reasons, for nonempty files not ending in whitespace).

scala> import scala.reflect.internal.util._
import scala.reflect.internal.util._

scala> val f = new BatchSourceFile("test", "")
f: scala.reflect.internal.util.BatchSourceFile = test

scala> f.content
res0: Array[Char] =
Array(
)

scala> f.content.length
res1: Int = 1

@scabug scabug added this to the 2.12.1 milestone Apr 7, 2017

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