Skip to content

Commit bf55da2

Browse files
SerchinasticoSerchinastico
authored
No else branch in switch for enum/sealed classes (#10)
* Create a new detector that visits when/switch statements/expressions and checks if the evaluated expression is an enum/sealed class type. If that's the case then we verify if there is a default/else branch and forbid it. * Refactor with an inline function the way we create issues to make it more readable and less constant-based. * Refactor how to report issues in the context so that we have a shorter function that does some magic for you like getting the location of the warning automatically.
1 parent d90f194 commit bf55da2

File tree

7 files changed

+334
-45
lines changed

7 files changed

+334
-45
lines changed

‎rules/src/main/java/com/serchinastico/rules/Extensions.kt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,50 @@
11
package com.serchinastico.rules
22

3+
import com.android.tools.lint.detector.api.Issue
4+
import com.android.tools.lint.detector.api.JavaContext
5+
import com.android.tools.lint.detector.api.Location
6+
import com.android.tools.lint.detector.api.TextFormat
37
import com.intellij.lang.Language
48
import com.intellij.psi.PsiType
9+
import com.intellij.psi.impl.source.PsiClassReferenceType
10+
import com.intellij.psi.impl.source.PsiImmediateClassType
11+
import org.jetbrains.kotlin.asJava.classes.KtLightClass
12+
import org.jetbrains.kotlin.psi.KtClass
513
import org.jetbrains.uast.*
14+
import org.jetbrains.uast.java.JavaUDefaultCaseExpression
615
import org.jetbrains.uast.kotlin.KotlinUClass
716

817
val Any?.exhaustive get() = Unit
918

19+
fun JavaContext.report(issue: Issue) {
20+
report(issue, Location.create(file), issue.getBriefDescription(TextFormat.TEXT))
21+
}
22+
23+
val USwitchExpression.clauses: List<USwitchClauseExpression>
24+
get() = body.expressions.mapNotNull { it as? USwitchClauseExpression }
25+
26+
val USwitchClauseExpression.isElseBranch: Boolean
27+
get() = caseValues.isEmpty() || caseValues.any { it is JavaUDefaultCaseExpression }
28+
29+
val PsiType.isSealed: Boolean
30+
get() = when (this) {
31+
is PsiClassReferenceType -> {
32+
val resolvedType = resolve()
33+
when (resolvedType) {
34+
is KtLightClass -> (resolvedType.kotlinOrigin as? KtClass)?.isSealed() ?: false
35+
else -> false
36+
}
37+
}
38+
else -> false
39+
}
40+
41+
val PsiType.isEnum: Boolean
42+
get() = when (this) {
43+
is PsiImmediateClassType -> resolve()?.isEnum ?: false
44+
is PsiClassReferenceType -> resolve()?.isEnum ?: false
45+
else -> false
46+
}
47+
1048
val UField.isPrivate: Boolean
1149
get() {
1250
when {

‎rules/src/main/java/com/serchinastico/rules/IssueBuilder.kt‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package com.serchinastico.rules
2+
3+
import com.android.tools.lint.detector.api.*
4+
import java.util.*
5+
6+
inline fun <reified T : Detector> createIssue(
7+
id: String,
8+
scope: EnumSet<Scope>,
9+
description: String,
10+
explanation: String,
11+
category: Category,
12+
priority: Int = 5,
13+
severity: Severity = Severity.ERROR
14+
) = Issue.create(id, description, explanation, category, priority, severity, Implementation(T::class.java, scope))

‎rules/src/main/java/com/serchinastico/rules/NoDataFrameworksFromAndroidClassDetector.kt‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package com.serchinastico.rules
22

33
import com.android.tools.lint.client.api.UElementHandler
4-
import com.android.tools.lint.detector.api.*
4+
import com.android.tools.lint.detector.api.Category
5+
import com.android.tools.lint.detector.api.Detector
6+
import com.android.tools.lint.detector.api.JavaContext
7+
import com.android.tools.lint.detector.api.Scope
58
import org.jetbrains.uast.UElement
69
import org.jetbrains.uast.UImportStatement
710
import java.util.*
@@ -10,20 +13,16 @@ import java.util.*
1013
class NoDataFrameworksFromAndroidClassDetector : Detector(), Detector.UastScanner {
1114

1215
companion object {
13-
private val DETECTOR_CLASS = NoDataFrameworksFromAndroidClassDetector::class.java
1416
private val DETECTOR_SCOPE = Scope.JAVA_FILE_SCOPE
15-
private val IMPLEMENTATION = Implementation(DETECTOR_CLASS, DETECTOR_SCOPE)
16-
private const val ISSUE_ID = "NoDataFrameworksFromAndroidClass"
17-
private const val ISSUE_DESCRIPTION =
18-
"Framework classes to get or store data should never be called from Activities, Fragments or any other Android related view."
19-
private const val ISSUE_EXPLANATION =
20-
"Your Android classes should not be responsible for retrieving or storing information, that should be responsibility of another classes."
21-
private val ISSUE_CATEGORY = Category.INTEROPERABILITY
22-
private const val ISSUE_PRIORITY = 5
23-
private val ISSUE_SEVERITY = Severity.ERROR
24-
val ISSUE = Issue.create(
25-
ISSUE_ID, ISSUE_DESCRIPTION, ISSUE_EXPLANATION, ISSUE_CATEGORY, ISSUE_PRIORITY,
26-
ISSUE_SEVERITY, IMPLEMENTATION
17+
18+
val ISSUE = createIssue<NoDataFrameworksFromAndroidClassDetector>(
19+
"NoDataFrameworksFromAndroidClass",
20+
DETECTOR_SCOPE,
21+
"Framework classes to get or store data should never be called from Activities, Fragments or any other" +
22+
" Android related view.",
23+
"Your Android classes should not be responsible for retrieving or storing information, that should be " +
24+
"responsibility of another classes.",
25+
Category.INTEROPERABILITY
2726
)
2827
}
2928

@@ -46,7 +45,7 @@ class NoDataFrameworksFromAndroidClassDetector : Detector(), Detector.UastScanne
4645
.any { it.isAndroidFrameworkType }
4746

4847
if (fileContainsActivityClass) {
49-
context.report(ISSUE, Location.create(context.file), ISSUE.getBriefDescription(TextFormat.TEXT))
48+
context.report(ISSUE)
5049
}
5150
}
5251
}

‎rules/src/main/java/com/serchinastico/rules/NoElseInSwitchWithEnumOrSealedDetector.kt‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package com.serchinastico.rules
2+
3+
import com.android.tools.lint.client.api.UElementHandler
4+
import com.android.tools.lint.detector.api.Category
5+
import com.android.tools.lint.detector.api.Detector
6+
import com.android.tools.lint.detector.api.JavaContext
7+
import com.android.tools.lint.detector.api.Scope
8+
import org.jetbrains.uast.UElement
9+
import org.jetbrains.uast.USwitchExpression
10+
import java.util.*
11+
12+
class NoElseInSwitchWithEnumOrSealedDetector : Detector(), Detector.UastScanner {
13+
14+
companion object {
15+
private val DETECTOR_SCOPE = Scope.JAVA_FILE_SCOPE
16+
17+
val ISSUE = createIssue<NoElseInSwitchWithEnumOrSealedDetector>(
18+
"NoElseInSwitchWithEnum",
19+
DETECTOR_SCOPE,
20+
"There should not be else/default branches on a switch statement checking for enum/sealed class values",
21+
"Adding an else/default branch breaks extensibility because it won't let you know if there is a missing " +
22+
"implementation when adding new types to the enum/sealed class",
23+
Category.CORRECTNESS
24+
)
25+
}
26+
27+
override fun getApplicableFiles(): EnumSet<Scope> = DETECTOR_SCOPE
28+
29+
override fun getApplicableUastTypes(): List<Class<out UElement>>? =
30+
listOf(USwitchExpression::class.java)
31+
32+
override fun createUastHandler(context: JavaContext): UElementHandler? =
33+
LinElementHandler(context)
34+
35+
private class LinElementHandler(private val context: JavaContext) : UElementHandler() {
36+
override fun visitSwitchExpression(node: USwitchExpression) {
37+
val classReferenceType = node.expression?.getExpressionType() ?: return
38+
39+
if (!classReferenceType.isEnum && !classReferenceType.isSealed) {
40+
return
41+
}
42+
43+
node.clauses.forEach { clause ->
44+
if (clause.isElseBranch) {
45+
context.report(ISSUE)
46+
}
47+
}
48+
}
49+
}
50+
}

‎rules/src/main/java/com/serchinastico/rules/NoPrintStackTraceCallsDetector.kt‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,25 @@
11
package com.serchinastico.rules
22

33
import com.android.tools.lint.client.api.UElementHandler
4-
import com.android.tools.lint.detector.api.*
4+
import com.android.tools.lint.detector.api.Category
5+
import com.android.tools.lint.detector.api.Detector
6+
import com.android.tools.lint.detector.api.JavaContext
7+
import com.android.tools.lint.detector.api.Scope
58
import org.jetbrains.uast.UCallExpression
69
import org.jetbrains.uast.UElement
710
import java.util.*
811

912
class NoPrintStackTraceCallsDetector : Detector(), Detector.UastScanner {
13+
1014
companion object {
11-
private val DETECTOR_CLASS = NoPrintStackTraceCallsDetector::class.java
1215
private val DETECTOR_SCOPE = Scope.JAVA_FILE_SCOPE
13-
private val IMPLEMENTATION = Implementation(DETECTOR_CLASS, DETECTOR_SCOPE)
14-
private const val ISSUE_ID = "NoPrintStackTraceCalls"
15-
private const val ISSUE_DESCRIPTION =
16-
"There should not be calls to the printStackTrace method in Throwable instances"
17-
private const val ISSUE_EXPLANATION =
18-
"Errors should be logged with a configured logger or sent to the backend for faster response"
19-
private val ISSUE_CATEGORY = Category.CORRECTNESS
20-
private const val ISSUE_PRIORITY = 5
21-
private val ISSUE_SEVERITY = Severity.ERROR
22-
val ISSUE = Issue.create(
23-
ISSUE_ID, ISSUE_DESCRIPTION, ISSUE_EXPLANATION, ISSUE_CATEGORY, ISSUE_PRIORITY,
24-
ISSUE_SEVERITY, IMPLEMENTATION
16+
17+
val ISSUE = createIssue<NoPrintStackTraceCallsDetector>(
18+
"NoPrintStackTraceCalls",
19+
DETECTOR_SCOPE,
20+
"There should not be calls to the printStackTrace method in Throwable instances",
21+
"Errors should be logged with a configured logger or sent to the backend for faster response",
22+
Category.CORRECTNESS
2523
)
2624
}
2725

@@ -41,7 +39,7 @@ class NoPrintStackTraceCallsDetector : Detector(), Detector.UastScanner {
4139
val isMethodPrintStackTrace = node.methodIdentifier?.name == "printStackTrace"
4240

4341
if (isReceiverChildOfThrowable && isMethodPrintStackTrace) {
44-
context.report(ISSUE, Location.create(context.file), ISSUE.getBriefDescription(TextFormat.TEXT))
42+
context.report(ISSUE)
4543
}
4644
}
4745
}

‎rules/src/main/java/com/serchinastico/rules/NoPublicViewPropertiesDetector.kt‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,27 @@
11
package com.serchinastico.rules
22

33
import com.android.tools.lint.client.api.UElementHandler
4-
import com.android.tools.lint.detector.api.*
4+
import com.android.tools.lint.detector.api.Category
5+
import com.android.tools.lint.detector.api.Detector
6+
import com.android.tools.lint.detector.api.JavaContext
7+
import com.android.tools.lint.detector.api.Scope
58
import org.jetbrains.uast.UElement
69
import org.jetbrains.uast.UField
710
import java.util.*
811

912
class NoPublicViewPropertiesDetector : Detector(), Detector.UastScanner {
1013

1114
companion object {
12-
private val DETECTOR_CLASS = NoPublicViewPropertiesDetector::class.java
1315
private val DETECTOR_SCOPE = Scope.JAVA_FILE_SCOPE
14-
private val IMPLEMENTATION = Implementation(DETECTOR_CLASS, DETECTOR_SCOPE)
15-
private const val ISSUE_ID = "NoPublicViewProperties"
16-
private const val ISSUE_DESCRIPTION =
17-
"View properties should always be private"
18-
private const val ISSUE_EXPLANATION =
19-
"Exposing views to other classes, be it from activities or custom views is leaking too much information to other classes and is prompt to break if the inner implementation of the layout changes, the only exception is if those views are part of an implemented interface/superclass"
20-
private val ISSUE_CATEGORY = Category.CORRECTNESS
21-
private const val ISSUE_PRIORITY = 5
22-
private val ISSUE_SEVERITY = Severity.ERROR
23-
val ISSUE = Issue.create(
24-
ISSUE_ID, ISSUE_DESCRIPTION, ISSUE_EXPLANATION, ISSUE_CATEGORY, ISSUE_PRIORITY,
25-
ISSUE_SEVERITY, IMPLEMENTATION
16+
17+
val ISSUE = createIssue<NoPublicViewPropertiesDetector>(
18+
"NoPublicViewProperties",
19+
DETECTOR_SCOPE,
20+
"View properties should always be private",
21+
"Exposing views to other classes, be it from activities or custom views is leaking too much" +
22+
" information to other classes and is prompt to break if the inner implementation of" +
23+
" the layout changes, the only exception is if those views are part of an implemented interface",
24+
Category.CORRECTNESS
2625
)
2726
}
2827

@@ -40,7 +39,7 @@ class NoPublicViewPropertiesDetector : Detector(), Detector.UastScanner {
4039
val isViewType = node.isClassOrSubclassOf("android.view.View")
4140

4241
if (!isPrivateField && isViewType) {
43-
context.report(ISSUE, Location.create(context.file), ISSUE.getBriefDescription(TextFormat.TEXT))
42+
context.report(ISSUE)
4443
}
4544
}
4645
}

0 commit comments

Comments
 (0)