Skip to content

Commit

Permalink
Stricter paths (#7147)
Browse files Browse the repository at this point in the history
* Simplify tests

* Change tests

* Make FilePath more strict
  • Loading branch information
BraisGabin committed Apr 26, 2024
1 parent 785ca96 commit 78fa964
Show file tree
Hide file tree
Showing 18 changed files with 177 additions and 138 deletions.
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package io.gitlab.arturbosch.detekt.api

import io.gitlab.arturbosch.detekt.test.fromRelative
import io.gitlab.arturbosch.detekt.test.location
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import java.io.File
import kotlin.io.path.Path
import kotlin.io.path.absolute

class CodeSmellSpec {

Expand All @@ -18,19 +19,20 @@ class CodeSmellSpec {
source = SourceLocation(1, 1),
endSource = SourceLocation(1, 1),
text = TextLocation(0, 0),
filePath = fromRelative(Path("/Users/tester/detekt/"), Path("TestFile.kt"))
filePath = fromRelative(
Path("/").absolute().resolve("Users/tester/detekt/"),
Path("TestFile.kt"),
),
),
ktElement = null
),
message = "TestMessage"
)
val basePath = "${File.separator}Users${File.separator}tester${File.separator}detekt"

assertThat(codeSmell.toString()).isEqualTo(
"CodeSmell(entity=Entity(name=TestEntity, signature=TestEntitySignature, " +
"location=Location(source=1:1, endSource=1:1, text=0:0, " +
"filePath=FilePath(absolutePath=$basePath${File.separator}TestFile.kt, " +
"basePath=$basePath, relativePath=TestFile.kt)), ktElement=null), message=TestMessage, " +
"filePath=${codeSmell.location.filePath}), ktElement=null), message=TestMessage, " +
"references=[])"
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.gitlab.arturbosch.detekt.api

import io.gitlab.arturbosch.detekt.test.createEntity
import io.gitlab.arturbosch.detekt.test.createLocation
import io.gitlab.arturbosch.detekt.test.location
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

Expand All @@ -10,16 +10,15 @@ class CorrectableCodeSmellSpec {
@Test
fun `toString contains all information`() {
val codeSmell: CorrectableCodeSmell = object : CorrectableCodeSmell(
entity = createEntity(location = createLocation("TestFile.kt")),
entity = createEntity(),
message = "TestMessage",
autoCorrectEnabled = true
) {}

assertThat(codeSmell.toString()).isEqualTo(
"CorrectableCodeSmell(autoCorrectEnabled=true, " +
"entity=Entity(name=TestEntity, signature=TestEntitySignature, " +
"location=Location(source=1:1, endSource=1:1, text=0:0, " +
"filePath=FilePath(absolutePath=TestFile.kt, basePath=null, relativePath=null)), " +
"location=Location(source=1:1, endSource=1:1, text=0:0, filePath=${codeSmell.location.filePath}), " +
"ktElement=null), message=TestMessage, references=[])"
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import io.gitlab.arturbosch.detekt.api.TextLocation
import org.jetbrains.kotlin.psi.KtElement
import java.nio.file.Path
import kotlin.io.path.Path
import kotlin.io.path.absolute

fun createIssue(
ruleName: String = "TestSmell",
Expand Down Expand Up @@ -41,6 +42,20 @@ fun createIssue(
autoCorrectEnabled = autoCorrectEnabled,
)

fun createIssue(
ruleInfo: Issue.RuleInfo,
location: Location,
message: String = "TestMessage",
severity: Severity = Severity.Error,
autoCorrectEnabled: Boolean = false,
): Issue = IssueImpl(
ruleInfo = ruleInfo,
entity = createEntity(location = location),
message = message,
severity = severity,
autoCorrectEnabled = autoCorrectEnabled,
)

fun createRuleInfo(
id: String = "TestSmell",
ruleSetId: String = "RuleSet$id",
Expand All @@ -53,23 +68,26 @@ fun createRuleInfo(

fun createIssueForRelativePath(
ruleInfo: Issue.RuleInfo,
basePath: String = "/Users/tester/detekt/",
basePath: String = "Users/tester/detekt/",
relativePath: String = "TestFile.kt"
): Issue = IssueImpl(
ruleInfo = ruleInfo,
entity = Entity(
name = "TestEntity",
signature = "TestEntitySignature",
location = Location(
source = SourceLocation(1, 1),
endSource = SourceLocation(1, 1),
text = TextLocation(0, 0),
filePath = fromRelative(Path(basePath), Path(relativePath))
): Issue {
require(!basePath.startsWith("/")) { "The path shouldn't start with '/'" }
return IssueImpl(
ruleInfo = ruleInfo,
entity = Entity(
name = "TestEntity",
signature = "TestEntitySignature",
location = Location(
source = SourceLocation(1, 1),
endSource = SourceLocation(1, 1),
text = TextLocation(0, 0),
filePath = fromRelative(Path("/").absolute().resolve(basePath), Path(relativePath))
),
ktElement = null
),
ktElement = null
),
message = "TestMessage"
)
message = "TestMessage"
)
}

fun createEntity(
signature: String = "TestEntitySignature",
Expand All @@ -88,13 +106,16 @@ fun createLocation(
position: Pair<Int, Int> = 1 to 1,
endPosition: Pair<Int, Int> = 1 to 1,
text: IntRange = 0..0,
) = Location(
source = SourceLocation(position.first, position.second),
endSource = SourceLocation(endPosition.first, endPosition.second),
text = TextLocation(text.first, text.last),
filePath = basePath?.let { fromRelative(Path(it), Path(path)) }
?: fromAbsolute(Path(path)),
)
): Location {
require(!path.startsWith("/")) { "The path shouldn't start with '/'" }
return Location(
source = SourceLocation(position.first, position.second),
endSource = SourceLocation(endPosition.first, endPosition.second),
text = TextLocation(text.first, text.last),
filePath = basePath?.let { fromRelative(Path("/").absolute().resolve(it), Path(path)) }
?: fromAbsolute(Path("/").absolute().resolve(path)),
)
}

private data class IssueImpl(
override val ruleInfo: Issue.RuleInfo,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package io.gitlab.arturbosch.detekt.core.reporting.console

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.core.reporting.AutoCorrectableIssueAssert
import io.gitlab.arturbosch.detekt.core.reporting.decolorized
import io.gitlab.arturbosch.detekt.test.TestDetektion
import io.gitlab.arturbosch.detekt.test.createEntity
import io.gitlab.arturbosch.detekt.test.createIssue
import io.gitlab.arturbosch.detekt.test.createLocation
import io.gitlab.arturbosch.detekt.test.createRuleInfo
Expand All @@ -18,21 +16,23 @@ class FileBasedIssuesReportSpec {

@Test
fun `has the reference content`() {
val location1 = createLocation("File1.kt")
val location2 = createLocation("File2.kt")
val detektion = TestDetektion(
createIssue(ruleSetId = "Ruleset1", fileName = "File1.kt"),
createIssue(ruleSetId = "Ruleset1", fileName = "File2.kt"),
createIssue(ruleSetId = "Ruleset2", fileName = "File1.kt"),
createIssue(createRuleInfo(ruleSetId = "Ruleset1"), location1),
createIssue(createRuleInfo(ruleSetId = "Ruleset1"), location2),
createIssue(createRuleInfo(ruleSetId = "Ruleset2"), location1),
)

val output = subject.render(detektion)?.decolorized()

assertThat(output).isEqualTo(
"""
File1.kt
TestSmell - [TestMessage] at File1.kt:1:1
TestSmell - [TestMessage] at File1.kt:1:1
File2.kt
TestSmell - [TestMessage] at File2.kt:1:1
${location1.filePath.absolutePath}
TestSmell - [TestMessage] at ${location1.compact()}
TestSmell - [TestMessage] at ${location1.compact()}
${location2.filePath.absolutePath}
TestSmell - [TestMessage] at ${location2.compact()}
""".trimIndent()
)
Expand All @@ -56,8 +56,3 @@ private fun createFileBasedIssuesReport(): FileBasedIssuesReport {
report.init(Config.empty)
return report
}

private fun createIssue(ruleSetId: String, fileName: String): Issue = createIssue(
ruleInfo = createRuleInfo(ruleSetId = ruleSetId),
entity = createEntity(location = createLocation(fileName)),
)
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.gitlab.arturbosch.detekt.core.reporting.AutoCorrectableIssueAssert
import io.gitlab.arturbosch.detekt.core.reporting.decolorized
import io.gitlab.arturbosch.detekt.test.TestDetektion
import io.gitlab.arturbosch.detekt.test.createIssue
import io.gitlab.arturbosch.detekt.test.createLocation
import io.gitlab.arturbosch.detekt.test.createRuleInfo
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
Expand All @@ -15,21 +16,22 @@ class IssuesReportSpec {

@Test
fun `has the reference content`() {
val location = createLocation()
val detektion = TestDetektion(
createIssue(createRuleInfo(ruleSetId = "Ruleset1")),
createIssue(createRuleInfo(ruleSetId = "Ruleset1")),
createIssue(createRuleInfo(ruleSetId = "Ruleset2")),
createIssue(createRuleInfo(ruleSetId = "Ruleset1"), location),
createIssue(createRuleInfo(ruleSetId = "Ruleset1"), location),
createIssue(createRuleInfo(ruleSetId = "Ruleset2"), location),
)

val output = subject.render(detektion)?.decolorized()

assertThat(output).isEqualTo(
"""
Ruleset1
TestSmell - [TestMessage] at TestFile.kt:1:1
TestSmell - [TestMessage] at TestFile.kt:1:1
TestSmell - [TestMessage] at ${location.compact()}
TestSmell - [TestMessage] at ${location.compact()}
Ruleset2
TestSmell - [TestMessage] at TestFile.kt:1:1
TestSmell - [TestMessage] at ${location.compact()}
""".trimIndent()
)
Expand All @@ -49,20 +51,23 @@ class IssuesReportSpec {

@Test
fun `truncates long message`() {
val expectedContent = """
Ruleset
LongRule - [This is just a long message that should be truncated after a given threshold is (...)] at TestFile.kt:1:1
MultilineRule - [A multiline message.] at TestFile.kt:1:1
""".trimIndent()
val longMessage = "This is just a long message that should be truncated after a given " +
"threshold is reached."
val multilineMessage = "A multiline\n\r\tmessage.\t "
val detektion = TestDetektion(
createIssue(createRuleInfo("LongRule", "Ruleset"), message = longMessage),
createIssue(createRuleInfo("MultilineRule", "Ruleset"), message = multilineMessage),
createIssue(
createRuleInfo("LongRule", "Ruleset"),
message = "This is just a long message that should be truncated after a given threshold is reached.",
),
createIssue(
createRuleInfo("MultilineRule", "Ruleset"),
message = "A multiline\n\r\tmessage.\t ",
),
)
assertThat(subject.render(detektion)?.decolorized()).isEqualTo(expectedContent)
val output = subject.render(detektion)?.decolorized()
assertThat(output)
.contains(
"LongRule - [This is just a long message that should be truncated after a given threshold is (...)]"
)
assertThat(output)
.contains("MultilineRule - [A multiline message.]")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.core.reporting.AutoCorrectableIssueAssert
import io.gitlab.arturbosch.detekt.test.TestDetektion
import io.gitlab.arturbosch.detekt.test.createIssue
import io.gitlab.arturbosch.detekt.test.createLocation
import io.gitlab.arturbosch.detekt.test.createRuleInfo
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

Expand All @@ -13,14 +15,15 @@ class LiteIssuesReportSpec {

@Test
fun `reports non-empty issues`() {
val location = createLocation()
val detektion = TestDetektion(
createIssue("SpacingAfterPackageDeclaration"),
createIssue("UnnecessarySafeCall")
createIssue(createRuleInfo("SpacingAfterPackageDeclaration"), location),
createIssue(createRuleInfo("UnnecessarySafeCall"), location),
)
assertThat(subject.render(detektion)).isEqualTo(
"""
TestFile.kt:1:1: TestMessage [SpacingAfterPackageDeclaration]
TestFile.kt:1:1: TestMessage [UnnecessarySafeCall]
${location.compact()}: TestMessage [SpacingAfterPackageDeclaration]
${location.compact()}: TestMessage [UnnecessarySafeCall]
""".trimIndent()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ internal fun buildFinding(element: KtElement?): Finding = CodeSmell(
private fun buildEmptyEntity(): Entity = Entity(
name = "",
signature = "",
location = createLocation(path = "/"),
location = createLocation(""),
ktElement = null,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ class FilePath(
) {
"Absolute path = $absolutePath much match base path = $basePath and relative path = $relativePath"
}
require(absolutePath.isAbsolute) { "absolutePath should be absolute" }
require(basePath?.isAbsolute != false) { "basePath should be absolute" }
require(relativePath?.isAbsolute != true) { "relativePath should not be absolute" }
}

override fun toString(): String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.com.intellij.psi.PsiFile
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import kotlin.io.path.Path
import kotlin.io.path.absolute

class KtFilesSpec {

Expand Down Expand Up @@ -43,4 +45,15 @@ class KtFilesSpec {
}

private fun makeFile(filename: String): PsiFile = FakePsiFile(name = filename)

@Test
fun `FilePath toString`() {
val basePath = Path("/").absolute().resolve("a/b")
val relativePath = Path("c/d")
val absolutePath = Path("/").absolute().resolve("a/b/c/d")
val filePath = FilePath(absolutePath, basePath, relativePath)

assertThat(filePath.toString())
.isEqualTo("FilePath(absolutePath=$absolutePath, basePath=$basePath, relativePath=$relativePath)")
}
}

0 comments on commit 78fa964

Please sign in to comment.