Detect application classes with missing or final supertypes and acces…

…s-check protected members (#13)

Resolves the type hierarchy for all application types, so that we can detect missing and final supertypes as well as do access checks for protected members. Resolves the following issues:

* #9
* #10
* #12

README.md

@@ -24,9 +24,10 @@ This tool can detect _some_ binary incompatibilities at build time. Specifically
 * Missing classes
 * Duplicate classes
 * Classes with missing superclasses and interfaces
+* Classes extending final classes
 * Missing methods and fields
 * Static methods and fields accessed non-statically and vice-versa
-* Inaccessible methods and fields (partially)
+* Inaccessible methods and fields
 
 ## Application vs platform classes
 

animal-sniffer-format/src/main/kotlin/com/toasttab/expediter/sniffer/AnimalSnifferParser.kt

@@ -20,6 +20,7 @@ import com.toasttab.expediter.types.AccessProtection
 import com.toasttab.expediter.types.MemberDescriptor
 import com.toasttab.expediter.types.MemberSymbolicReference
 import com.toasttab.expediter.types.TypeDescriptor
+import com.toasttab.expediter.types.TypeExtensibility
 import com.toasttab.expediter.types.TypeFlavor
 import java.io.InputStream
 import java.util.zip.GZIPInputStream
@@ -61,7 +62,8 @@ object AnimalSnifferParser {
                 it.superInterfaces,
                 members,
                 AccessProtection.UNKNOWN,
-                TypeFlavor.UNKNOWN
+                TypeFlavor.UNKNOWN,
+                TypeExtensibility.UNKNOWN
             )
         }
     }

core/src/main/kotlin/com/toasttab/expediter/AccessCheck.kt

@@ -1,21 +1,26 @@
 package com.toasttab.expediter
 
 import com.toasttab.expediter.types.AccessProtection
+import com.toasttab.expediter.types.ApplicationTypeWithResolvedHierarchy
 import com.toasttab.expediter.types.MemberDescriptor
 import com.toasttab.expediter.types.MemberType
+import com.toasttab.expediter.types.ResolvedTypeHierarchy
 import com.toasttab.expediter.types.TypeDescriptor
 
 object AccessCheck {
-    fun <M : MemberType> allowedAccess(caller: TypeDescriptor, target: TypeDescriptor, member: MemberDescriptor<M>): Boolean {
+    fun <M : MemberType> allowedAccess(caller: ApplicationTypeWithResolvedHierarchy, target: TypeDescriptor, member: MemberDescriptor<M>): Boolean {
         return if (member.protection == AccessProtection.PRIVATE) {
-            return caller.sameClassAs(target)
+            caller.name == target.name
         } else if (member.protection == AccessProtection.PACKAGE_PRIVATE || target.protection == AccessProtection.PACKAGE_PRIVATE) {
-            return caller.samePackageAs(target)
+            samePackage(caller.name, target.name)
+        } else if (member.protection == AccessProtection.PROTECTED) {
+            samePackage(caller.name, target.name) ||
+                // if we can't resolve all supertypes, we can't say for sure that access is not allowed
+                caller.hierarchy !is ResolvedTypeHierarchy.CompleteTypeHierarchy ||
+                caller.hierarchy.allTypes.any { it.name == target.name }
         } else {
             true
-        } // TODO: add other checks
+        }
     }
-
-    private fun TypeDescriptor.sameClassAs(other: TypeDescriptor) = name == other.name
-    private fun TypeDescriptor.samePackageAs(other: TypeDescriptor) = name.substringBeforeLast('/') == other.name.substringBeforeLast('/')
+    private fun samePackage(a: String, b: String) = a.substringBeforeLast('/') == b.substringBeforeLast('/')
 }

core/src/main/kotlin/com/toasttab/expediter/AttributeParser.kt

@@ -17,10 +17,12 @@ package com.toasttab.expediter
 
 import com.toasttab.expediter.types.AccessDeclaration
 import com.toasttab.expediter.types.AccessProtection
