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

Make plugin testable by stubbing the detekt instance avoiding compiler classpath errors #177

Merged
merged 3 commits into from Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -13,12 +13,14 @@ import io.github.detekt.tooling.api.spec.RulesSpec
import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.api.UnstableApi
import io.gitlab.arturbosch.detekt.idea.config.DetektConfigStorage
import io.gitlab.arturbosch.detekt.idea.util.DirectExecuter
import io.gitlab.arturbosch.detekt.idea.util.DirectExecutor
import io.gitlab.arturbosch.detekt.idea.util.PluginUtils
import io.gitlab.arturbosch.detekt.idea.util.absolutePath
import io.gitlab.arturbosch.detekt.idea.util.extractPaths
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.Paths
import java.util.ServiceLoader

class ConfiguredService(private val project: Project) {

Expand Down Expand Up @@ -70,7 +72,7 @@ class ConfiguredService(private val project: Project) {
}
}
execution {
executorService = DirectExecuter()
executorService = DirectExecutor()
}
}

Expand All @@ -97,7 +99,7 @@ class ConfiguredService(private val project: Project) {
}

val spec: ProcessingSpec = settings(filename, autoCorrect)
val detekt = DetektProvider.load().get(spec)
val detekt = loadProviderConsiderStubbed().get(spec)
val result = if (autoCorrect) {
runWriteAction { detekt.run(fileContent, filename) }
} else {
Expand All @@ -113,3 +115,10 @@ class ConfiguredService(private val project: Project) {
return result.container?.findings?.flatMap { it.value } ?: emptyList()
}
}

fun loadProviderConsiderStubbed(): DetektProvider {
val providers = ServiceLoader.load(DetektProvider::class.java, PluginUtils::class.java.classLoader).toList()
return providers
.find { it.javaClass.simpleName == "StubDetektProvider" }
?: providers.first()
}
Expand Up @@ -11,8 +11,8 @@ import io.gitlab.arturbosch.detekt.api.TextLocation
import io.gitlab.arturbosch.detekt.idea.config.DetektConfigStorage
import io.gitlab.arturbosch.detekt.idea.intention.AutoCorrectIntention
import io.gitlab.arturbosch.detekt.idea.util.isDetektEnabled
import io.gitlab.arturbosch.detekt.idea.util.isKotlinFile
import io.gitlab.arturbosch.detekt.idea.util.showNotification
import org.jetbrains.kotlin.idea.KotlinLanguage

