Skip to content

Commit

Permalink
More comprehensive validation of referenced types
Browse files Browse the repository at this point in the history
This adds more comprehensive validation of types referenced by application types.

Specifically, expediter will now report when types referenced via

* member declaration
* catching exceptions
* invokedynamic descriptors
* instanceof/cast
* arguments or return types of methods invoked on other types
* types of fields accessed from other types

are missing or have missing supertypes. It will also report issues with
array types.

The report is compacted to filter out some issues that can be considered
duplicates

* if application type A references application type B with missing
supertype C, issue with A will not be reported
* if type A extends a missing type B and also references it (e.g. via a
super constructor), the generic "A reference missing type B" will not be
reported
* if type A refers to a type B whose supertype C is missing, but A also
refers to C directly, only the latter will be reported
  • Loading branch information
ogolberg authored Dec 16, 2023
1 parent 33f6e0a commit d2c9edb
Show file tree
Hide file tree
Showing 31 changed files with 445 additions and 146 deletions.
115 changes: 80 additions & 35 deletions core/src/main/kotlin/com/toasttab/expediter/Expediter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,23 @@

package com.toasttab.expediter

import com.toasttab.expediter.access.AccessCheck
import com.toasttab.expediter.ignore.Ignore
import com.toasttab.expediter.issue.Issue
import com.toasttab.expediter.provider.ApplicationTypesProvider
import com.toasttab.expediter.provider.PlatformTypeProvider
import com.toasttab.expediter.types.ApplicationType
import com.toasttab.expediter.types.InspectedTypes
import com.toasttab.expediter.types.MemberAccess
import com.toasttab.expediter.types.MemberType
import com.toasttab.expediter.types.MethodAccessType
import com.toasttab.expediter.types.OptionalResolvedTypeHierarchy
import com.toasttab.expediter.types.PlatformTypeProvider
import com.toasttab.expediter.types.PlatformType
import com.toasttab.expediter.types.ResolvedTypeHierarchy
import com.toasttab.expediter.types.Type
import com.toasttab.expediter.types.members
import protokt.v1.toasttab.expediter.v1.AccessDeclaration
import protokt.v1.toasttab.expediter.v1.MemberDescriptor
import protokt.v1.toasttab.expediter.v1.TypeDescriptor
import protokt.v1.toasttab.expediter.v1.TypeExtensibility
import protokt.v1.toasttab.expediter.v1.TypeFlavor

Expand All @@ -42,21 +45,25 @@ class Expediter(
}

