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

Pass structure warnings to verification reports #1080

Merged
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.
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 @@ -6,12 +6,13 @@ package com.jetbrains.plugin.structure.intellij.plugin

import com.jetbrains.plugin.structure.base.plugin.PluginIcon
import com.jetbrains.plugin.structure.base.plugin.ThirdPartyDependency
import com.jetbrains.plugin.structure.base.problems.PluginProblem
import com.jetbrains.plugin.structure.intellij.version.IdeVersion
import org.jdom2.Document
import org.jdom2.Element
import java.nio.file.Path

class IdePluginImpl : IdePlugin {
class IdePluginImpl : IdePlugin, StructurallyValidated {
override var pluginId: String? = null

override var pluginName: String? = null
Expand Down Expand Up @@ -77,6 +78,8 @@ class IdePluginImpl : IdePlugin {
override fun isCompatibleWithIde(ideVersion: IdeVersion) =
(sinceBuild == null || sinceBuild!! <= ideVersion) && (untilBuild == null || ideVersion <= untilBuild!!)

override val problems: MutableList<PluginProblem> = mutableListOf()

override fun toString(): String =
(pluginId ?: pluginName ?: "<unknown plugin ID>") + (pluginVersion?.let { ":$it" } ?: "")
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ internal class PluginCreator private constructor(
val contentModules = arrayListOf<Module>()

private val plugin = IdePluginImpl()
private val problems = arrayListOf<PluginProblem>()
private val problems: MutableList<PluginProblem>
get() = plugin.problems

val pluginId: String?
get() = plugin.pluginId ?: parentPlugin?.pluginId
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.jetbrains.plugin.structure.intellij.plugin

import com.jetbrains.plugin.structure.base.problems.PluginProblem

interface StructurallyValidated {
val problems: List<PluginProblem>
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,12 @@ class HtmlResultPrinter(
printShortAndFullDescriptionItems("Override-only API usages", overrideOnlyMethodUsages) { it.shortDescription to it.fullDescription }
if (pluginStructureWarnings.isNotEmpty()) {
printShortAndFullDescription("Plugin structure defects") {
pluginStructureWarnings.forEach {
+it.message
ul {
pluginStructureWarnings.forEach {
li {
+it.message
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ interface PluginDetailsProvider {
*/
fun providePluginDetails(pluginInfo: PluginInfo, pluginFileLock: FileLock): Result

/**
* Creates [PluginDetails] for existing plugin with warnings of the plugin structure.
*/
fun providePluginDetails(pluginInfo: PluginInfo, idePlugin: IdePlugin, warnings: List<PluginProblem>): Result

/**
* Represents possible results of [providing] [providePluginDetails] the [PluginDetails].
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.jetbrains.plugin.structure.intellij.classes.locator.CompileServerExte
import com.jetbrains.plugin.structure.intellij.classes.plugin.IdePluginClassesFinder
import com.jetbrains.plugin.structure.intellij.plugin.IdePlugin
import com.jetbrains.plugin.structure.intellij.plugin.IdePluginManager
import com.jetbrains.plugin.structure.intellij.plugin.StructurallyValidated
import com.jetbrains.plugin.structure.intellij.problems.UnableToReadPluginFile
import com.jetbrains.pluginverifier.repository.PluginInfo
import com.jetbrains.pluginverifier.repository.files.FileLock
Expand All @@ -34,7 +35,7 @@ class PluginDetailsProviderImpl(private val extractDirectory: Path) : PluginDeta
readPluginClasses(
pluginInfo,
plugin,
warnings,
plugin.problems,
pluginFileLock
)
}
Expand All @@ -50,14 +51,9 @@ class PluginDetailsProviderImpl(private val extractDirectory: Path) : PluginDeta
override fun providePluginDetails(
pluginInfo: PluginInfo,
idePlugin: IdePlugin
) = readPluginClasses(pluginInfo, idePlugin, emptyList(), null)

override fun providePluginDetails(
pluginInfo: PluginInfo,
idePlugin: IdePlugin,
warnings: List<PluginProblem>
) = readPluginClasses(pluginInfo, idePlugin, warnings, null)

): PluginDetailsProvider.Result {
return readPluginClasses(pluginInfo, idePlugin, idePlugin.problems, null)
}

private fun readPluginClasses(
pluginInfo: PluginInfo,
Expand Down Expand Up @@ -85,4 +81,7 @@ class PluginDetailsProviderImpl(private val extractDirectory: Path) : PluginDeta
)
}

private val IdePlugin.problems: List<PluginProblem>
get() = if (this is StructurallyValidated) this.problems else emptyList()

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
package com.jetbrains.pluginverifier.output

import com.jetbrains.plugin.structure.classes.resolvers.FileOrigin
import com.jetbrains.plugin.structure.intellij.plugin.IdePluginManager
import com.jetbrains.plugin.structure.intellij.plugin.PluginDependencyImpl
import com.jetbrains.plugin.structure.intellij.problems.ForbiddenPluginIdPrefix
import com.jetbrains.plugin.structure.intellij.problems.NoModuleDependencies
import com.jetbrains.plugin.structure.intellij.version.IdeVersion
import com.jetbrains.pluginverifier.PluginVerificationResult
import com.jetbrains.pluginverifier.PluginVerificationTarget
import com.jetbrains.pluginverifier.dependencies.DependenciesGraph
import com.jetbrains.pluginverifier.dependencies.DependencyNode
import com.jetbrains.pluginverifier.dependencies.MissingDependency
import com.jetbrains.pluginverifier.dymamic.DynamicPluginStatus
import com.jetbrains.pluginverifier.jdk.JdkVersion
import com.jetbrains.pluginverifier.repository.PluginInfo
import com.jetbrains.pluginverifier.results.hierarchy.ClassHierarchy
import com.jetbrains.pluginverifier.results.instruction.Instruction
import com.jetbrains.pluginverifier.results.location.ClassLocation
import com.jetbrains.pluginverifier.results.location.MethodLocation
import com.jetbrains.pluginverifier.results.location.toReference
import com.jetbrains.pluginverifier.results.modifiers.Modifiers
import com.jetbrains.pluginverifier.results.problems.CompatibilityProblem
import com.jetbrains.pluginverifier.results.problems.MethodNotFoundProblem
import com.jetbrains.pluginverifier.results.problems.SuperInterfaceBecameClassProblem
import com.jetbrains.pluginverifier.results.reference.ClassReference
import com.jetbrains.pluginverifier.results.reference.MethodReference
import com.jetbrains.pluginverifier.tasks.InvalidPluginFile
import com.jetbrains.pluginverifier.usages.experimental.ExperimentalClassUsage
import com.jetbrains.pluginverifier.usages.internal.InternalClassUsage
import com.jetbrains.pluginverifier.usages.nonExtendable.NonExtendableTypeInherited
import com.jetbrains.pluginverifier.warnings.PluginStructureWarning
import org.junit.Assert.assertEquals
import java.io.StringWriter
import kotlin.io.path.Path

const val PLUGIN_ID = "pluginId"
const val PLUGIN_VERSION = "1.0"

typealias VerifiedPluginHandler = (PluginVerificationResult.Verified) -> Unit

open class BaseOutputTest<T : ResultPrinter> {
private val pluginInfo = mockPluginInfo()
private val verificationTarget = PluginVerificationTarget.IDE(IdeVersion.createIdeVersion("232"), JdkVersion("11", null))

protected lateinit var out: StringWriter
protected lateinit var resultPrinter: T

fun interface VerifiedPluginWithPrinterRunner<T, R> {
fun run(resultPrinter: T, result: R)
}

open fun setUp() {
out = StringWriter()
}

private val dependenciesGraph: DependenciesGraph = DependenciesGraph(
verifiedPlugin = DependencyNode(PLUGIN_ID, PLUGIN_VERSION),
vertices = emptyList(),
edges = emptyList(),
missingDependencies = emptyMap())

open fun `when plugin is compatible`(testRunner: VerifiedPluginHandler) {
testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph))
}

open fun `when plugin has compatibility warnings`(testRunner: VerifiedPluginHandler) {
testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph, mockCompatibilityProblems()))
}

open fun `when plugin has structural problems`(testRunner: VerifiedPluginHandler) {
val structureWarnings = setOf(
PluginStructureWarning(NoModuleDependencies(IdePluginManager.PLUGIN_XML))
)
testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph, pluginStructureWarnings = structureWarnings))
}

open fun `when plugin has internal API usage problems`(testRunner: VerifiedPluginHandler) {
val internalApiUsages = setOf(
InternalClassUsage(ClassReference("com.jetbrains.InternalClass"), internalApiClassLocation, mockMethodLocationInSampleStuffFactory)
)

testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph, internalApiUsages = internalApiUsages))
}

open fun `when plugin has non-extendable API usages problems`(testRunner: VerifiedPluginHandler) {
val nonExtendableClass = ClassLocation("NonExtendableClass", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
val extendingClass = ClassLocation("ExtendingClass", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)

val nonExtendableApiUsages = setOf(
NonExtendableTypeInherited(nonExtendableClass, extendingClass)
)

testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph, nonExtendableApiUsages = nonExtendableApiUsages))
}

open fun `when plugin has experimental API usage problems`(testRunner: VerifiedPluginHandler) {
val experimentalClass = ClassLocation("ExperimentalClass", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
val extendingClass = ClassLocation("ExtendingClass", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
val usageLocation = MethodLocation(
extendingClass,
"someMethod",
"()V",
emptyList(),
null,
Modifiers.of(Modifiers.Modifier.PUBLIC)
)

val experimentalApiUsages = setOf(
ExperimentalClassUsage(experimentalClass.toReference(), experimentalClass, usageLocation)
)

testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph, experimentalApiUsages = experimentalApiUsages))
}

open fun `when plugin has missing dependencies`(testRunner: VerifiedPluginHandler) {
val pluginDependency = DependencyNode(PLUGIN_ID, PLUGIN_VERSION)
val expectedDependency = MissingDependency(PluginDependencyImpl("MissingPlugin", true, false), "Dependency MissingPlugin is not found among the bundled plugins of IU-211.500")

val dependenciesGraph = DependenciesGraph(
verifiedPlugin = pluginDependency,
vertices = emptyList(),
edges = emptyList(),
missingDependencies = mapOf(pluginDependency to setOf(expectedDependency))
)

testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph))
}

open fun `when plugin is dynamic`(testRunner: VerifiedPluginHandler) {
testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph, dynamicPluginStatus = DynamicPluginStatus.MaybeDynamic))
}

open fun `when plugin is dynamic and has structural warnings`(testRunner: VerifiedPluginHandler) {
val structureWarnings = setOf(
PluginStructureWarning(NoModuleDependencies(IdePluginManager.PLUGIN_XML))
)
testRunner.runTest(PluginVerificationResult.Verified(pluginInfo, verificationTarget, dependenciesGraph,
dynamicPluginStatus = DynamicPluginStatus.MaybeDynamic,
pluginStructureWarnings = structureWarnings
))
}

open fun `when plugin has structural problems with invalid plugin ID`(testRunner: VerifiedPluginWithPrinterRunner<T, List<InvalidPluginFile>>) {
val pluginId = "com.example.intellij"
val prefix = "com.example"
val invalidPluginFiles = listOf(
InvalidPluginFile(Path("plugin.zip"), listOf(ForbiddenPluginIdPrefix(pluginId, prefix)))
)

testRunner.run(resultPrinter, invalidPluginFiles)
}

fun output() = out.buffer.toString()

private fun printResults(verificationResult: PluginVerificationResult) {
resultPrinter.printResults(listOf(verificationResult))
}

fun assertOutput(expected: String) {
assertEquals(expected, output())
}

private fun VerifiedPluginHandler.runTest(verificationResult: PluginVerificationResult.Verified) {
printResults(verificationResult)
this(verificationResult)
}
}

private fun mockPluginInfo(): PluginInfo =
object : PluginInfo(PLUGIN_ID, PLUGIN_ID, PLUGIN_VERSION, null, null, null) {}

private fun mockCompatibilityProblems(): Set<CompatibilityProblem> =
setOf(superInterfaceBecameClassProblem(), superInterfaceBecameClassProblemInOtherLocation(), methodNotFoundProblem(), methodNotFoundProblemInSampleStuffFactoryClass())

private fun superInterfaceBecameClassProblem(): SuperInterfaceBecameClassProblem {
val child = ClassLocation("com.jetbrains.plugin.Child", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
val clazz = ClassLocation("com.jetbrains.plugin.Parent", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
return SuperInterfaceBecameClassProblem(child, clazz)
}

private fun superInterfaceBecameClassProblemInOtherLocation(): SuperInterfaceBecameClassProblem {
val child = ClassLocation("com.jetbrains.plugin.pkg.Child", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
val clazz = ClassLocation("com.jetbrains.plugin.pkg.Parent", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
return SuperInterfaceBecameClassProblem(child, clazz)
}

private val javaLangObjectClassHierarchy = ClassHierarchy(
"java/lang/Object",
false,
null,
emptyList()
)

private val sampleStuffFactoryLocation = ClassLocation("SampleStuffFactory", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)
private val internalApiClassLocation = ClassLocation("InternalApiRegistrar", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin)

private val mockMethodLocationInSampleStuffFactory = MethodLocation(
sampleStuffFactoryLocation,
"produceStuff",
"()V",
emptyList(),
null,
Modifiers.of(Modifiers.Modifier.PUBLIC)
)

private fun methodNotFoundProblem(): MethodNotFoundProblem {
val deletedClassRef = ClassReference("org/some/deleted/Class")
val referencingMethodLocation = MethodLocation(
ClassLocation("SomeClassUsingDeletedClass", null, Modifiers.of(Modifiers.Modifier.PUBLIC), SomeFileOrigin),
"someMethodReferencingDeletedClass",
"()V",
emptyList(),
null,
Modifiers.of(Modifiers.Modifier.PUBLIC)
)
return MethodNotFoundProblem(
MethodReference(deletedClassRef, "foo", "()V"),
referencingMethodLocation,
Instruction.INVOKE_VIRTUAL,
javaLangObjectClassHierarchy
)
}

private fun methodNotFoundProblemInSampleStuffFactoryClass(): MethodNotFoundProblem {
val deletedClassRef = ClassReference("org/some/deleted/Class")
return MethodNotFoundProblem(
MethodReference(deletedClassRef, "foo", "()V"),
mockMethodLocationInSampleStuffFactory,
Instruction.INVOKE_VIRTUAL,
javaLangObjectClassHierarchy
)
}

private object SomeFileOrigin : FileOrigin {
override val parent: FileOrigin? = null
}
Loading
Loading