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

Fix unknown classes fields #260

Merged
merged 1 commit into from
Aug 14, 2024
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
7 changes: 7 additions & 0 deletions buildSrc/src/main/kotlin/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ object Versions {

// libs for tests only
const val jgit_test_only_version = "5.9.0.202009080501-r"
const val commons_compress_test_only_version = "1.21"
}

fun dep(group: String, name: String, version: String): String = "$group:$name:$version"
Expand Down Expand Up @@ -332,6 +333,12 @@ object Libs {
name = "org.eclipse.jgit",
version = Versions.jgit_test_only_version
)

val commons_compress_test_only_lib = dep(
group = "org.apache.commons",
name = "commons-compress",
version = Versions.commons_compress_test_only_version
)
}

object Plugins {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface JcClasspath : Closeable, CommonProject {
/** locations of this classpath */
val locations: List<JcByteCodeLocation>
val registeredLocations: List<RegisteredLocation>
val registeredLocationIds: Set<Long>
val features: List<JcClasspathFeature>?

/**
Expand Down
1 change: 1 addition & 0 deletions jacodb-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ dependencies {
testFixturesImplementation(Libs.jetbrains_annotations)
testFixturesImplementation(Libs.kotlinx_coroutines_core)
testFixturesImplementation(Libs.jgit_test_only_lib)
testFixturesImplementation(Libs.commons_compress_test_only_lib)
}

tasks {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class JcClasspathImpl(

override val locations: List<JcByteCodeLocation> = locationsRegistrySnapshot.locations.mapNotNull { it.jcLocation }
override val registeredLocations: List<RegisteredLocation> = locationsRegistrySnapshot.locations

override val registeredLocationIds: Set<Long> = locationsRegistrySnapshot.ids
private val classpathVfs = ClasspathVfs(globalClassVFS, locationsRegistrySnapshot)
private val featuresChain = run {
val strictFeatures = features.filter { it !is UnknownClasses }
Expand Down
41 changes: 32 additions & 9 deletions jacodb-core/src/main/kotlin/org/jacodb/impl/JcDatabaseImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,31 @@

package org.jacodb.impl

import kotlinx.coroutines.*
import org.jacodb.api.jvm.*
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.joinAll
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.jacodb.api.jvm.JavaVersion
import org.jacodb.api.jvm.JcByteCodeLocation
import org.jacodb.api.jvm.JcClasspath
import org.jacodb.api.jvm.JcClasspathFeature
import org.jacodb.api.jvm.JcDatabase
import org.jacodb.api.jvm.JcDatabasePersistence
import org.jacodb.api.jvm.JcFeature
import org.jacodb.api.jvm.RegisteredLocation
import org.jacodb.impl.features.classpaths.ClasspathCache
import org.jacodb.impl.features.classpaths.KotlinMetadata
import org.jacodb.impl.features.classpaths.MethodInstructionsFeature
import org.jacodb.impl.features.classpaths.UnknownClassMethodsAndFields
import org.jacodb.impl.features.classpaths.UnknownClasses
import org.jacodb.impl.fs.JavaRuntime
import org.jacodb.impl.fs.asByteCodeLocation
import org.jacodb.impl.fs.filterExisting
Expand Down Expand Up @@ -76,14 +96,17 @@ class JcDatabaseImpl(
}

private fun List<JcClasspathFeature>?.appendBuiltInFeatures(): List<JcClasspathFeature> {
if (this != null && any { it is ClasspathCache }) {
return this + listOf(KotlinMetadata, MethodInstructionsFeature(settings.keepLocalVariableNames))
return mutableListOf<JcClasspathFeature>().also { result ->
result += orEmpty()
if (!result.any { it is ClasspathCache }) {
result += ClasspathCache(settings.cacheSettings)
}
result += KotlinMetadata
result += MethodInstructionsFeature(settings.keepLocalVariableNames)
if (result.any { it is UnknownClasses } && !result.any { it is UnknownClassMethodsAndFields }) {
result += UnknownClassMethodsAndFields
}
}
return listOf(
ClasspathCache(settings.cacheSettings),
KotlinMetadata,
MethodInstructionsFeature(settings.keepLocalVariableNames)
) + orEmpty()
}

override suspend fun classpath(dirOrJars: List<File>, features: List<JcClasspathFeature>?): JcClasspath {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ open class LocationsRegistrySnapshot(
val locations: List<RegisteredLocation>
) : Closeable {

val ids = locations.map { it.id }.toHashSet()
val ids = locations.mapTo(mutableSetOf()) { it.id }

override fun close() = registry.close(this)

Expand Down
19 changes: 7 additions & 12 deletions jacodb-core/src/main/kotlin/org/jacodb/impl/bytecode/KMetadata.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,15 @@ val JcMethod.kmConstructor: KmConstructor?

val JcParameter.kmParameter: KmValueParameter?
get() {
try {
method.kmFunction?.let {
// Shift needed to properly handle extension functions
val shift = if (it.receiverParameterType != null) 1 else 0
method.kmFunction?.let {
// Shift needed to properly handle extension functions
val shift = if (it.receiverParameterType != null) 1 else 0

// index - shift could be out of bounds if generated JVM parameter is fictive
// E.g., see how extension functions and coroutines are compiled
return it.valueParameters.getOrNull(index - shift)
}

return method.kmConstructor?.valueParameters?.get(index)
} catch (e: Exception) {
return null
// index - shift could be out of bounds if generated JVM parameter is fictive
// E.g., see how extension functions and coroutines are compiled
return it.valueParameters.getOrNull(index - shift)
}
return method.kmConstructor?.valueParameters?.getOrNull(index)
}

// If parameter is a receiver parameter, it doesn't have KmValueParameter instance, but we still can get KmType for it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ object Builders : JcFeature<Set<String>, BuildersResponse> {
}

fun syncQuery(classpath: JcClasspath, req: Set<String>): Sequence<BuildersResponse> {
val locationIds = classpath.registeredLocations.map { it.id }
val locationIds = classpath.registeredLocationIds
val persistence = classpath.db.persistence
return sequence {
val result = persistence.read { context ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
sqlAction = { jooq ->
jooq.select(SYMBOLS.NAME, DSL.count(SYMBOLS.NAME)).from(CLASSES)
.join(SYMBOLS).on(SYMBOLS.ID.eq(CLASSES.NAME))
.where(CLASSES.LOCATION_ID.`in`(registeredLocations.map { it.id }))
.where(CLASSES.LOCATION_ID.`in`(registeredLocationIds))

Check warning on line 42 in jacodb-core/src/main/kotlin/org/jacodb/impl/features/Diagnostics.kt

View check run for this annotation

Codecov / codecov/patch

jacodb-core/src/main/kotlin/org/jacodb/impl/features/Diagnostics.kt#L42

Added line #L42 was not covered by tests
.groupBy(SYMBOLS.NAME)
.having(DSL.count(SYMBOLS.NAME).greaterThan(1))
.fetch()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ suspend fun JcClasspath.hierarchyExt(): HierarchyExtension {
fun JcClasspath.asyncHierarchyExt(): Future<HierarchyExtension> = GlobalScope.future { hierarchyExt() }

internal fun JcClasspath.allClassesExceptObject(context: JCDBContext, direct: Boolean): Sequence<ClassSource> {
val locationIds = registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = registeredLocationIds
return context.execute(
sqlAction = { jooq ->
if (direct) {
Expand Down Expand Up @@ -162,7 +162,7 @@ private class HierarchyExtensionERS(cp: JcClasspath) : HierarchyExtensionBase(cp
if (name == JAVA_OBJECT) {
cp.allClassesExceptObject(context, !entireHierarchy)
} else {
val locationIds = cp.registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = cp.registeredLocationIds
val nameId = name.asSymbolId(persistence.symbolInterner)
if (entireHierarchy) {
entireHierarchy(txn, nameId, mutableSetOf())
Expand Down Expand Up @@ -251,7 +251,7 @@ private fun JcClasspath.subClasses(name: String, entireHierarchy: Boolean): Sequ
if (name == JAVA_OBJECT) {
allClassesExceptObject(context, !entireHierarchy)
} else {
val locationIds = registeredLocations.joinToString(", ") { it.id.toString() }
val locationIds = registeredLocationIds.joinToString(", ") { it.toString() }
val dslContext = context.dslContext
BatchedSequence(defaultBatchSize) { offset, batchSize ->
val query = when {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ object InMemoryHierarchy : JcFeature<InMemoryHierarchyReq, ClassSource> {
}
}

val locationIds = classpath.registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = classpath.registeredLocationIds
val classSymbolId = persistence.findSymbolId(req.name)

val allSubclasses = hashSetOf<Long>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ object Usages : JcFeature<UsageFeatureRequest, UsageFeatureResponse> {
}

fun syncQuery(classpath: JcClasspath, req: UsageFeatureRequest): Sequence<UsageFeatureResponse> {
val locationIds = classpath.registeredLocations.mapTo(mutableSetOf()) { it.id }
val locationIds = classpath.registeredLocationIds
val persistence = classpath.db.persistence
val symbolInterner = persistence.symbolInterner
val name = req.methodName ?: req.field ?: throw IllegalArgumentException("Callee name should be specified")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ class SQLitePersistenceImpl(

private val JcClasspath.clause: Condition
get() {
val ids = registeredLocations.map { it.id }
return CLASSES.LOCATION_ID.`in`(ids)
return CLASSES.LOCATION_ID.`in`(registeredLocationIds)
}

private fun JcDatabase.classSources(clause: Condition, single: Boolean = false): List<ClassSource> = read { context ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class ErsPersistenceImpl(
}

private fun findClassSourcesImpl(context: JCDBContext, cp: JcClasspath, fullName: String): Sequence<ClassSource> {
val ids = cp.registeredLocations.mapTo(hashSetOf()) { it.id }
val ids = cp.registeredLocationIds
return context.txn.find("Class", "nameId", findSymbolId(fullName).compressed)
.asSequence().filter { it.getCompressed<Long>("locationId") in ids }
.map { it.toClassSource(cp.db, fullName) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import org.junit.jupiter.api.Test

class ClassesTest : DatabaseEnvTest() {

companion object : WithGlobalDB()
companion object : WithGlobalRAMDB()

override val cp: JcClasspath = runBlocking { db.classpath(allClasspath) }

Expand Down
23 changes: 11 additions & 12 deletions jacodb-core/src/test/kotlin/org/jacodb/testing/cfg/IRTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@

package org.jacodb.testing.cfg

import org.jacodb.api.common.cfg.CommonAssignInst
import org.jacodb.api.common.cfg.CommonCallInst
import org.jacodb.api.common.cfg.CommonExpr
import org.jacodb.api.common.cfg.CommonGotoInst
import org.jacodb.api.common.cfg.CommonIfInst
import org.jacodb.api.common.cfg.CommonInst
import org.jacodb.api.common.cfg.CommonReturnInst
import org.jacodb.api.jvm.JavaVersion
import org.jacodb.api.jvm.JcClassType
import org.jacodb.api.jvm.JcMethod
Expand Down Expand Up @@ -61,10 +54,12 @@ import org.jacodb.impl.cfg.Simplifier
import org.jacodb.impl.cfg.util.ExprMapper
import org.jacodb.impl.features.classpaths.ClasspathCache
import org.jacodb.impl.features.classpaths.StringConcatSimplifier
import org.jacodb.impl.features.classpaths.UnknownClasses
import org.jacodb.impl.fs.JarLocation
import org.jacodb.testing.WithDB
import org.jacodb.testing.WithRAMDB
import org.jacodb.testing.asmLib
import org.jacodb.testing.commonsCompressLib
import org.jacodb.testing.guavaLib
import org.jacodb.testing.jgitLib
import org.jacodb.testing.kotlinStdLib
Expand All @@ -73,7 +68,6 @@ import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Disabled
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertDoesNotThrow
import java.io.File
Expand All @@ -95,8 +89,8 @@ class OverridesResolver(
return methods.firstOrNull { typedMethod ->
val jcMethod = typedMethod.method
jcMethod.name == name &&
jcMethod.returnType.typeName == returnType.typeName &&
jcMethod.parameters.map { param -> param.type.typeName } == argTypes.map { it.typeName }
jcMethod.returnType.typeName == returnType.typeName &&
jcMethod.parameters.map { param -> param.type.typeName } == argTypes.map { it.typeName }
} ?: error("Could not find a method with correct signature")
}

Expand Down Expand Up @@ -350,6 +344,11 @@ abstract class IRTest : BaseInstructionsTest() {
runAlongLib(jgitLib)
}

@Test
fun `get ir of commons compress`() {
runAlongLib(commonsCompressLib)
}

@Test
fun `get ir of asm`() {
runAlongLib(asmLib, muteGraphChecker = true)
Expand Down Expand Up @@ -392,10 +391,10 @@ abstract class IRTest : BaseInstructionsTest() {

class IRSqlTest : IRTest() {

companion object : WithDB(StringConcatSimplifier)
companion object : WithDB(StringConcatSimplifier, UnknownClasses)
}

class IRRAMTest : IRTest() {

companion object : WithRAMDB(StringConcatSimplifier)
companion object : WithRAMDB(StringConcatSimplifier, UnknownClasses)
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ val jgitLib: File
}
}

val commonsCompressLib: File
get() {
val commonsCompressUrl = classpath.first { it.contains("commons-compress-") }
return File(commonsCompressUrl).also {
Assertions.assertTrue(it.isFile && it.exists())
}
}

val asmLib: File
get() {
val asmUrl = classpath.first { it.contains("${File.separator}asm${File.separator}") }
Expand Down
Loading