private fun findIssues(appType: ApplicationType): Collection<Issue> {
val hierarchy = inspectedTypes.resolveHierarchy(appType.type)
val hierarchy = inspectedTypes.resolveHierarchy(appType)
val issues = mutableListOf<Issue>()

val missingApplicationSupertypes = hashSetOf<String>()

when (hierarchy) {
is ResolvedTypeHierarchy.IncompleteTypeHierarchy -> {
hierarchy.missingType.mapTo(missingApplicationSupertypes) { it.name }

issues.add(
Issue.MissingApplicationSuperType(
appType.name,
hierarchy.missingType.map { it.name }.toSet()
missingApplicationSupertypes
)
)
}

is ResolvedTypeHierarchy.CompleteTypeHierarchy -> {
val finalSupertypes = hierarchy.superTypes.filter { it.extensibility == TypeExtensibility.FINAL }
val finalSupertypes = hierarchy.superTypes.filter { it.descriptor.extensibility == TypeExtensibility.FINAL }
.toList()
if (finalSupertypes.isNotEmpty()) {
issues.add(
Expand All @@ -69,60 +76,98 @@ class Expediter(
}
}

val missingTypes = hashSetOf<String>()

for (refType in appType.referencedTypes) {
val chain = inspectedTypes.resolveHierarchy(refType)

if (chain is OptionalResolvedTypeHierarchy.NoType) {
missingTypes.add(refType)
} else if (chain is ResolvedTypeHierarchy.IncompleteTypeHierarchy && chain.type is PlatformType) {
issues.add(Issue.MissingSuperType(appType.name, refType, chain.missingType.map { it.name }.toSet()))
}
}

issues.addAll(missingTypes.map { Issue.MissingType(appType.name, it) })

issues.addAll(
appType.refs.mapNotNull { access ->
appType.memberAccess.mapNotNull { access ->
findIssue(appType, hierarchy, access, inspectedTypes.resolveHierarchy(access.targetType))
}
)

return issues
return issues.filter {
when (it) {
// if application type A extends a missing type B and refers to it otherwise
// (which is typically via the super constructor), only report the missing supertype issue
is Issue.MissingType -> !missingApplicationSupertypes.contains(it.target)
// if application type A refers to a type B with a missing supertype C
// and also refers to C directly, only report the latter
is Issue.MissingSuperType -> !missingTypes.containsAll(it.missing)
else -> true
}
}
}

fun findIssues(): Set<Issue> {
return (
inspectedTypes.classes.flatMap { appType ->
findIssues(appType)
} + inspectedTypes.duplicateTypes
).filter { !ignore.ignore(it) }.toSet()
).filter { !ignore.ignore(it) }
.toSet()
}
}

fun <M : MemberType> findIssue(type: ApplicationType, hierarchy: ResolvedTypeHierarchy, access: MemberAccess<M>, chain: OptionalResolvedTypeHierarchy): Issue? {
return when (chain) {
is ResolvedTypeHierarchy.IncompleteTypeHierarchy -> Issue.MissingSuperType(
type.name,
access.targetType,
chain.missingType.map { it.name }.toSet()
)
fun <M : MemberType> findIssue(
type: ApplicationType,
hierarchy: ResolvedTypeHierarchy,
access: MemberAccess<M>,
chain: OptionalResolvedTypeHierarchy
): Issue? {
return when (chain) {
is OptionalResolvedTypeHierarchy.NoType -> Issue.MissingType(type.name, access.targetType)

is OptionalResolvedTypeHierarchy.NoType -> Issue.MissingType(type.name, access.targetType)
is ResolvedTypeHierarchy.CompleteTypeHierarchy -> {
val member = chain.resolveMember(access)

if (member == null) {
Issue.MissingMember(type.name, access)
} else {
val resolvedAccess = access.withDeclaringType(member.declaringType.name)

if (member.member.declaration == AccessDeclaration.STATIC && !access.accessType.isStatic()) {
Issue.AccessStaticMemberNonStatically(type.name, resolvedAccess)
} else if (member.member.declaration == AccessDeclaration.INSTANCE && access.accessType.isStatic()) {
Issue.AccessInstanceMemberStatically(type.name, resolvedAccess)
} else if (!AccessCheck.allowedAccess(hierarchy, chain, member.member, member.declaringType)) {
Issue.AccessInaccessibleMember(type.name, resolvedAccess)
is ResolvedTypeHierarchy.IncompleteTypeHierarchy -> {
if (chain.type is PlatformType) {
Issue.MissingSuperType(
type.name,
access.targetType,
chain.missingType.map { it.name }.toSet()
)
} else {
// missing supertypes of application types will be reported separately
null
}
}

is ResolvedTypeHierarchy.CompleteTypeHierarchy -> {
val member = chain.resolveMember(access)

if (member == null) {
Issue.MissingMember(type.name, access)
} else {
val resolvedAccess = access.withDeclaringType(member.declaringType.name)

if (member.member.declaration == AccessDeclaration.STATIC && !access.accessType.isStatic()) {
Issue.AccessStaticMemberNonStatically(type.name, resolvedAccess)
} else if (member.member.declaration == AccessDeclaration.INSTANCE && access.accessType.isStatic()) {
Issue.AccessInstanceMemberStatically(type.name, resolvedAccess)
} else if (!AccessCheck.allowedAccess(hierarchy, chain, member.member, member.declaringType)) {
Issue.AccessInaccessibleMember(type.name, resolvedAccess)
} else {
null
}
}
}
}
}
}
private class MemberWithDeclaringType(
val member: MemberDescriptor,
val declaringType: TypeDescriptor
val declaringType: Type
)

private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.filterToAccessType(access: MemberAccess<M>): Sequence<TypeDescriptor> {
private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.filterToAccessType(access: MemberAccess<M>): Sequence<Type> {
return if (access !is MemberAccess.MethodAccess ||
access.accessType == MethodAccessType.VIRTUAL ||
access.accessType == MethodAccessType.STATIC ||
Expand All @@ -133,7 +178,7 @@ private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.filterT
allTypes
} else if (access.accessType == MethodAccessType.INTERFACE) {
// methods invoked via invokeinterface must be declared on an interface
allTypes.filter { it.flavor != TypeFlavor.CLASS }
allTypes.filter { it.descriptor.flavor != TypeFlavor.CLASS }
} else {
// constructors must always be declared by the type being constructed
sequenceOf(type)
Expand All @@ -142,7 +187,7 @@ private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.filterT

private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.resolveMember(access: MemberAccess<M>): MemberWithDeclaringType? {
for (cls in filterToAccessType(access)) {
for (m in cls.members) {
for (m in cls.descriptor.members) {
if (access.ref.same(m.ref)) {
return MemberWithDeclaringType(m, cls)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.toasttab.expediter
package com.toasttab.expediter.access

import com.toasttab.expediter.types.ResolvedTypeHierarchy
import com.toasttab.expediter.types.Type
import protokt.v1.toasttab.expediter.v1.AccessDeclaration
import protokt.v1.toasttab.expediter.v1.AccessProtection
import protokt.v1.toasttab.expediter.v1.MemberDescriptor
Expand All @@ -25,10 +26,10 @@ object AccessCheck {
caller: ResolvedTypeHierarchy,
refType: ResolvedTypeHierarchy.CompleteTypeHierarchy,
member: MemberDescriptor,
declaringType: TypeDescriptor
declaringType: Type
): Boolean {
// reference type must be accessible from the caller
if (!isClassAccessible(caller, refType.type)) {
if (!isClassAccessible(caller, refType.type.descriptor)) {
return false
}

Expand All @@ -40,7 +41,7 @@ object AccessCheck {
// private members are accessible to classes in the same nest as the declaring type
AccessProtection.PRIVATE -> sameNest(caller.name, declaringType.name)
// protected logic is more complicated, see below
AccessProtection.PROTECTED -> isProtectedAccessAllowed(caller, refType, member, declaringType)
AccessProtection.PROTECTED -> isProtectedAccessAllowed(caller, refType, member, declaringType.descriptor)
}
}

Expand All @@ -66,8 +67,8 @@ object AccessCheck {
// the caller, the reference type, and the declaring type must be part of the same hierarchy,
// i.e. the caller must be a subtype or a supertype of the reference type
member.declaration != AccessDeclaration.INSTANCE ||
refType.isSubtypeOf(caller.type) ||
caller.isSubtypeOf(refType.type)
refType.isSubtypeOf(caller.type.descriptor) ||
caller.isSubtypeOf(refType.type.descriptor)
} else {
false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* limitations under the License.
*/

package com.toasttab.expediter
package com.toasttab.expediter.parser

import org.objectweb.asm.Opcodes
import protokt.v1.toasttab.expediter.v1.AccessDeclaration
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package com.toasttab.expediter.parser

class SignatureParser private constructor(private val signature: String) {
private var idx = 0

private fun nextType(internal: Boolean): TypeSignature {
var primitive = true
var dimensions = 0
var c = signature[idx]
while (c == '[') {
dimensions++
c = signature[++idx]
}

val name = if (dimensions > 0 || !internal) {
if (c == 'L') {
primitive = false
val next = signature.indexOf(';', startIndex = idx + 1)
if (next < 0) {
error("error parsing type from $signature, cannot find ';' after index $idx")
}
val start = idx + 1
idx = next + 1
signature.substring(start, next)
} else {
signature.substring(idx, ++idx)
}
} else {
signature.substring(idx)
}

return TypeSignature(name, dimensions, primitive)
}

private fun parseMethod(): MethodSignature {
if (signature[idx++] != '(') {
error("error parsing $signature, expected to start with '('")
}

val args = mutableListOf<TypeSignature>()

while (signature[idx] != ')') {
args.add(nextType(false))
}

idx++

return MethodSignature(nextType(false), args)
}

companion object {
/**
* Parses a standard method descriptor, e.g.
*
* (Ljava/lang/Object;)V for void fun(Object)
*/
fun parseMethod(method: String) = SignatureParser(method).parseMethod()

/**
* Parses a standard type descriptor, as it appears in a method descriptor, e.g.
*
* L/java/lang/Object; for Object
* [L/java/lang/Object; for Object[]
*/
fun parseType(type: String) = SignatureParser(type).nextType(false)

/**
* Parses a internal type descriptor, as reported by ASM for method owners, instanceof, etc; e.g.
*
* [L/java/lang/Object; for Object[]
*
* but just
*
* java/lang/Object for Object
*/
fun parseInternalType(type: String) = SignatureParser(type).nextType(true)
}
}

class MethodSignature(
val returnType: TypeSignature,
val argumentTypes: List<TypeSignature>
) {
fun referencedTypes() = (argumentTypes + returnType).filter { !it.primitive }.map { it.scalarName }
}

class TypeSignature(
val scalarName: String,
val dimensions: Int,
val primitive: Boolean
) {
fun isArray() = dimensions > 0
fun referencedTypes() = if (primitive) emptySet() else setOf(scalarName)

fun scalarSignature() = TypeSignature(scalarName, 0, primitive)
val name get() = scalarName + "[]".repeat(dimensions)
}
Loading

0 comments on commit d2c9edb

Please sign in to comment.