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.

Sign up for GitHub

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

Jump to bottom

Remove PsiFile.absolutePath #7254

Merged
merged 3 commits into from
May 7, 2024
Merged

Remove PsiFile.absolutePath #7254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5444c1d
Remove PsiFile.absolutePath
3flex May 3, 2024
0dbdcbf
Use real file to check path includes/excludes
3flex May 3, 2024
292a526
Add KtFile.absolutePath()
3flex May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/AnalyzerSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.gitlab.arturbosch.detekt.core
import io.github.detekt.test.utils.StringPrintStream
import io.github.detekt.test.utils.compileContentForTest
import io.github.detekt.test.utils.compileForTest
import io.github.detekt.test.utils.resourceAsPath
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Entity
Expand All @@ -25,8 +26,6 @@ import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.ValueSource
import kotlin.io.path.Path
import kotlin.io.path.absolute

@KotlinCoreEnvironmentTest
class AnalyzerSpec(val env: KotlinCoreEnvironment) {
Expand Down Expand Up @@ -349,12 +348,13 @@ class AnalyzerSpec(val env: KotlinCoreEnvironment) {
path: String,
config: Config,
): Boolean {
val root = Path("").absolute()
val root = resourceAsPath("include_exclude")
val pathToCheck = resourceAsPath("include_exclude").resolve(path)

return createProcessingSettings(config = config) { project { basePath = root } }
.use { settings ->
Analyzer(settings, listOf(CustomRuleSetProvider()), emptyList())
.run(listOf(compileContentForTest("", Path(path))))
.run(listOf(compileForTest(pathToCheck)))
.isNotEmpty()
}
}
Expand Down
9 changes: 4 additions & 5 deletions detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/util/ConfigExtensionsSpec.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package io.gitlab.arturbosch.detekt.core.util

import io.github.detekt.test.utils.compileContentForTest
import io.github.detekt.test.utils.compileForTest
import io.github.detekt.test.utils.resourceAsPath
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.yamlConfigFromContent
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.DisplayName
import org.junit.jupiter.api.Nested
import org.junit.jupiter.api.Test
import kotlin.io.path.Path
import kotlin.io.path.absolute

class IsActiveOrDefaultSpec {

Expand All @@ -35,8 +34,8 @@ class IsActiveOrDefaultSpec {
@Nested
class ShouldAnalyzeFileSpec {

private val basePath = Path("/cases").absolute()
private val file = compileContentForTest("", path = Path("/cases/Default.kt"))
private val basePath = resourceAsPath("cases")
private val file = compileForTest(basePath.resolve("Default.kt"))

@Test
fun `analyzes file with an empty config`() {
Expand Down
Empty file added detekt-core/src/test/resources/include_exclude/foo/Test.kt
View file
Open in desktop
Empty file.
1 change: 0 additions & 1 deletion detekt-parser/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ plugins {

dependencies {
api(libs.kotlin.compilerEmbeddable)
implementation(projects.detektPsiUtils)
testImplementation(projects.detektTestUtils)
testImplementation(libs.assertj)
}
Expand Down
6 changes: 1 addition & 5 deletions detekt-parser/src/main/kotlin/io/github/detekt/parser/KtCompiler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package io.github.detekt.parser

import io.github.detekt.psi.absolutePath
import org.intellij.lang.annotations.Language
import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment
import org.jetbrains.kotlin.com.intellij.openapi.util.text.StringUtilRt
import org.jetbrains.kotlin.com.intellij.psi.util.PsiUtilCore
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtPsiFactory
import java.nio.file.Path
import kotlin.io.path.absolute
import kotlin.io.path.isRegularFile
import kotlin.io.path.name

Expand All @@ -28,7 +26,5 @@ open class KtCompiler(
}

fun createKtFile(@Language("kotlin") content: String, path: Path): KtFile =
psiFileFactory.createPhysicalFile(path.name, StringUtilRt.convertLineSeparators(content)).apply {
absolutePath = path.absolute().normalize()
}
psiFileFactory.createPhysicalFile(path.name, StringUtilRt.convertLineSeparators(content))
}
3 changes: 1 addition & 2 deletions detekt-psi-utils/api/detekt-psi-utils.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ public final class io/github/detekt/psi/FunctionMatcher$Companion {

public final class io/github/detekt/psi/KtFilesKt {
public static final fun absolutePath (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;)Ljava/nio/file/Path;
public static final fun absolutePath (Lorg/jetbrains/kotlin/psi/KtFile;)Ljava/nio/file/Path;
public static final fun fileNameWithoutSuffix (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;Ljava/util/List;)Ljava/lang/String;
public static synthetic fun fileNameWithoutSuffix$default (Lorg/jetbrains/kotlin/com/intellij/psi/PsiFile;Ljava/util/List;ILjava/lang/Object;)Ljava/lang/String;
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 final class io/gitlab/arturbosch/detekt/rules/AllowedExceptionNamePatternKt {
Expand Down
12 changes: 4 additions & 8 deletions detekt-psi-utils/src/main/kotlin/io/github/detekt/psi/KtFiles.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package io.github.detekt.psi

import org.jetbrains.kotlin.com.intellij.openapi.util.Key
import org.jetbrains.kotlin.com.intellij.openapi.util.TextRange
import org.jetbrains.kotlin.com.intellij.psi.PsiFile
import org.jetbrains.kotlin.diagnostics.DiagnosticUtils
import org.jetbrains.kotlin.diagnostics.PsiDiagnosticUtils
import org.jetbrains.kotlin.psi.UserDataProperty
import org.jetbrains.kotlin.psi.KtFile
import java.nio.file.Path
import kotlin.io.path.Path

Expand All @@ -26,13 +25,10 @@ fun PsiFile.fileNameWithoutSuffix(multiplatformTargetSuffixes: List<String> = em
return fileName
}

var PsiFile.absolutePath: Path? by UserDataProperty(Key("absolutePath"))
fun PsiFile.absolutePath(): Path = Path(virtualFile.path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even remove this one, right? I guess that there should be really few usages of this one. And, even if they are, it should be safe to cast them to KtFile. To keep the api as small as possible. This way we don't even need the comment :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My idea for keeping it is that 3rd party rule authors will have an easier time finding the right function to call - requiring a cast makes them less discoverable, particularly when using something like KtElement/PsiElement.containingFile which returns PsiFile.

One improvement would be to change this line to

fun PsiFile.absolutePath(): Path = (this as KtFile).absolutePath()

So that way it always queries the cached path. That would provide some additional value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with make it easier to find. Then maybe we can always cast it and just remove the new extension function. This is not really important, anyway.


/*
absolutePath will be null when the Kotlin compiler plugin is used. The file's path can be obtained from the virtual file
instead.
*/
fun PsiFile.absolutePath(): Path = absolutePath ?: Path(virtualFile.path)
// KtFile.virtualFilePath is cached so should be a tiny bit more performant when called repeatedly for the same file.
fun KtFile.absolutePath(): Path = Path(virtualFilePath)

// #3317 If any rule mutates the PsiElement, searching the original PsiElement may throw an exception.
fun getLineAndColumnInPsiFile(file: PsiFile, range: TextRange): PsiDiagnosticUtils.LineAndColumn? {
Expand Down