Skip to content

Commit a571fbc

Browse files
authored
Fixed prettyIndex for one-line files, use binarySearch (#554)
Pretty index was failing with following exception for files with one line and no `\n` at the end. The `lines` array was empty: ``` java.lang.ArrayIndexOutOfBoundsException: Index -1 out of bounds for length 0 ``` There are actually some generated files in our code based that follow this pattern. Also, for expressions in the very first line it was creating values with negative columns, like: ``` x.libsonnet:2:-63 ``` Fixed the issue and added tests, also for edge case with completely empty file. While changing it also optimized searching for line - previously `indexWhere` was used which is linear search. Replaced with binary search. It does not matter to much usually, but actually was causing slowdown of profiler code that gets locations of all evaluated expressions - this will be added in a subsequent PR.
1 parent abbcf33 commit a571fbc

File tree

2 files changed

+94
-9
lines changed

2 files changed

+94
-9
lines changed

sjsonnet/src/sjsonnet/Importer.scala

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ final case class FileParserInput(file: File) extends ParserInput {
5757
override def checkTraceable(): Unit = {}
5858

5959
private lazy val lineNumberLookup: Array[Int] = {
60-
val lines = mutable.ArrayBuffer[Int]()
60+
val lines = mutable.ArrayBuffer[Int](0)
6161
val bufferedStream = new BufferedInputStream(new FileInputStream(file))
6262
var byteRead: Int = 0
6363
var currentPosition = 0
@@ -74,14 +74,8 @@ final case class FileParserInput(file: File) extends ParserInput {
7474
lines.toArray
7575
}
7676

77-
def prettyIndex(index: Int): String = {
78-
val line = lineNumberLookup.indexWhere(_ > index) match {
79-
case -1 => lineNumberLookup.length - 1
80-
case n => math.max(0, n - 1)
81-
}
82-
val col = index - lineNumberLookup(line)
83-
s"${line + 1}:${col + 1}"
84-
}
77+
def prettyIndex(index: Int): String =
78+
Util.prettyIndex(lineNumberLookup, index)
8579
}
8680

8781
class BufferedRandomAccessFile(fileName: String, bufferSize: Int) {
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package sjsonnet
2+
3+
import utest._
4+
5+
import java.io.{File, FileOutputStream}
6+
import java.nio.file.Files
7+
import scala.collection.mutable
8+
9+
object FileParserInputTests extends TestSuite {
10+
private val tempFiles = mutable.ArrayBuffer[File]()
11+
12+
def createTestFile(content: String): FileParserInput = {
13+
val tempFile = Files.createTempFile("fileparser-", ".txt").toFile
14+
tempFiles += tempFile
15+
val fos = new FileOutputStream(tempFile)
16+
try {
17+
fos.write(content.getBytes("UTF-8"))
18+
} finally {
19+
fos.close()
20+
}
21+
FileParserInput(tempFile)
22+
}
23+
24+
val tests: Tests = Tests {
25+
test("prettyIndex - empty file") {
26+
val input = createTestFile("")
27+
val result = input.prettyIndex(0)
28+
assert(result == "1:1")
29+
}
30+
31+
test("prettyIndex - single line without newline") {
32+
val input = createTestFile("hello world")
33+
34+
assert(input.prettyIndex(0) == "1:1")
35+
assert(input.prettyIndex(1) == "1:2")
36+
assert(input.prettyIndex(6) == "1:7")
37+
assert(input.prettyIndex(10) == "1:11")
38+
}
39+
40+
test("prettyIndex - single line with newline") {
41+
val input = createTestFile("hello world\n")
42+
43+
assert(input.prettyIndex(0) == "1:1")
44+
assert(input.prettyIndex(5) == "1:6")
45+
assert(input.prettyIndex(11) == "1:12")
46+
}
47+
48+
test("prettyIndex - multiple lines with different lengths") {
49+
val input = createTestFile("a\nbb\nccc\n")
50+
51+
assert(input.prettyIndex(0) == "1:1")
52+
assert(input.prettyIndex(1) == "1:2")
53+
54+
assert(input.prettyIndex(2) == "2:1")
55+
assert(input.prettyIndex(3) == "2:2")
56+
assert(input.prettyIndex(4) == "2:3")
57+
58+
assert(input.prettyIndex(5) == "3:1")
59+
assert(input.prettyIndex(6) == "3:2")
60+
assert(input.prettyIndex(7) == "3:3")
61+
assert(input.prettyIndex(8) == "3:4")
62+
}
63+
64+
test("prettyIndex - no trailing newline") {
65+
val input = createTestFile("line 1\nline 2\nline 3")
66+
67+
assert(input.prettyIndex(0) == "1:1")
68+
assert(input.prettyIndex(7) == "2:1")
69+
assert(input.prettyIndex(14) == "3:1")
70+
assert(input.prettyIndex(19) == "3:6")
71+
}
72+
73+
test("prettyIndex - large file with many lines") {
74+
val lines = (1 to 1000).map(i => s"Line $i").mkString("\n")
75+
val input = createTestFile(lines)
76+
77+
val line1Bytes = 9 * 7
78+
val line10Bytes = 90 * 8
79+
val line100Bytes = 400 * 9
80+
val offset500 = line1Bytes + line10Bytes + line100Bytes
81+
82+
val result = input.prettyIndex(offset500)
83+
assert(result == "500:1")
84+
}
85+
}
86+
87+
override def utestAfterAll(): Unit = {
88+
tempFiles.foreach(_.delete())
89+
tempFiles.clear()
90+
}
91+
}

0 commit comments

Comments
 (0)