From c3ec6d5ff6bcccbf07b98aa8e28a609a7aab1fe5 Mon Sep 17 00:00:00 2001 From: Marc GRIS <62659262+m-gris@users.noreply.github.com> Date: Thu, 10 Jul 2025 13:03:57 +0200 Subject: [PATCH] fix: StringIndexOutOfBoundsException in presentation compiler's hasColon method (#23498) This PR fixes a `StringIndexOutOfBoundsException` that could occur in the presentation compiler when checking for colons in override completions. ## Problem The `dotty.tools.pc.completions.OverrideCompletions.hasColon` method was accessing characters at specific indices without first verifying that the indices were within the text bounds. This could cause crashes when the span end positions were at or beyond the text length. ## Solution Added bounds checking before accessing characters: - For `TypeDef` cases: verify `td.rhs.span.end < text.length` before `text.charAt(td.rhs.span.end)` - For `Template` parent cases: filter to ensure `text.length > idx` before `text.charAt(idx)` ## Testing The existing test suite passes, and a new test case `no-new-line` was added to cover the edge case. [resolves scalameta/metals#7575](https://github.com/scalameta/metals/issues/7575) [Cherry-picked 01447dfcb45ffdeaf1f7790a247ddfe43bb1b385] --- .../pc/completions/OverrideCompletions.scala | 4 +- .../AutoImplementAbstractMembersSuite.scala | 72 ++++++++++++------- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/OverrideCompletions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/OverrideCompletions.scala index 807f959a2406..f01a1e9b8cd8 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/OverrideCompletions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/OverrideCompletions.scala @@ -511,10 +511,10 @@ object OverrideCompletions: Context ): Option[Int] = defn match - case td: TypeDef if text.charAt(td.rhs.span.end) == ':' => + case td: TypeDef if (td.rhs.span.end < text.length) && text.charAt(td.rhs.span.end) == ':' => Some(td.rhs.span.end) case TypeDef(_, temp : Template) => - temp.parentsOrDerived.lastOption.map(_.span.end).filter(text.charAt(_) == ':') + temp.parentsOrDerived.lastOption.map(_.span.end).filter(idx => text.length > idx && text.charAt(idx) == ':') case _ => None private def fallbackFromParent(parent: Tree, name: String)(using Context) = diff --git a/presentation-compiler/test/dotty/tools/pc/tests/edit/AutoImplementAbstractMembersSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/edit/AutoImplementAbstractMembersSuite.scala index ffe4e293ba30..38e78e4e545b 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/edit/AutoImplementAbstractMembersSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/edit/AutoImplementAbstractMembersSuite.scala @@ -75,39 +75,59 @@ class AutoImplementAbstractMembersSuite extends BaseCodeActionSuite: |""".stripMargin ) - @Test def `empty-lines-between-members` = + @Test def `no-new-line` = checkEdit( """|package a - | - |object A { - | trait Base { - | def foo(x: Int): Int - | def bar(x: String): String - | } - | class <> extends Base { - | - | def bar(x: String): String = ??? - | - | } - |} - |""".stripMargin, + | + |trait X: + | def foo: Unit + | + |class <> extends X""".stripMargin, """|package a | - |object A { - | trait Base { - | def foo(x: Int): Int - | def bar(x: String): String - | } - | class Concrete extends Base { - | + |trait X: + | def foo: Unit | - | override def foo(x: Int): Int = ??? + |class Y extends X { | - | def bar(x: String): String = ??? + | override def foo: Unit = ??? | - | } - |} - |""".stripMargin + |}""".stripMargin, + ) + + @Test def `empty-lines-between-members` = + checkEdit( + """|package a + | + |object A { + | trait Base { + | def foo(x: Int): Int + | def bar(x: String): String + | } + | class <> extends Base { + | + | def bar(x: String): String = ??? + | + | } + |} + |""".stripMargin, + """|package a + | + |object A { + | trait Base { + | def foo(x: Int): Int + | def bar(x: String): String + | } + | class Concrete extends Base { + | + | + | override def foo(x: Int): Int = ??? + | + | def bar(x: String): String = ??? + | + | } + |} + |""".stripMargin ) @Test def `objectdef` =