Skip to content

Commit

Permalink
fix Caret arguments ordering (#313)
Browse files Browse the repository at this point in the history
* fix Caret arguments ordering

`line` was filled with `offset`, `col` was filled with `line` and `offset` with `col`.

I'm guessing at one point Caret was `Caret(offset, line, column)`, but was changed to `Caret(line, column, offset)` without re-examing argument order.

This caused this behavior:

```scala
val p = (Parser.caret.with1 <* Parser.anyChar).rep

val pattern = """aaa"""

val parsed = p.parseAll(pattern).right.get
// parsed: cats.data.NonEmptyList[Caret] = NonEmptyList(Caret(0, 0, 0), List(Caret(1, 0, 1), Caret(2, 0, 2)))
```

Expected:
```scala
// parsed: cats.data.NonEmptyList[Caret] = NonEmptyList(Caret(0, 0, 0), List(Caret(0, 1, 1), Caret(0, 2, 2)))
```

* formatting

* Deconstruct Caret properly

* remove unneeded case

* add assert for offset
  • Loading branch information
hugo-vrijswijk committed Nov 20, 2021
1 parent 95ce42a commit 00362b4
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 7 deletions.
12 changes: 6 additions & 6 deletions core/shared/src/main/scala/cats/parse/LocationMap.scala
Expand Up @@ -69,7 +69,7 @@ class LocationMap(val input: String) {
*/
def toLineCol(offset: Int): Option[(Int, Int)] =
if (isValidOffset(offset)) {
val Caret(_, line, col) = toCaretUnsafeImpl(offset)
val Caret(line, col, _) = toCaretUnsafeImpl(offset)
Some((line, col))
} else None

Expand All @@ -81,9 +81,9 @@ class LocationMap(val input: String) {
// this is end of line
if (offset == 0) Caret.Start
else {
val Caret(_, line, col) = toCaretUnsafeImpl(offset - 1)
if (endsWithNewLine) Caret(offset, line + 1, 0)
else Caret(offset, line, col + 1)
val Caret(line, col, _) = toCaretUnsafeImpl(offset - 1)
if (endsWithNewLine) Caret(line = line + 1, col = 0, offset = offset)
else Caret(line = line, col = col + 1, offset = offset)
}
} else {
val idx = Arrays.binarySearch(firstPos, offset)
Expand All @@ -98,10 +98,10 @@ class LocationMap(val input: String) {
// so we are pointing into a line
val lineStart = firstPos(line)
val col = offset - lineStart
Caret(offset, line, col)
Caret(line = line, col = col, offset = offset)
} else {
// idx is exactly the right value because offset is beginning of a line
Caret(offset, idx, 0)
Caret(line = idx, col = 0, offset = offset)
}
}

Expand Down
3 changes: 2 additions & 1 deletion core/shared/src/test/scala/cats/parse/LocationMapTest.scala
Expand Up @@ -226,7 +226,8 @@ class LocationMapTest extends munit.ScalaCheckSuite {
val lc = lm.toLineCol(offset)

assertEquals(oc, Some(c))
assertEquals(lc, oc.map { case Caret(_, r, c) => (r, c) })
assertEquals(lc, oc.map { c => (c.line, c.col) })
assertEquals(c.offset, offset)
}

if (other < 0 || s.length < other) {
Expand Down

0 comments on commit 00362b4

Please sign in to comment.