class DetektAnnotator : ExternalAnnotator<PsiFile, List<Finding>>() {

Expand All @@ -22,7 +22,7 @@ class DetektAnnotator : ExternalAnnotator<PsiFile, List<Finding>>() {
if (!collectedInfo.project.isDetektEnabled()) {
return emptyList()
}
if (collectedInfo.isKotlinFile()) {
Copy link
Member Author

@arturbosch arturbosch Jun 5, 2022

Choose a reason for hiding this comment

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

This line was wrong. It would have skipped every Kotlin file but did not because apparently psiFile.language != KotlinLanguage.INSTANCE,
Fixing the tests made this mistake obvious.
Testing for KtFile does not work either as IntelliJ may use some kind of LightPsiElement structures.
We have to compare language id's here. Kotlin light elements also use KotlinLanguage.INSTANCE.

if (collectedInfo.language.id != KotlinLanguage.INSTANCE.id) {
return emptyList()
}

Expand Down
@@ -1,7 +1,6 @@
package io.gitlab.arturbosch.detekt.idea.config

import com.intellij.openapi.components.PersistentStateComponent
import com.intellij.openapi.components.ServiceManager
import com.intellij.openapi.components.State
import com.intellij.openapi.components.Storage
import com.intellij.openapi.project.Project
Expand Down
Expand Up @@ -3,7 +3,7 @@ package io.gitlab.arturbosch.detekt.idea.util
import java.util.concurrent.AbstractExecutorService
import java.util.concurrent.TimeUnit

class DirectExecuter : AbstractExecutorService() {
class DirectExecutor : AbstractExecutorService() {

override fun isTerminated(): Boolean = true

Expand Down
Expand Up @@ -7,20 +7,16 @@ import com.intellij.openapi.actionSystem.AnActionEvent
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.options.newEditor.SettingsDialog
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiFile
import io.gitlab.arturbosch.detekt.idea.DETEKT
import io.gitlab.arturbosch.detekt.idea.config.DetektConfig
import io.gitlab.arturbosch.detekt.idea.config.DetektConfigStorage
import org.jetbrains.kotlin.idea.KotlinLanguage
import java.io.File
import java.nio.file.Path
import java.nio.file.Paths

fun Project.isDetektEnabled(): Boolean =
DetektConfigStorage.instance(this).enableDetekt

fun PsiFile.isKotlinFile(): Boolean = language == KotlinLanguage.INSTANCE

fun absolutePath(project: Project, path: String): String =
if (path.isBlank() || File(path).isAbsolute) {
path
Expand Down
Expand Up @@ -4,7 +4,6 @@ import io.github.detekt.test.utils.readResourceContent
import io.github.detekt.test.utils.resourceAsPath
import io.gitlab.arturbosch.detekt.idea.config.DetektConfigStorage
import org.assertj.core.api.Assertions.assertThat
import org.assertj.core.api.Assertions.assertThatCode
import org.junit.jupiter.api.Test

class ConfiguredServiceTest : DetektPluginTestCase() {
Expand Down Expand Up @@ -58,22 +57,13 @@ class ConfiguredServiceTest : DetektPluginTestCase() {
fun `expect detekt runs successfully`() {
val service = ConfiguredService(project)

// If a NoSuchMethodError is thrown, it means detekt-core was called and creating a KotlinEnvironment
// failed due to conflicted compiler vs embedded-compiler dependency.
// IntelliJ isolates plugins in an own classloader so detekt runs fine.
// In the testcase this is not possible but it is enough to prove detekt runs and does not crash due to
// regressions in this plugin.
assertThatCode {
assertThat(
service.execute(
readResourceContent("testData/Poko.kt"),
resourceAsPath("testData/Poko.kt").toString(),
autoCorrect = false
)
}
.isInstanceOf(ClassCastException::class.java)
.hasMessageContaining(
"class org.jetbrains.kotlin.idea.KotlinFileType cannot be cast"
)
).isNotEmpty
}

@Test
Expand Down
Expand Up @@ -2,32 +2,27 @@ package io.gitlab.arturbosch.detekt.idea

import com.intellij.openapi.application.WriteAction
import com.intellij.psi.PsiFile
import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.idea.config.DetektConfigStorage
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test

class DetektAnnotatorTest : DetektPluginTestCase() {

@Test
fun `returns empty list when detekt is disabled`() {
val file = myFixture.copyFileToProject("Poko.kt")
val psi = WriteAction.computeAndWait<PsiFile, Throwable> { psiManager.findFile(file)!! }

val findings = DetektAnnotator().doAnnotate(psi)

assertThat(findings).isEmpty()
assertThat(runAnnotator(enabled = false)).isEmpty()
}

@Test
@Disabled("Dependency conflicts between embeddable and normal kotlin compiler.")
fun `report issues when detekt is enabled`() {
DetektConfigStorage.instance(project).enableDetekt = true
assertThat(runAnnotator(enabled = true)).isNotEmpty
}

private fun runAnnotator(enabled: Boolean): List<Finding> {
DetektConfigStorage.instance(project).enableDetekt = enabled
val file = myFixture.copyFileToProject("Poko.kt")
val psi = WriteAction.computeAndWait<PsiFile, Throwable> { psiManager.findFile(file)!! }

val findings = DetektAnnotator().doAnnotate(psi)

assertThat(findings).isNotEmpty()
return DetektAnnotator().doAnnotate(psi)
}
}
Expand Up @@ -15,6 +15,12 @@ import org.junit.jupiter.api.TestInstance
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
open class DetektPluginTestCase : BasePlatformTestCase() {

init {
// necessary in tests up from 2021.3
// https://plugins.jetbrains.com/docs/intellij/api-changes-list-2021.html#20213
System.setProperty("idea.force.use.core.classloader", "true")
}

override fun getTestDataPath(): String = resourceAsPath("testData").toString()

override fun isWriteActionRequired(): Boolean = true
Expand Down
95 changes: 95 additions & 0 deletions src/test/kotlin/io/gitlab/arturbosch/detekt/idea/DetektStub.kt
@@ -0,0 +1,95 @@
package io.gitlab.arturbosch.detekt.idea

import io.github.detekt.psi.FilePath
import io.github.detekt.tooling.api.AnalysisResult
import io.github.detekt.tooling.api.Detekt
import io.github.detekt.tooling.api.DetektProvider
import io.github.detekt.tooling.api.spec.ProcessingSpec
import io.github.detekt.tooling.internal.DefaultAnalysisResult
import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Detektion
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Finding
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Location
import io.gitlab.arturbosch.detekt.api.Notification
import io.gitlab.arturbosch.detekt.api.ProjectMetric
import io.gitlab.arturbosch.detekt.api.RuleSetId
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.com.intellij.openapi.util.Key
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.resolve.BindingContext
import java.nio.file.Path
import java.nio.file.Paths

class DetektProviderStub : DetektProvider {

override fun get(processingSpec: ProcessingSpec): Detekt = DetektStub()
}

class DetektStub : Detekt {

override fun run(): AnalysisResult {
throw UnsupportedOperationException()
}

override fun run(path: Path): AnalysisResult {
throw UnsupportedOperationException()
}

override fun run(sourceCode: String, filename: String): AnalysisResult {
if (!filename.contains("Poko.kt")) {
throw UnsupportedOperationException("Only Poko.kt runs are supported.")
}
return DefaultAnalysisResult(object : Detektion {
override val findings: Map<RuleSetId, List<Finding>> = mapOf(
"empty-blocks" to listOf(
CodeSmell(
Issue(
"EmptyDefaultConstructor",
Severity.Minor,
"empty",
Debt.FIVE_MINS
),
Entity(
"Poko",
"testData.Poko.kt",
Location(
SourceLocation(3, 10),
TextLocation(28, 30),
FilePath(Paths.get(filename))
)
),
"empty constructor"
)
)
)
override val metrics: Collection<ProjectMetric> = emptyList()
override val notifications: Collection<Notification> = emptyList()

override fun add(notification: Notification) {
// ignore
}

override fun add(projectMetric: ProjectMetric) {
// ignore
}

override fun <V> addData(key: Key<V>, value: V) {
// ignore
}

override fun <V> getData(key: Key<V>): V? {
// ignore
return null
}
})
}

override fun run(files: Collection<KtFile>, bindingContext: BindingContext): AnalysisResult {
throw UnsupportedOperationException()
}
}
@@ -0,0 +1 @@
io.gitlab.arturbosch.detekt.idea.DetektProviderStub