+import com.toasttab.expediter.types.TypeExtensibility
+import com.toasttab.expediter.types.TypeFlavor
 import org.objectweb.asm.Opcodes
 
 object AttributeParser {
-    fun isInterface(access: Int) = (access and Opcodes.ACC_INTERFACE) != 0
+    fun flavor(access: Int) = if (access and Opcodes.ACC_INTERFACE != 0) TypeFlavor.INTERFACE else TypeFlavor.CLASS
 
     fun protection(access: Int) =
         if (access and Opcodes.ACC_PRIVATE != 0) {
@@ -39,4 +41,11 @@ object AttributeParser {
         } else {
             AccessDeclaration.INSTANCE
         }
+
+    fun extensibility(access: Int) =
+        if (access and Opcodes.ACC_FINAL != 0) {
+            TypeExtensibility.FINAL
+        } else {
+            TypeExtensibility.NOT_FINAL
+        }
 }

core/src/main/kotlin/com/toasttab/expediter/Expediter.kt

@@ -18,50 +18,98 @@ package com.toasttab.expediter
 import com.toasttab.expediter.ignore.Ignore
 import com.toasttab.expediter.issue.Issue
 import com.toasttab.expediter.types.AccessDeclaration
+import com.toasttab.expediter.types.ApplicationTypeWithResolvedHierarchy
 import com.toasttab.expediter.types.InspectedTypes
 import com.toasttab.expediter.types.MemberAccess
 import com.toasttab.expediter.types.MemberDescriptor
-import com.toasttab.expediter.types.MemberSymbolicReference
 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.ResolvedTypeHierarchy
 import com.toasttab.expediter.types.TypeDescriptor
-import com.toasttab.expediter.types.TypeHierarchy
+import com.toasttab.expediter.types.TypeExtensibility
+import com.toasttab.expediter.types.TypeFlavor
 
 class Expediter(
     private val ignore: Ignore,
     private val applicationTypesProvider: ApplicationTypesProvider,
     private val platformTypeProvider: PlatformTypeProvider
 ) {
-    fun findIssues(): Set<Issue> {
-        val inspectedTypes = InspectedTypes(applicationTypesProvider.types(), platformTypeProvider)
+    private val inspectedTypes: InspectedTypes by lazy {
+        InspectedTypes(applicationTypesProvider.types(), platformTypeProvider)
+    }
 
-        return (
-            inspectedTypes.classes.flatMap { cls ->
-                cls.refs.mapNotNull { access ->
-                    if (!ignore.ignore(cls.type.name, access.targetType, access.ref)) {
-                        findIssue(cls.type, access, inspectedTypes.hierarchy(access))
-                    } else {
-                        null
-                    }
+    private fun duplicates() = inspectedTypes.duplicateTypes.filter {
+        !ignore.ignore(null, it.target, null)
+    }
+
+    private fun findIssues(appTypeWithHierarchy: ApplicationTypeWithResolvedHierarchy): Collection<Issue> {
+        val issues = mutableListOf<Issue>()
+
+        when (appTypeWithHierarchy.hierarchy) {
+            is ResolvedTypeHierarchy.IncompleteTypeHierarchy -> {
+                issues.add(
+                    Issue.MissingApplicationSuperType(
+                        appTypeWithHierarchy.name,
+                        appTypeWithHierarchy.hierarchy.missingType.map { it.name }.toSet()
+                    )
+                )
+            }
+
+            is ResolvedTypeHierarchy.CompleteTypeHierarchy -> {
+                val finalSupertypes =
+                    appTypeWithHierarchy.hierarchy.superTypes.filter { it.extensibility == TypeExtensibility.FINAL }
+                        .toList()
+                if (finalSupertypes.isNotEmpty()) {
+                    issues.add(
+                        Issue.FinalApplicationSuperType(
+                            appTypeWithHierarchy.name,
+                            finalSupertypes.map { it.name }.toSet()
+                        )
+                    )
                 }
-            } + inspectedTypes.duplicateTypes.filter {
-                !ignore.ignore(null, it.target, null)
             }
+        }
+
+        issues.addAll(
+            appTypeWithHierarchy.appType.refs.mapNotNull { access ->
+                if (!ignore.ignore(appTypeWithHierarchy.name, access.targetType, access.ref)) {
+                    findIssue(appTypeWithHierarchy, access, inspectedTypes.resolveHierarchy(access.targetType))
+                } else {
+                    null
+                }
+            }
+        )
+
+        return issues
+    }
+
+    fun findIssues(): Set<Issue> {
+        return (
+            inspectedTypes.classes.flatMap { appType ->
+                findIssues(
+                    ApplicationTypeWithResolvedHierarchy(
+                        appType,
+                        inspectedTypes.resolveHierarchy(appType.type)
+                    )
+                )
+            } + duplicates()
             ).toSet()
     }
 }
 
-fun <M : MemberType> findIssue(type: TypeDescriptor, access: MemberAccess<M>, chain: TypeHierarchy): Issue? {
+fun <M : MemberType> findIssue(type: ApplicationTypeWithResolvedHierarchy, access: MemberAccess<M>, chain: OptionalResolvedTypeHierarchy): Issue? {
     return when (chain) {
-        is TypeHierarchy.IncompleteTypeHierarchy -> Issue.MissingSuperType(
+        is ResolvedTypeHierarchy.IncompleteTypeHierarchy -> Issue.MissingSuperType(
             type.name,
             access.targetType,
             chain.missingType.map { it.name }.toSet()
         )
 
-        is TypeHierarchy.NoType -> Issue.MissingType(type.name, access.targetType)
-        is TypeHierarchy.CompleteTypeHierarchy -> {
-            val member = chain.findMember(access.ref)
+        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)
@@ -86,10 +134,28 @@ private class MemberWithDeclaringType<M : MemberType> (
     val declaringType: TypeDescriptor
 )
 
-private fun <M : MemberType> TypeHierarchy.CompleteTypeHierarchy.findMember(ref: MemberSymbolicReference<M>): MemberWithDeclaringType<M>? {
-    for (cls in classes) {
+private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.filterToAccessType(access: MemberAccess<M>): Sequence<TypeDescriptor> {
+    return if (access !is MemberAccess.MethodAccess ||
+        access.accessType == MethodAccessType.VIRTUAL ||
+        access.accessType == MethodAccessType.STATIC ||
+        (access.accessType == MethodAccessType.SPECIAL && !access.ref.isConstructor())
+    ) {
+        // fields and methods, except for constructors and methods invoked via invokeinterface
+        // can be declared on any type in the hierarchy
+        allTypes
+    } else if (access.accessType == MethodAccessType.INTERFACE) {
+        // methods invoked via invokeinterface must be declared on an interface
+        allTypes.filter { it.flavor != TypeFlavor.CLASS }
+    } else {
+        // constructors must always be declared by the type being constructed
+        sequenceOf(type)
+    }
+}
+
+private fun <M : MemberType> ResolvedTypeHierarchy.CompleteTypeHierarchy.resolveMember(access: MemberAccess<M>): MemberWithDeclaringType<M>? {
+    for (cls in filterToAccessType(access)) {
         for (m in cls.members) {
-            if (ref == m.ref) {
+            if (access.ref == m.ref) {
                 return MemberWithDeclaringType(m as MemberDescriptor<M>, cls)
             }
         }

core/src/main/kotlin/com/toasttab/expediter/TypeParsers.kt

@@ -22,6 +22,7 @@ import com.toasttab.expediter.types.MemberDescriptor
 import com.toasttab.expediter.types.MemberSymbolicReference
 import com.toasttab.expediter.types.MethodAccessType
 import com.toasttab.expediter.types.TypeDescriptor
+import com.toasttab.expediter.types.TypeExtensibility
 import com.toasttab.expediter.types.TypeFlavor
 import org.objectweb.asm.ClassReader
 import org.objectweb.asm.ClassReader.SKIP_DEBUG
@@ -109,8 +110,9 @@ private class TypeDescriptorParser : ClassVisitor(ASM9) {
     private val members: MutableList<MemberDescriptor<*>> = mutableListOf()
     private var access: Int = 0
     private var typeFlavor: TypeFlavor = TypeFlavor.UNKNOWN
+    private var typeExtensibility: TypeExtensibility = TypeExtensibility.UNKNOWN
 
-    fun get() = TypeDescriptor(name!!, superName, interfaces, members, AttributeParser.protection(access), typeFlavor)
+    fun get() = TypeDescriptor(name!!, superName, interfaces, members, AttributeParser.protection(access), typeFlavor, typeExtensibility)
 
     override fun visit(
         version: Int,
@@ -123,7 +125,8 @@ private class TypeDescriptorParser : ClassVisitor(ASM9) {
         this.name = name
         this.superName = superName
         this.access = access
-        this.typeFlavor = if (AttributeParser.isInterface(access)) TypeFlavor.INTERFACE else TypeFlavor.CLASS
+        this.typeFlavor = AttributeParser.flavor(access)
+        this.typeExtensibility = AttributeParser.extensibility(access)
         this.interfaces.addAll(interfaces)
     }
 

core/src/main/kotlin/com/toasttab/expediter/ignore/Ignore.kt

@@ -19,56 +19,56 @@ import com.toasttab.expediter.types.MemberSymbolicReference
 import java.io.Serializable
 
 interface Ignore : Serializable {
-    fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?): Boolean
+    fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?): Boolean
 
     companion object {
         val NOTHING: Ignore = object : Ignore {
-            override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = false
+            override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = false
         }
     }
 
     class Not(
         private val ignore: Ignore
     ) : Ignore {
-        override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = !ignore.ignore(caller, type, ref)
+        override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = !ignore.ignore(caller, type, ref)
     }
 
     class And(
         private vararg val ignores: Ignore
     ) : Ignore {
-        override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = ignores.all { it.ignore(caller, type, ref) }
+        override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = ignores.all { it.ignore(caller, type, ref) }
     }
 
     class Or(
         private vararg val ignores: Ignore
     ) : Ignore {
-        override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = ignores.any { it.ignore(caller, type, ref) }
+        override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = ignores.any { it.ignore(caller, type, ref) }
     }
 
     object IsConstructor : Ignore {
-        override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = ref is MemberSymbolicReference.MethodSymbolicReference && ref.isConstructor()
+        override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = ref is MemberSymbolicReference.MethodSymbolicReference && ref.isConstructor()
     }
 
     object Caller {
         class StartsWith(
             private vararg val partial: String
         ) : Ignore {
-            override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = caller != null && partial.any { caller.startsWith(it) }
+            override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = caller != null && partial.any { caller.startsWith(it) }
         }
     }
 
     object Type {
         class StartsWith(
             private vararg val partial: String
         ) : Ignore {
-            override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = partial.any { type.startsWith(it) }
+            override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = partial.any { type != null && type.startsWith(it) }
         }
     }
 
     class Signature(
         private val signature: String
     ) : Ignore {
-        override fun ignore(caller: String?, type: String, ref: MemberSymbolicReference<*>?) = ref?.signature == signature
+        override fun ignore(caller: String?, type: String?, ref: MemberSymbolicReference<*>?) = ref?.signature == signature
 
         companion object {
             val IS_BLANK = Signature("()V")