Skip to content

Commit d460977

Browse files
pavlosptsamtstern
pavlospt
authored and
samtstern
committed
Firestore adapter lifecycle lint checks (#1340)
1 parent 5fac07e commit d460977

File tree

5 files changed

+220
-18
lines changed

5 files changed

+220
-18
lines changed

‎internal/lint/src/main/java/com/firebaseui/lint/FirestoreRecyclerAdapterLifecycleDetector.kt

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package com.firebaseui.lint
2+
3+
import com.android.tools.lint.client.api.UElementHandler
4+
import com.android.tools.lint.detector.api.Category.Companion.CORRECTNESS
5+
import com.android.tools.lint.detector.api.Detector
6+
import com.android.tools.lint.detector.api.Implementation
7+
import com.android.tools.lint.detector.api.Issue
8+
import com.android.tools.lint.detector.api.JavaContext
9+
import com.android.tools.lint.detector.api.Scope
10+
import com.android.tools.lint.detector.api.Severity.WARNING
11+
import org.jetbrains.uast.UCallExpression
12+
import org.jetbrains.uast.UClass
13+
import org.jetbrains.uast.UField
14+
import org.jetbrains.uast.visitor.AbstractUastVisitor
15+
import java.util.EnumSet
16+
17+
class FirestoreRecyclerAdapterLifecycleDetector : Detector(), Detector.UastScanner {
18+
19+
override fun getApplicableUastTypes() = listOf(UClass::class.java)
20+
21+
override fun createUastHandler(context: JavaContext) = MissingLifecycleMethodsVisitor(context)
22+
23+
class MissingLifecycleMethodsVisitor(
24+
private val context: JavaContext
25+
) : UElementHandler() {
26+
private val FIRESTORE_RECYCLER_ADAPTER_TYPE =
27+
"FirestoreRecyclerAdapter"
28+
29+
override fun visitClass(node: UClass) {
30+
val adapterReferences = node
31+
.fields
32+
.filter { FIRESTORE_RECYCLER_ADAPTER_TYPE == it.type.canonicalText }
33+
.map { AdapterReference(it) }
34+
35+
node.accept(AdapterStartListeningMethodVisitor(adapterReferences))
36+
node.accept(AdapterStopListeningMethodVisitor(adapterReferences))
37+
38+
adapterReferences.forEach {
39+
if (it.hasCalledStart && !it.hasCalledStop) {
40+
context.report(
41+
ISSUE_MISSING_LISTENING_STOP_METHOD,
42+
it.uField,
43+
context.getLocation(it.uField),
44+
"Have called .startListening() without .stopListening()."
45+
)
46+
} else if (!it.hasCalledStart) {
47+
context.report(
48+
ISSUE_MISSING_LISTENING_START_METHOD,
49+
it.uField,
50+
context.getLocation(it.uField),
51+
"Have not called .startListening()."
52+
)
53+
}
54+
}
55+
}
56+
}
57+
58+
class AdapterStartListeningMethodVisitor(
59+
private val adapterReferences: List<AdapterReference>
60+
) : AbstractUastVisitor() {
61+
private val START_LISTENING_METHOD_NAME = "startListening"
62+
63+
override fun visitCallExpression(node: UCallExpression): Boolean =
64+
if (START_LISTENING_METHOD_NAME == node.methodName) {
65+
adapterReferences
66+
.find { it.uField.name == node.receiver?.asRenderString() }
67+
?.let {
68+
it.hasCalledStart = true
69+
}
70+
true
71+
} else {
72+
super.visitCallExpression(node)
73+
}
74+
}
75+
76+
class AdapterStopListeningMethodVisitor(
77+
private val adapterReferences: List<AdapterReference>
78+
) : AbstractUastVisitor() {
79+
private val STOP_LISTENING_METHOD_NAME = "stopListening"
80+
81+
override fun visitCallExpression(node: UCallExpression): Boolean =
82+
if (STOP_LISTENING_METHOD_NAME == node.methodName) {
83+
adapterReferences
84+
.find { it.uField.name == node.receiver?.asRenderString() }
85+
?.let {
86+
it.hasCalledStop = true
87+
}
88+
true
89+
} else {
90+
super.visitCallExpression(node)
91+
}
92+
}
93+
94+
companion object {
95+
val ISSUE_MISSING_LISTENING_START_METHOD = Issue.create(
96+
"FirestoreAdapterMissingStartListeningMethod",
97+
"Checks if FirestoreAdapter has called .startListening() method.",
98+
"If a class is using a FirestoreAdapter and does not call startListening it won't be " +
99+
"notified on changes.",
100+
CORRECTNESS, 10, WARNING,
101+
Implementation(
102+
FirestoreRecyclerAdapterLifecycleDetector::class.java,
103+
EnumSet.of(Scope.JAVA_FILE)
104+
)
105+
)
106+
107+
val ISSUE_MISSING_LISTENING_STOP_METHOD = Issue.create(
108+
"FirestoreAdapterMissingStopListeningMethod",
109+
"Checks if FirestoreAdapter has called .stopListening() method.",
110+
"If a class is using a FirestoreAdapter and has called .startListening() but missing " +
111+
" .stopListening() might cause issues with RecyclerView data changes.",
112+
CORRECTNESS, 10, WARNING,
113+
Implementation(
114+
FirestoreRecyclerAdapterLifecycleDetector::class.java,
115+
EnumSet.of(Scope.JAVA_FILE)
116+
)
117+
)
118+
}
119+
}
120+
121+
data class AdapterReference(
122+
val uField: UField,
123+
var hasCalledStart: Boolean = false,
124+
var hasCalledStop: Boolean = false
125+
)

‎internal/lint/src/main/java/com/firebaseui/lint/LintIssueRegistry.kt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,9 @@ import com.android.tools.lint.client.api.IssueRegistry
66
* Registry for custom FirebaseUI lint checks.
77
*/
88
class LintIssueRegistry : IssueRegistry() {
9-
override val issues = listOf(NonGlobalIdDetector.NON_GLOBAL_ID)
9+
override val issues = listOf(
10+
NonGlobalIdDetector.NON_GLOBAL_ID,
11+
FirestoreRecyclerAdapterLifecycleDetector.ISSUE_MISSING_LISTENING_START_METHOD,
12+
FirestoreRecyclerAdapterLifecycleDetector.ISSUE_MISSING_LISTENING_STOP_METHOD
13+
)
1014
}

‎internal/lint/src/test/java/com/firebaseui/lint/FirestoreRecyclerAdapterLifecycleDetectorTest.kt

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
package com.firebaseui.lint
2+
3+
import com.android.tools.lint.checks.infrastructure.TestFiles.java
4+
import com.firebaseui.lint.FirestoreRecyclerAdapterLifecycleDetector.Companion.ISSUE_MISSING_LISTENING_START_METHOD
5+
import com.firebaseui.lint.FirestoreRecyclerAdapterLifecycleDetector.Companion.ISSUE_MISSING_LISTENING_STOP_METHOD
6+
import com.firebaseui.lint.LintTestHelper.configuredLint
7+
import org.junit.Test
8+
9+
class FirestoreRecyclerAdapterLifecycleDetectorTest {
10+
11+
@Test
12+
fun `Checks missing startListening() method call`() {
13+
configuredLint()
14+
.files(java("""
15+
|public class MissingStartListeningMethodCall {
16+
| private FirestoreRecyclerAdapter adapter;
17+
|}
18+
""".trimMargin()))
19+
.issues(ISSUE_MISSING_LISTENING_START_METHOD)
20+
.run()
21+
.expect("""
22+
|src/MissingStartListeningMethodCall.java:2: Warning: Have not called .startListening(). [FirestoreAdapterMissingStartListeningMethod]
23+
| private FirestoreRecyclerAdapter adapter;
24+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
25+
|0 errors, 1 warnings
26+
""".trimMargin())
27+
}
28+
29+
@Test
30+
fun `Checks missing stopListening() method call`() {
31+
configuredLint()
32+
.files(java("""
33+
|public class MissingStopListeningMethodCall {
34+
| private FirestoreRecyclerAdapter adapter;
35+
|
36+
| public void onStart() {
37+
| adapter.startListening();
38+
| }
39+
|}
40+
""".trimMargin()))
41+
.issues(ISSUE_MISSING_LISTENING_STOP_METHOD)
42+
.run()
43+
.expect("""
44+
|src/MissingStopListeningMethodCall.java:2: Warning: Have called .startListening() without .stopListening(). [FirestoreAdapterMissingStopListeningMethod]
45+
| private FirestoreRecyclerAdapter adapter;
46+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
47+
|0 errors, 1 warnings
48+
""".trimMargin())
49+
}
50+
51+
@Test
52+
fun `Checks no warnings when startListening & stopListening methods called`() {
53+
configuredLint()
54+
.files(java("""
55+
|public class HasCalledStartStopListeningMethods {
56+
| private FirestoreRecyclerAdapter adapter;
57+
|
58+
| public void onStart() {
59+
| adapter.startListening();
60+
| }
61+
|
62+
| public void onStop() {
63+
| adapter.stopListening();
64+
| }
65+
|}
66+
""".trimMargin()))
67+
.issues(ISSUE_MISSING_LISTENING_START_METHOD, ISSUE_MISSING_LISTENING_STOP_METHOD)
68+
.run()
69+
.expectClean()
70+
}
71+
}

‎internal/lint/src/test/java/com/firebaseui/lint/LintTestHelper.kt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package com.firebaseui.lint
2+
3+
import com.android.tools.lint.checks.infrastructure.TestLintTask
4+
import java.io.File
5+
6+
object LintTestHelper {
7+
// Nasty hack to make lint tests pass on Windows. For some reason, lint doesn't
8+
// automatically find the Android SDK in its standard path on Windows. This hack looks
9+
// through the system properties to find the path defined in `local.properties` and then
10+
// sets lint's SDK home to that path if it's found.
11+
private val sdkPath = System.getProperty("java.library.path").split(';').find {
12+
it.contains("SDK", true)
13+
}
14+
15+
fun configuredLint(): TestLintTask = TestLintTask.lint().apply {
16+
sdkHome(File(sdkPath ?: return@apply))
17+
}
18+
}

‎internal/lint/src/test/java/com/firebaseui/lint/NonGlobalIdDetectorTest.kt

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
package com.firebaseui.lint
22

33
import com.android.tools.lint.checks.infrastructure.TestFiles.xml
4-
import com.android.tools.lint.checks.infrastructure.TestLintTask
5-
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
4+
import com.firebaseui.lint.LintTestHelper.configuredLint
65
import com.firebaseui.lint.NonGlobalIdDetector.Companion.NON_GLOBAL_ID
76
import org.junit.Test
8-
import java.io.File
97

108
class NonGlobalIdDetectorTest {
119
@Test
@@ -42,18 +40,4 @@ class NonGlobalIdDetectorTest {
4240
|+ android:id="@+id/invalid"/>
4341
|""".trimMargin())
4442
}
45-
46-
companion object {
47-
// Nasty hack to make lint tests pass on Windows. For some reason, lint doesn't
48-
// automatically find the Android SDK in its standard path on Windows. This hack looks
49-
// through the system properties to find the path defined in `local.properties` and then
50-
// sets lint's SDK home to that path if it's found.
51-
private val sdkPath = System.getProperty("java.library.path").split(';').find {
52-
it.contains("SDK", true)
53-
}
54-
55-
fun configuredLint(): TestLintTask = lint().apply {
56-
sdkHome(File(sdkPath ?: return@apply))
57-
}
58-
}
5943
}

0 commit comments

Comments
 (0)