Skip to content

Commit

Permalink
Remove FilePath (#7235)
Browse files Browse the repository at this point in the history
* Replace FilePath with absolute path in Location

* Update tests

* Remove FilePath
  • Loading branch information
3flex committed May 5, 2024
1 parent 6c1de58 commit 65f3164
Show file tree
Hide file tree
Showing 30 changed files with 84 additions and 158 deletions.
4 changes: 2 additions & 2 deletions detekt-api/api/detekt-api.api
Expand Up @@ -190,10 +190,10 @@ public abstract interface class io/gitlab/arturbosch/detekt/api/Issue$RuleInfo {

public final class io/gitlab/arturbosch/detekt/api/Location : io/gitlab/arturbosch/detekt/api/Compactable {
public static final field Companion Lio/gitlab/arturbosch/detekt/api/Location$Companion;
public fun <init> (Lio/gitlab/arturbosch/detekt/api/SourceLocation;Lio/gitlab/arturbosch/detekt/api/SourceLocation;Lio/gitlab/arturbosch/detekt/api/TextLocation;Lio/github/detekt/psi/FilePath;)V
public fun <init> (Lio/gitlab/arturbosch/detekt/api/SourceLocation;Lio/gitlab/arturbosch/detekt/api/SourceLocation;Lio/gitlab/arturbosch/detekt/api/TextLocation;Ljava/nio/file/Path;)V
public fun compact ()Ljava/lang/String;
public final fun getEndSource ()Lio/gitlab/arturbosch/detekt/api/SourceLocation;
public final fun getFilePath ()Lio/github/detekt/psi/FilePath;
public final fun getPath ()Ljava/nio/file/Path;
public final fun getSource ()Lio/gitlab/arturbosch/detekt/api/SourceLocation;
public final fun getText ()Lio/gitlab/arturbosch/detekt/api/TextLocation;
public fun toString ()Ljava/lang/String;
Expand Down
@@ -1,15 +1,15 @@
package io.gitlab.arturbosch.detekt.api

import dev.drewhamilton.poko.Poko
import io.github.detekt.psi.FilePath
import io.github.detekt.psi.absolutePath
import io.github.detekt.psi.getLineAndColumnInPsiFile
import io.github.detekt.psi.toFilePath
import org.jetbrains.kotlin.com.intellij.openapi.util.TextRange
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.diagnostics.PsiDiagnosticUtils
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.psiUtil.endOffset
import org.jetbrains.kotlin.psi.psiUtil.startOffset
import java.nio.file.Path

/**
* Specifies a position within a source code fragment.
Expand All @@ -18,13 +18,13 @@ class Location(
val source: SourceLocation,
val endSource: SourceLocation,
val text: TextLocation,
val filePath: FilePath,
val path: Path,
) : Compactable {

override fun compact(): String = "${filePath.absolutePath}:$source"
override fun compact(): String = "$path:$source"

override fun toString(): String =
"Location(source=$source, endSource=$endSource, text=$text, filePath=$filePath)"
"Location(source=$source, endSource=$endSource, text=$text, path=$path)"

companion object {
/**
Expand All @@ -37,7 +37,7 @@ class Location(
val end = endLineAndColumn(element, offset)
val endSourceLocation = SourceLocation(end.line, end.column)
val textLocation = TextLocation(element.startOffset + offset, element.endOffset + offset)
return Location(sourceLocation, endSourceLocation, textLocation, element.containingFile.toFilePath())
return Location(sourceLocation, endSourceLocation, textLocation, element.containingFile.absolutePath())
}

/**
Expand Down
@@ -1,6 +1,5 @@
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
Expand All @@ -19,10 +18,7 @@ class CodeSmellSpec {
source = SourceLocation(1, 1),
endSource = SourceLocation(1, 1),
text = TextLocation(0, 0),
filePath = fromRelative(
Path("/").absolute().resolve("Users/tester/detekt/"),
Path("TestFile.kt"),
),
path = Path("/").absolute().resolve("Users/tester/detekt/TestFile.kt"),
),
ktElement = null
),
Expand All @@ -32,7 +28,7 @@ class CodeSmellSpec {
assertThat(codeSmell.toString()).isEqualTo(
"CodeSmell(entity=Entity(name=TestEntity, signature=TestEntitySignature, " +
"location=Location(source=1:1, endSource=1:1, text=0:0, " +
"filePath=${codeSmell.location.filePath}), ktElement=null), message=TestMessage, " +
"path=${codeSmell.location.path}), ktElement=null), message=TestMessage, " +
"references=[])"
)
}
Expand Down
Expand Up @@ -18,8 +18,9 @@ class CorrectableCodeSmellSpec {
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=${codeSmell.location.filePath}), " +
"ktElement=null), message=TestMessage, references=[])"
"location=Location(source=1:1, endSource=1:1, text=0:0, " +
"path=${codeSmell.location.path}), ktElement=null), " +
"message=TestMessage, references=[])"
)
}
}
Expand Up @@ -10,14 +10,10 @@ import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import kotlin.io.path.Path
import kotlin.io.path.absolute
import kotlin.io.path.relativeToOrSelf
import kotlin.io.path.toPath

class EntitySpec {

private val path = Path("src/test/resources/EntitySpecFixture.kt").absolute()
private val basePath = EntitySpec::class.java.getResource("/")!!.toURI().toPath()
private val relativePath = path.relativeToOrSelf(basePath)
private val code = compileForTest(path)

@Nested
Expand Down Expand Up @@ -57,7 +53,7 @@ class EntitySpec {
.isEqualTo(
"Entity(name=memberFun, signature=EntitySpecFixture.kt\$C\$private fun memberFun(): Int, " +
"location=Location(source=5:17, endSource=5:26, text=49:58, " +
"filePath=FilePath(absolutePath=$path, basePath=$basePath, relativePath=$relativePath)), " +
"path=$path), " +
"ktElement=FUN)"
)
}
Expand All @@ -84,7 +80,7 @@ class EntitySpec {
.isEqualTo(
"Entity(name=C, signature=EntitySpecFixture.kt\$C : Any, " +
"location=Location(source=3:7, endSource=3:8, text=20:21, " +
"filePath=FilePath(absolutePath=$path, basePath=$basePath, relativePath=$relativePath)), " +
"path=$path), " +
"ktElement=CLASS)"
)
}
Expand Down Expand Up @@ -115,7 +111,7 @@ class EntitySpec {
.isEqualTo(
"Entity(name=EntitySpecFixture.kt, signature=EntitySpecFixture.kt\$test.EntitySpecFixture.kt, " +
"location=Location(source=1:1, endSource=9:1, text=0:109, " +
"filePath=FilePath(absolutePath=$path, basePath=$basePath, relativePath=$relativePath)), " +
"path=$path), " +
"ktElement=KtFile: EntitySpecFixture.kt)"
)
}
Expand Down
Expand Up @@ -47,8 +47,7 @@ class LocationSpec {

assertThat(location.toString()).isEqualTo(
"Location(source=2:5, endSource=2:11, text=22:28, " +
"filePath=FilePath(absolutePath=${location.filePath.absolutePath}, " +
"basePath=${location.filePath.basePath}, relativePath=${location.filePath.relativePath}))"
"path=${location.path})"
)
}
}
@@ -1,6 +1,5 @@
package io.gitlab.arturbosch.detekt.test

import io.github.detekt.psi.FilePath
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Location
Expand All @@ -10,7 +9,6 @@ import io.gitlab.arturbosch.detekt.api.Severity
import io.gitlab.arturbosch.detekt.api.SourceLocation
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

Expand Down Expand Up @@ -80,7 +78,7 @@ fun createIssueForRelativePath(
source = SourceLocation(1, 1),
endSource = SourceLocation(1, 1),
text = TextLocation(0, 0),
filePath = fromRelative(Path("/").absolute().resolve(basePath), Path(relativePath))
path = Path("/").absolute().resolve(basePath).resolve(relativePath)
),
ktElement = null
),
Expand Down Expand Up @@ -111,8 +109,7 @@ fun createLocation(
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)),
path = basePath?.let { Path(it).absolute().resolve(path) } ?: Path(path).absolute(),
)
}

Expand All @@ -130,10 +127,3 @@ private data class IssueImpl(
override val description: String,
) : Issue.RuleInfo
}

fun fromAbsolute(path: Path) = FilePath(absolutePath = path.normalize())
fun fromRelative(basePath: Path, relativePath: Path) = FilePath(
absolutePath = basePath.resolve(relativePath).normalize(),
basePath = basePath.normalize(),
relativePath = relativePath
)
Expand Up @@ -2,6 +2,7 @@ package io.gitlab.arturbosch.detekt.core.reporting.console

import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.core.reporting.printIssues
import kotlin.io.path.absolutePathString

/**
* Contains all rule violations grouped by file location.
Expand All @@ -12,7 +13,7 @@ class FileBasedIssuesReport : AbstractIssuesReport() {
override val id: String = "FileBasedIssuesReport"

override fun render(issues: List<Issue>): String {
val issuesPerFile = issues.groupBy { it.entity.location.filePath.absolutePath.toString() }
val issuesPerFile = issues.groupBy { it.entity.location.path.absolutePathString() }
return printIssues(issuesPerFile)
}
}
Expand Up @@ -28,10 +28,10 @@ class FileBasedIssuesReportSpec {

assertThat(output).isEqualTo(
"""
${location1.filePath.absolutePath}
${location1.path}
TestSmell - [TestMessage] at ${location1.compact()}
TestSmell - [TestMessage] at ${location1.compact()}
${location2.filePath.absolutePath}
${location2.path}
TestSmell - [TestMessage] at ${location2.compact()}
""".trimIndent()
Expand Down
Expand Up @@ -4,8 +4,7 @@ import com.pinterest.ktlint.rule.engine.core.api.editorconfig.CODE_STYLE_PROPERT
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfigProperty
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.INDENT_STYLE_PROPERTY
import io.github.detekt.psi.FilePath
import io.github.detekt.psi.toFilePath
import io.github.detekt.psi.absolutePath
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.CorrectableCodeSmell
Expand All @@ -21,6 +20,7 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.JavaDummyElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.JavaDummyHolder
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtPsiFactory
import java.nio.file.Path

/**
* Rule to detect formatting violations.
Expand All @@ -38,13 +38,13 @@ abstract class FormattingRule(config: Config, description: String) : Rule(config

private lateinit var positionByOffset: (offset: Int) -> Pair<Int, Int>
private lateinit var root: KtFile
private lateinit var originalFilePath: FilePath
private lateinit var originalFilePath: Path

override fun visit(root: KtFile) {
val fileCopy = KtPsiFactory(root.project).createPhysicalFile(root.name, root.modifiedText ?: root.text)

this.root = fileCopy
originalFilePath = root.toFilePath()
originalFilePath = root.absolutePath()
positionByOffset = KtLintLineColCalculator.calculateLineColByOffset(fileCopy.text)

wrapping.beforeFirstNode(computeEditorConfigProperties())
Expand Down Expand Up @@ -95,7 +95,7 @@ abstract class FormattingRule(config: Config, description: String) : Rule(config
endSource = SourceLocation(line, column),
// Use offset + 1 since ktlint always reports a single location.
text = TextLocation(offset, offset + 1),
filePath = originalFilePath
path = originalFilePath
)
val entity = Entity.from(node.psi, location)

Expand Down
Expand Up @@ -75,7 +75,7 @@ class FormattingRuleSpec {
val rule = ChainWrapping(Config.empty)
val findings = rule.visitFile(compileForTest(expectedPath))
assertThat(findings).anySatisfy { finding ->
assertThat(finding.location.filePath.absolutePath.toString()).isEqualTo(expectedPath.toString())
assertThat(finding.location.path).isEqualTo(expectedPath)
}
}
}
10 changes: 0 additions & 10 deletions detekt-psi-utils/api/detekt-psi-utils.api
Expand Up @@ -3,15 +3,6 @@ public final class io/github/detekt/psi/AnnotationExcluder {
public final fun shouldExclude (Ljava/util/List;)Z
}

public final class io/github/detekt/psi/FilePath {
public fun <init> (Ljava/nio/file/Path;Ljava/nio/file/Path;Ljava/nio/file/Path;)V
public synthetic fun <init> (Ljava/nio/file/Path;Ljava/nio/file/Path;Ljava/nio/file/Path;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getAbsolutePath ()Ljava/nio/file/Path;
public final fun getBasePath ()Ljava/nio/file/Path;
public final fun getRelativePath ()Ljava/nio/file/Path;
public fun toString ()Ljava/lang/String;
}

public abstract class io/github/detekt/psi/FunctionMatcher {
public static final field Companion Lio/github/detekt/psi/FunctionMatcher$Companion;
public abstract fun match (Lorg/jetbrains/kotlin/descriptors/CallableDescriptor;)Z
Expand All @@ -38,7 +29,6 @@ public final class io/github/detekt/psi/KtFilesKt {
public static final fun getAbsolutePath (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;)Ljava/nio/file/Path;
public static final fun getLineAndColumnInPsiFile (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;Lorg/jetbrains/kotlin/com/intellij/openapi/util/TextRange;)Lorg/jetbrains/kotlin/diagnostics/PsiDiagnosticUtils$LineAndColumn;
public static final fun setAbsolutePath (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;Ljava/nio/file/Path;)V
public static final fun toFilePath (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;)Lio/github/detekt/psi/FilePath;
}

public final class io/gitlab/arturbosch/detekt/rules/AllowedExceptionNamePatternKt {
Expand Down
39 changes: 0 additions & 39 deletions detekt-psi-utils/src/main/kotlin/io/github/detekt/psi/KtFiles.kt
Expand Up @@ -34,45 +34,6 @@ instead.
*/
fun PsiFile.absolutePath(): Path = absolutePath ?: Path(virtualFile.path)

/**
* Represents both absolute path and relative path if available.
*/
class FilePath(
val absolutePath: Path,
val basePath: Path? = null,
val relativePath: Path? = null
) {

init {
require(
basePath == null ||
relativePath == null ||
absolutePath == basePath.resolve(relativePath).normalize()
) {
"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 =
"FilePath(absolutePath=$absolutePath, basePath=$basePath, relativePath=$relativePath)"
}

fun PsiFile.toFilePath(): FilePath {
return when {
basePath != null && relativePath != null -> FilePath(
absolutePath = absolutePath(),
basePath = basePath,
relativePath = relativePath
)

basePath == null && relativePath == null -> FilePath(absolutePath = absolutePath())
else -> error("Cannot build a FilePath from base path = $basePath and relative path = $relativePath")
}
}

// #3317 If any rule mutates the PsiElement, searching the original PsiElement may throw an exception.
fun getLineAndColumnInPsiFile(file: PsiFile, range: TextRange): PsiDiagnosticUtils.LineAndColumn? {
return if (file.textLength == 0) {
Expand Down
Expand Up @@ -5,8 +5,6 @@ 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 @@ -45,15 +43,4 @@ 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)")
}
}
Expand Up @@ -142,7 +142,7 @@ class HtmlOutputReport : BuiltInOutputReport, OutputReport() {
issues
.sortedWith(
compareBy(
{ it.location.filePath.absolutePath.toString() },
{ it.location.path },
{ it.location.source.line },
{ it.location.source.column },
)
Expand All @@ -157,8 +157,7 @@ class HtmlOutputReport : BuiltInOutputReport, OutputReport() {
}

private fun FlowContent.renderIssue(issue: Issue) {
val filePath = basePath?.let { issue.location.filePath.absolutePath.relativeTo(it) }
?: issue.location.filePath.absolutePath
val filePath = basePath?.let { issue.location.path.relativeTo(it) } ?: issue.location.path
val pathString = filePath.invariantSeparatorsPathString
span("location") {
text(
Expand Down

0 comments on commit 65f3164

Please sign in to comment.