MaxLineLength highlights a strange part of raw strings with escape sequences (Regex)

[TWiStErRob]

Expected Behavior

The whole line is highlighted.

Observed Behavior

Steps to Reproduce

Based on the real example above, I minified a test-case:
MaxLineLengthSpec:

    @Test
    fun repro() {
        val rule = MaxLineLength(
            TestConfig(
                MAX_LINE_LENGTH to "30",
                EXCLUDE_RAW_STRINGS to "false",
            )
        )

        rule.visitKtFile(compileContentForTest(
            """
                // some other content
                val x = Regex(${"\"\"\""}
                    Text (.*?)\(in parens\) this is too long to be valid.
                    The regex/raw string continues down another line.
                ${"\"\"\""}.trimIndent())
                // that is the right length
            """.trimIndent()
        ))
        assertThat(rule.findings).hasSize(2)
    }

I also debugged it to see why it's wrong (breakpoint at line 65 in MaxLineLength / if (ktElement != null)):
(ktElement is the found "parent", the grey-highlighted line in the debugger)

Context and investigation

I was fixing Detekt issues on a project when I found this. It baffled me how it's only highlighting the half of the line. After debugging (see above) I'm still baffled, but I can see the escape sequences are separate and there's no common parent. I also found #5171 by @BraisGabin which hides this issue by default (in 1.22). At this point I'm inclined to say this is a non-issue (consider very few people will observe it), but I wanted to report it in case you think otherwise. At the moment the only possible fix I see is to ignore the found "parent" in raw strings and explicitly highlight the entire line.

Note: this could be an improvement even when there's a parent found, because it's possible it could just highlight val for example if that line is too long, which is also strange:

Your Environment

• Version of detekt used: 1.21.0
• Link to your project (if it's a public repository): https://github.com/TWiStErRob/net.twisterrob.gradle/blob/1b632fd6e2b13fa9e871a7f46bdb66591e9290e8/quality/src/main/kotlin/net/twisterrob/gradle/quality/report/html/model/LintMessageDetailsSplitter.kt#L56-L62

[3flex]

I just had a look at this, but the repro test case doesn't fail in 1.22.0. Can you please confirm if this still an issue in 1.22.0 and provide an updated test case if so? Thanks!

[TWiStErRob]

I can't visually confirm without Detekt IDEA plugin 1.22. Will have a look at command line later.

[TWiStErRob]

HTML report looks exactly the same in 1.21 and 1.22, same as in screenshot of IDE code editor with strange highlight.

Same @Test (in OP) still reproduces the issue, but it's not a TDD test, it's useful for debugging.
If you add assertThat(rule.findings).hasTextLocations(40 to 97, 98 to 151), it becomes TDD red. Sorry about the confusion.

[BraisGabin]

This issue is really odd. if you check, in all your code samples the end of the underlined error is just before the char \. And that's not pure coincidence. I tested. If I move the position of the first \ in your test the reported number change.

I found a solution. I'll create a PR.

[TWiStErRob]

Yeah, that comes from the fact how KtLiteralString is split up (see debug shot in OP).

[TWiStErRob]

@BraisGabin found another instance where there's no \:

[BraisGabin]

Could you copy it as text so I can copy&paste to add a unit test and check it?

[TWiStErRob]

Sorry for the delay, here's a self-contained repro:

  MaxLineLength:
    excludeCommentStatements: true

interface TaskContainer {
	fun register(name: String, block: Number.() -> Unit = {})
}
interface Project {
	val tasks: TaskContainer
}
fun repros(project: Project) {
	val part = "name".capitalize()
	// nothing highlighted (expected)
	project.tasks.register("shortName${part}WithSuffix")
	// full line highlighted (expected)
	project.tasks.register("veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongName${part}WithSuffix")
	// project.tasks highlighted (unexpected)
	project.tasks.register("veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongName${part}WithSuffix") {
		this.toByte()
	}
	// dot before register highlighted (unexpected)
	project.tasks
		.register("veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongName${part}WithSuffix") {
		this.toByte()
	}
}

[BraisGabin]

I tested it and all those cases with the current PR are fixed. Adding that snippet as a unit test is kind of difficult so I'm not adding it.