Skip to content

Commit

Permalink
Ignore Throwable and String types in data classes
Browse files Browse the repository at this point in the history 
Throwable and String classes have non final fields in their hierarchy.
Data classes with these types are ignored from immutability
 check.
  • Loading branch information
@kozaxinan
kozaxinan committed Sep 26, 2024
1 parent f12febf commit 19044ab
Show file tree
Hide file tree
Showing 5 changed files with 316 additions and 88 deletions.
16 changes: 10 additions & 6 deletions checks/src/main/java/com/kozaxinan/android/checks/ImmutableDataClassDetector.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ import com.android.tools.lint.detector.api.Issue
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.Scope
import com.android.tools.lint.detector.api.Severity
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiClassType
import com.intellij.psi.PsiField
import com.intellij.psi.PsiModifier
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UField
import org.jetbrains.uast.getContainingUClass
import org.jetbrains.uast.toUElementOfType

/**
Expand All @@ -40,8 +39,16 @@ internal class ImmutableDataClassDetector : Detector(), UastScanner {
if (containsEqualHashCode) {
val fields: List<UField> = node
.allFields
.flatMap {
if (it.type is PsiClassType) {
findAllInnerFields(it.type as PsiClassType) + it
} else {
listOf(it)
}
}
.filterNot { it.name.contains("$") }
.mapNotNull(PsiField::toUElementOfType)
.mapNotNull<PsiField, UField>(PsiField::toUElementOfType)
.filterNot(UField::hasSuperClassWithNonFinalValue)

validateDataClassFields(node, fields, context.evaluator)
}
Expand All @@ -53,9 +60,6 @@ internal class ImmutableDataClassDetector : Detector(), UastScanner {
evaluator: JavaEvaluator,
) {
val problematicFields = fields
.filterNot { field: UField ->
hasThrowableSuperClass(field.getContainingUClass())
}
.filter { field: UField ->
!field
.hasModifierProperty(PsiModifier.FINAL) ||
Expand Down
96 changes: 92 additions & 4 deletions checks/src/main/java/com/kozaxinan/android/checks/KtDeclarationKt.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,32 @@ package com.kozaxinan.android.checks

import com.android.tools.lint.client.api.JavaEvaluator
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiClassType
import com.intellij.psi.PsiEnumConstant
import com.intellij.psi.PsiMember
import com.intellij.psi.PsiSubstitutor
import com.intellij.psi.PsiType
import com.intellij.psi.PsiWildcardType
import com.intellij.psi.impl.source.PsiClassReferenceType
import org.jetbrains.kotlin.asJava.LightClassGenerationSupport
import org.jetbrains.kotlin.asJava.elements.KtLightMember
import org.jetbrains.kotlin.asJava.elements.KtLightModifierList
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.ValueDescriptor
import org.jetbrains.kotlin.js.descriptorUtils.getKotlinTypeFqName
import org.jetbrains.kotlin.js.descriptorUtils.nameIfStandardType
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtParameter
import org.jetbrains.kotlin.resolve.descriptorUtil.module
import org.jetbrains.kotlin.types.KotlinType
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UField
import org.jetbrains.uast.UMethod
import org.jetbrains.uast.getContainingUClass
import org.jetbrains.uast.toUElement
import org.jetbrains.uast.toUElementOfType

internal fun KtDeclaration.resolve(): DeclarationDescriptor? =
Expand Down Expand Up @@ -118,7 +129,7 @@ fun String?.matchesAnyOf(patterns: Sequence<Regex>): Boolean {
}

fun hasThrowableSuperClass(uClass: UClass?): Boolean {
var superClass: PsiClass? = uClass?.javaPsi?.superClass
var superClass: PsiClass? = uClass?.javaPsi
while (superClass != null) {
if (superClass.qualifiedName == "java.lang.Throwable" || superClass.qualifiedName == "kotlin.Throwable") {
return true
Expand All @@ -128,7 +139,84 @@ fun hasThrowableSuperClass(uClass: UClass?): Boolean {
return false
}

fun isStringClass(uClass: UClass): Boolean {
return uClass.qualifiedName == "java.lang.String" ||
uClass.qualifiedName == "kotlin.String"
fun hasStringSuperClass(uClass: UClass?): Boolean {
var superClass: PsiClass? = uClass?.javaPsi
while (superClass != null) {
if (superClass.qualifiedName == "java.lang.String" || superClass.qualifiedName == "kotlin.String") {
return true
}
superClass = superClass.superClass
}
return false
}

fun findAllInnerFields(
typeRef: PsiClassType,
visitedTypes: MutableSet<PsiClassType> = mutableSetOf(),
): Set<UField> {
val actualReturnType = findGenericClassType(typeRef)
val typeClass: UClass = actualReturnType
.resolve()
.toUElement() as? UClass
?: return emptySet()

if (visitedTypes.contains(actualReturnType)) {
return setOf()
}
visitedTypes.add(actualReturnType)

val innerFields: Set<UField> = typeClass
.fields
.filterNot(UField::hasSuperClassWithNonFinalValue)
.filterNot { it.isStatic && it !is PsiEnumConstant }
.toSet()

return innerFields +
innerFields
.asSequence()
.filterNot { it.isStatic }
.map { it.type }
.filterIsInstance<PsiClassType>()
.filterNot { visitedTypes.contains(it) }
.map { innerTypeRef -> findAllInnerFields(innerTypeRef, visitedTypes) }
.flatten()
.toSet()
}

fun UField.hasSuperClassWithNonFinalValue(): Boolean =
hasThrowableSuperClass(getContainingUClass()) || hasStringSuperClass(getContainingUClass())

fun findGenericClassType(returnType: PsiClassType): PsiClassType {
val substitutor: PsiSubstitutor = returnType
.resolveGenerics()
.substitutor
return if (substitutor == PsiSubstitutor.EMPTY) {
returnType
} else {
when (val psiType: PsiType? = substitutor.substitutionMap.values.first()) {
is PsiClassReferenceType -> findGenericClassType(psiType)
is PsiWildcardType -> {
when {
psiType.isSuper -> {
findGenericClassType(psiType.superBound as PsiClassType)
}

psiType.isExtends -> {
findGenericClassType(psiType.extendsBound as PsiClassType)
}

else -> {
returnType
}
}
}

else -> returnType
}
}
}

fun UMethod.isSuspend(): Boolean {
val modifiers = modifierList as? KtLightModifierList<*>
return modifiers?.kotlinOrigin?.hasModifier(KtTokens.SUSPEND_KEYWORD) ?: false
}
71 changes: 0 additions & 71 deletions checks/src/main/java/com/kozaxinan/android/checks/RetrofitReturnTypeDetector.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,76 +113,5 @@ internal abstract class RetrofitReturnTypeDetector : Detector(), UastScanner {
.intersect(listOfRetrofitAnnotations)
.isNotEmpty()
}

internal fun findAllInnerFields(
typeRef: PsiClassType,
visitedTypes: MutableSet<PsiClassType> = mutableSetOf(),
): Set<UField> {
val actualReturnType = findGenericClassType(typeRef)
val typeClass: UClass = actualReturnType
.resolve()
.toUElement() as? UClass
?: return emptySet()

if (hasThrowableSuperClass(typeClass) || isStringClass(typeClass)) {
return setOf()
}

if (visitedTypes.contains(actualReturnType)) {
return setOf()
}
visitedTypes.add(actualReturnType)

val innerFields: Set<UField> = typeClass
.fields
.filterNot { it.isStatic && it !is PsiEnumConstant }
.toSet()

return innerFields +
innerFields
.asSequence()
.filterNot { it.isStatic }
.map { it.type }
.filterIsInstance<PsiClassType>()
.filterNot { visitedTypes.contains(it) }
.map { innerTypeRef -> findAllInnerFields(innerTypeRef, visitedTypes) }
.flatten()
.toSet()
}

private fun findGenericClassType(returnType: PsiClassType): PsiClassType {
val substitutor: PsiSubstitutor = returnType
.resolveGenerics()
.substitutor
return if (substitutor == PsiSubstitutor.EMPTY) {
returnType
} else {
when (val psiType: PsiType? = substitutor.substitutionMap.values.first()) {
is PsiClassReferenceType -> findGenericClassType(psiType)
is PsiWildcardType -> {
when {
psiType.isSuper -> {
findGenericClassType(psiType.superBound as PsiClassType)
}

psiType.isExtends -> {
findGenericClassType(psiType.extendsBound as PsiClassType)
}

else -> {
returnType
}
}
}

else -> returnType
}
}
}

private fun UMethod.isSuspend(): Boolean {
val modifiers = modifierList as? KtLightModifierList<*>
return modifiers?.kotlinOrigin?.hasModifier(KtTokens.SUSPEND_KEYWORD) ?: false
}
}
}
36 changes: 29 additions & 7 deletions checks/src/test/java/com/kozaxinan/android/checks/ImmutableDataClassDetectorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
import com.kozaxinan.android.checks.ImmutableDataClassDetector.Companion.ISSUE_IMMUTABLE_DATA_CLASS_RULE
import org.junit.Test
import java.util.*

@Suppress("UnstableApiUsage")
internal class ImmutableDataClassDetectorTest : LintDetectorTest() {
Expand Down Expand Up @@ -50,17 +49,40 @@ internal class ImmutableDataClassDetectorTest : LintDetectorTest() {
"""
package foo
import java.lang.IllegalStateException
import java.lang.Exception
import java.lang.Throwable
data class DtoException(
val totalResults: Int,
val totalNewResults: Int,
private val name: java.lang.String,
val bool: Boolean,
val th: IllegalStateException,
): java.lang.Throwable(message = name)
val th: Throwable,
): Exception(message = name)
"""
).indented()
).indented(),
java(throwable),
java(exception),
)
.issues(ISSUE_IMMUTABLE_DATA_CLASS_RULE)
.run()
.expectClean()
}

@Test
fun `test data class with String`() {
lint()
.files(
kotlin(
"""
package foo
import java.lang.String
data class Dto(
val someString: String,
)
"""
).indented(),
java(string),
)
.issues(ISSUE_IMMUTABLE_DATA_CLASS_RULE)
.run()
Expand Down
Loading

0 comments on commit 19044ab

Please sign in to comment.