Skip to content
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

SONARKT-542 Fix CPD with string templates #560

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import org.jetbrains.kotlin.psi.KtFileAnnotationList
import org.jetbrains.kotlin.psi.KtImportDirective
import org.jetbrains.kotlin.psi.KtImportList
import org.jetbrains.kotlin.psi.KtPackageDirective
import org.jetbrains.kotlin.psi.KtStringTemplateEntry
import org.jetbrains.kotlin.psi.KtStringTemplateExpression
import org.jetbrains.kotlin.psi.psiUtil.allChildren
import org.slf4j.LoggerFactory
import org.sonar.api.batch.fs.InputFile
Expand All @@ -44,7 +44,7 @@ class CopyPasteDetector : KotlinFileVisitor() {
val cpdTokens = sensorContext.newCpdTokens().onFile(kotlinFileContext.inputFileContext.inputFile)

val tokens = collectCpdRelevantNodes(kotlinFileContext.ktFile).map { node ->
val text = if (node is KtStringTemplateEntry) "LITERAL" else node.text
val text = if (node is KtStringTemplateExpression) "LITERAL" else node.text
val cpdToken = CPDToken(kotlinFileContext.textRange(node), text)
cpdTokens.addToken(cpdToken.range, cpdToken.text)
cpdToken
Expand All @@ -60,7 +60,7 @@ class CopyPasteDetector : KotlinFileVisitor() {
acc: MutableList<PsiElement> = mutableListOf()
): List<PsiElement> {
if (!isExcludedFromCpd(node)) {
if ((node is LeafPsiElement && node !is PsiWhiteSpace) || node is KtStringTemplateEntry) {
if ((node is LeafPsiElement && node !is PsiWhiteSpace) || node is KtStringTemplateExpression) {
acc.add(node)
} else {
node.allChildren.forEach { collectCpdRelevantNodes(it, acc) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ internal class KotlinSensorTest : AbstractSensorTest() {
assertThat(context.measure(inputFile.key(), CoreMetrics.FUNCTIONS).value()).isEqualTo(1)
assertThat(context.measure(inputFile.key(), CoreMetrics.CLASSES).value()).isEqualTo(1)
assertThat(context.cpdTokens(inputFile.key())!![1].value)
.isEqualTo("print(1==1);print(\"LITERAL\");}")
.isEqualTo("print(1==1);print(LITERAL);}")
assertThat(context.measure(inputFile.key(), CoreMetrics.COMPLEXITY).value()).isEqualTo(1)
assertThat(context.measure(inputFile.key(), CoreMetrics.STATEMENTS).value()).isEqualTo(2)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,14 @@
package org.sonarsource.kotlin.plugin.cpd

import com.intellij.openapi.util.Disposer
import net.bytebuddy.utility.JavaConstant.Dynamic
import org.assertj.core.api.Assertions
import org.assertj.core.api.ObjectAssert
import org.jetbrains.kotlin.config.LanguageVersion
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.DynamicTest
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestFactory
import org.junit.jupiter.api.extension.RegisterExtension
import org.junit.jupiter.api.io.TempDir
import org.slf4j.event.Level
Expand All @@ -33,6 +36,7 @@ import org.sonarsource.kotlin.plugin.DummyReadCache
import org.sonarsource.kotlin.plugin.DummyWriteCache
import org.sonarsource.kotlin.api.checks.InputFileContextImpl
import org.sonarsource.kotlin.api.frontend.Environment
import org.sonarsource.kotlin.testapi.DummyInputFile
import org.sonarsource.kotlin.testapi.kotlinTreeOf
import java.nio.file.Path
import kotlin.io.path.createFile
Expand Down Expand Up @@ -100,25 +104,24 @@ class CopyPasteDetectorTest {
.hasEndUnit(8)

assertThat(cpdTokenLines[2])
.hasValue("""println("LITERAL")""")
.hasValue("""println(LITERAL)""")
.hasStartLine(10)
.hasStartUnit(9)
.hasEndUnit(14)
.hasEndUnit(12)

assertThat(cpdTokenLines[3])
.hasValue("}")
.hasStartLine(11)
.hasStartUnit(15)
.hasEndUnit(15)
.hasStartUnit(13)
.hasEndUnit(13)

assertThat(cpdTokenLines[4])
.hasValue("}")
.hasStartLine(12)
.hasStartUnit(16)
.hasEndUnit(16)
.hasStartUnit(14)
.hasEndUnit(14)
}


@Test
fun `cpd tokens are saved for the next analysis when the cache is enabled`() {
logTester.setLevel(Level.TRACE)
Expand Down Expand Up @@ -148,7 +151,7 @@ class CopyPasteDetectorTest {

Assertions.assertThat(logs)
.hasSize(1)
.containsExactly("Caching 16 CPD tokens for next analysis of input file moduleKey:dummy.kt.")
.containsExactly("Caching 14 CPD tokens for next analysis of input file moduleKey:dummy.kt.")
}

@Test
Expand Down Expand Up @@ -181,6 +184,53 @@ class CopyPasteDetectorTest {
.hasSize(1)
.containsExactly("No CPD tokens cached for next analysis of input file moduleKey:dummy.kt.")
}

private val d = "$"
private val tq = "\"\"\""

@TestFactory
fun `cpd tokens`() = listOf(
Triple("int literal", """ val x = 42 """, "valx=42"),
Triple("long literal", """ val x = 42L """, "valx=42L"),
Triple("float literal", """ val x = 42.0f """, "valx=42.0f"),
Triple("double literal", """ val x = 42.0 """, "valx=42.0"),
Triple("char literal", """ val x = 'a' """, "valx='a'"),
Triple("null literal", """ val x = null """, "valx=null"),
Triple("double-quote string literal", """ val x = "a" """, "valx=LITERAL"),
Triple("double-quote string literal concatenation", """ val x = "a" + "b" """, "valx=LITERAL+LITERAL"),
Triple("double-quote string template", """ val x = "a $d{1}" """, "valx=LITERAL"),
Triple("triple-quote string literal", """ val x = ${tq}a${tq} """, "valx=LITERAL"),
Triple("triple-quote string literal concatenation", """ val x = ${tq}a${tq} + ${tq}b${tq} """, "valx=LITERAL+LITERAL"),
Triple("triple-quote string template", """ val x = ${tq}a $d{1}${tq} """, "valx=LITERAL"),
Triple("mixed-quote string literal concatenation", """ val x = "a" + ${tq}b${tq} """, "valx=LITERAL+LITERAL"),
Triple(
"triple-quote string template with interpolated vars",
"""
val noInterpolations = "a literal"
val doubleQuoteTwoInterpolations = "$d{x} $d{x}"
val tripleQuoteTwoInterpolations = $tq$d{noInterpolations} $d{doubleQuoteTwoInterpolations}${tq}
var nestedInterpolations = $tq$d{ "$d{1 + 1}" }${tq}
""".trimIndent(),
"""
valnoInterpolations=LITERAL
valdoubleQuoteTwoInterpolations=LITERAL
valtripleQuoteTwoInterpolations=LITERAL
varnestedInterpolations=LITERAL
""".trimIndent()
)
).map { (title, input, expected) ->
DynamicTest.dynamicTest("with $title") {
val sensorContext: SensorContextTester = SensorContextTester.create(tmpFolder!!.root)
val inputFile = TestInputFileBuilder("moduleKey", "test.kt").setContents(input).build()
val root = kotlinTreeOf(input, Environment(disposable, emptyList(), LanguageVersion.LATEST_STABLE), inputFile)
val ctx = InputFileContextImpl(sensorContext, inputFile, false)
CopyPasteDetector().scan(ctx, root)

val cpdTokenLines = sensorContext.cpdTokens(inputFile.key())!!
val tokensStringified = cpdTokenLines.joinToString(separator = "\n", transform = { it.value })
Assertions.assertThat(tokensStringified).isEqualTo(expected)
}
}
}


Expand Down