Skip to content

Commit d816ea8

Browse files
vkryachkovkryachko
authored
Allow task subclasses to implement overloads. (#4394)
1 parent 0824cc4 commit d816ea8

File tree

4 files changed

+164
-25
lines changed

4 files changed

+164
-25
lines changed

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package com.google.firebase.appdistribution.impl;
1616

17-
import android.annotation.SuppressLint;
1817
import android.app.Activity;
1918
import androidx.annotation.GuardedBy;
2019
import androidx.annotation.NonNull;
@@ -36,7 +35,6 @@
3635

3736
/** Implementation of UpdateTask, the return type of updateApp. */
3837
// TODO(b/261013814): Use an explicit executor in continuations.
39-
@SuppressLint("TaskMainThread")
4038
class UpdateTaskImpl extends UpdateTask {
4139
@Nullable
4240
@GuardedBy("lock")

‎firebase-firestore/src/main/java/com/google/firebase/firestore/LoadBundleTask.java

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import static com.google.firebase.firestore.util.Assert.hardAssert;
1818

19-
import android.annotation.SuppressLint;
2019
import android.app.Activity;
2120
import androidx.annotation.GuardedBy;
2221
import androidx.annotation.NonNull;
@@ -137,8 +136,6 @@ public Exception getException() {
137136
*
138137
* @return this {@code Task}
139138
*/
140-
// TODO(b/261013682): Use an explicit executor in continuations.
141-
@SuppressLint("TaskMainThread")
142139
@NonNull
143140
@Override
144141
public Task<LoadBundleTaskProgress> addOnSuccessListener(
@@ -175,8 +172,6 @@ public Task<LoadBundleTaskProgress> addOnSuccessListener(
175172
* removed.
176173
* @return this {@code Task}
177174
*/
178-
// TODO(b/261013682): Use an explicit executor in continuations.
179-
@SuppressLint("TaskMainThread")
180175
@NonNull
181176
@Override
182177
public Task<LoadBundleTaskProgress> addOnSuccessListener(
@@ -194,8 +189,6 @@ public Task<LoadBundleTaskProgress> addOnSuccessListener(
194189
*
195190
* @return this {@code Task}
196191
*/
197-
// TODO(b/261013682): Use an explicit executor in continuations.
198-
@SuppressLint("TaskMainThread")
199192
@NonNull
200193
@Override
201194
public Task<LoadBundleTaskProgress> addOnFailureListener(
@@ -229,8 +222,6 @@ public Task<LoadBundleTaskProgress> addOnFailureListener(
229222
* removed.
230223
* @return this {@code Task}
231224
*/
232-
// TODO(b/261013682): Use an explicit executor in continuations.
233-
@SuppressLint("TaskMainThread")
234225
@NonNull
235226
@Override
236227
public Task<LoadBundleTaskProgress> addOnFailureListener(
@@ -247,8 +238,6 @@ public Task<LoadBundleTaskProgress> addOnFailureListener(
247238
*
248239
* @return this {@code Task}
249240
*/
250-
// TODO(b/261013682): Use an explicit executor in continuations.
251-
@SuppressLint("TaskMainThread")
252241
@NonNull
253242
@Override
254243
public Task<LoadBundleTaskProgress> addOnCompleteListener(
@@ -283,8 +272,6 @@ public Task<LoadBundleTaskProgress> addOnCompleteListener(
283272
* removed.
284273
* @return this {@code Task}
285274
*/
286-
// TODO(b/261013682): Use an explicit executor in continuations.
287-
@SuppressLint("TaskMainThread")
288275
@NonNull
289276
@Override
290277
public Task<LoadBundleTaskProgress> addOnCompleteListener(
@@ -302,8 +289,6 @@ public Task<LoadBundleTaskProgress> addOnCompleteListener(
302289
*
303290
* @return this {@code Task}
304291
*/
305-
// TODO(b/261013682): Use an explicit executor in continuations.
306-
@SuppressLint("TaskMainThread")
307292
@NonNull
308293
@Override
309294
public Task<LoadBundleTaskProgress> addOnCanceledListener(
@@ -338,8 +323,6 @@ public Task<LoadBundleTaskProgress> addOnCanceledListener(
338323
*
339324
* @return this Task
340325
*/
341-
// TODO(b/261013682): Use an explicit executor in continuations.
342-
@SuppressLint("TaskMainThread")
343326
@NonNull
344327
@Override
345328
public Task<LoadBundleTaskProgress> addOnCanceledListener(
@@ -355,8 +338,6 @@ public Task<LoadBundleTaskProgress> addOnCanceledListener(
355338
*
356339
* @see Continuation#then(Task)
357340
*/
358-
// TODO(b/261013682): Use an explicit executor in continuations.
359-
@SuppressLint("TaskMainThread")
360341
@NonNull
361342
@Override
362343
public <TContinuationResult> Task<TContinuationResult> continueWith(
@@ -387,8 +368,6 @@ public <TContinuationResult> Task<TContinuationResult> continueWith(
387368
*
388369
* @see Continuation#then(Task)
389370
*/
390-
// TODO(b/261013682): Use an explicit executor in continuations.
391-
@SuppressLint("TaskMainThread")
392371
@NonNull
393372
@Override
394373
public <TContinuationResult> Task<TContinuationResult> continueWithTask(
@@ -424,8 +403,6 @@ public <TContinuationResult> Task<TContinuationResult> continueWithTask(
424403
*
425404
* @see SuccessContinuation#then(ResultT)
426405
*/
427-
// TODO(b/261013682): Use an explicit executor in continuations.
428-
@SuppressLint("TaskMainThread")
429406
@NonNull
430407
@Override
431408
public <TContinuationResult> Task<TContinuationResult> onSuccessTask(

‎tools/lint/src/main/kotlin/TasksMainThreadDetector.kt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,13 @@ import com.android.tools.lint.detector.api.SourceCodeScanner
2525
import com.intellij.psi.PsiClass
2626
import com.intellij.psi.PsiMethod
2727
import com.intellij.psi.PsiParameter
28+
import com.intellij.psi.util.InheritanceUtil
2829
import org.jetbrains.uast.UCallExpression
30+
import org.jetbrains.uast.UClass
31+
import org.jetbrains.uast.UMethod
32+
import org.jetbrains.uast.getParentOfType
33+
34+
internal val GENERICS_PATTERN = Regex("<.*>")
2935

3036
@Suppress("DetectorIsMissingAnnotations")
3137
class TasksMainThreadDetector : Detector(), SourceCodeScanner {
@@ -46,6 +52,19 @@ class TasksMainThreadDetector : Detector(), SourceCodeScanner {
4652
return
4753
}
4854

55+
// It's ok to call from a subclass of Task as it needs to implement overloads that don't take an
56+
// executor.
57+
val callingClass = node.getParentOfType<UClass>()
58+
if (
59+
callingClass != null &&
60+
InheritanceUtil.isInheritor(callingClass.javaPsi, "com.google.android.gms.tasks.Task")
61+
) {
62+
val callingMethod = node.getParentOfType<UMethod>()?.javaPsi
63+
if (method.isSameMethodAs(callingMethod)) {
64+
return
65+
}
66+
}
67+
4968
val firstArgument: PsiParameter = method.parameterList.parameters.firstOrNull() ?: return
5069
if (!firstArgument.type.equalsToText("java.util.concurrent.Executor")) {
5170
context.report(
@@ -56,6 +75,12 @@ class TasksMainThreadDetector : Detector(), SourceCodeScanner {
5675
}
5776
}
5877

78+
private fun PsiMethod.isSameMethodAs(other: PsiMethod?): Boolean =
79+
other != null &&
80+
name == other.name &&
81+
parameterList.parameters.map { it.type.toString().replace(GENERICS_PATTERN, "") } ==
82+
other.parameterList.parameters.map { it.type.toString().replace(GENERICS_PATTERN, "") }
83+
5984
private fun isTaskMethod(method: PsiMethod): Boolean {
6085
(method.parent as? PsiClass)?.let {
6186
return it.qualifiedName == "com.google.android.gms.tasks.Task"

‎tools/lint/src/test/kotlin/TasksMainThreadDetectorTests.kt

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.lint.checks
16+
17+
import com.android.tools.lint.checks.infrastructure.LintDetectorTest
18+
import com.android.tools.lint.detector.api.Detector
19+
import com.android.tools.lint.detector.api.Issue
20+
21+
fun taskSource(): String {
22+
return """
23+
package com.google.android.gms.tasks;
24+
25+
import java.util.concurrent.Executor;
26+
27+
public interface Task<TResult> {
28+
Task<TResult> continueWith(Executor executor, String dummy);
29+
Task<TResult> continueWith(String dummy);
30+
}
31+
"""
32+
.trimIndent()
33+
}
34+
35+
class TasksMainThreadDetectorTests : LintDetectorTest() {
36+
override fun getDetector(): Detector = TasksMainThreadDetector()
37+
38+
override fun getIssues(): MutableList<Issue> =
39+
mutableListOf(TasksMainThreadDetector.TASK_MAIN_THREAD)
40+
41+
fun test_continueWith_withoutExecutor_shouldFail() {
42+
lint()
43+
.files(
44+
java(taskSource()),
45+
java(
46+
"""
47+
import com.google.android.gms.tasks.Task;
48+
class Hello {
49+
public static Task<String> useTask(Task<String> task) {
50+
return task.continueWith("hello");
51+
}
52+
}
53+
"""
54+
.trimIndent()
55+
)
56+
)
57+
.run()
58+
.checkContains("Use an Executor explicitly to avoid running on the main thread")
59+
}
60+
61+
fun test_continueWith_withExecutor_shouldSucceed() {
62+
lint()
63+
.files(
64+
java(taskSource()),
65+
java(
66+
"""
67+
import com.google.android.gms.tasks.Task;
68+
import java.util.concurrent.Executor;
69+
class Hello {
70+
public static Task<String> useTask(Executor executor, Task<String> task) {
71+
return task.continueWith(executor, "hello");
72+
}
73+
}
74+
"""
75+
.trimIndent()
76+
)
77+
)
78+
.run()
79+
.expectClean()
80+
}
81+
82+
fun test_continueWith_withoutExecutor_whileImplementingOverload_shouldSucceed() {
83+
lint()
84+
.files(
85+
java(taskSource()),
86+
java(
87+
"""
88+
import com.google.android.gms.tasks.Task;
89+
import java.util.concurrent.Executor;
90+
class Hello<TResult> implements Task<TResult> {
91+
private final Task<TResult> delegate;
92+
93+
Hello(Task<TResult> delegate) { this.delegate = delegate;}
94+
95+
@Override
96+
public Task<TResult> continueWith(Executor executor, String dummy) {
97+
return delegate.continueWith(executor, dummy);
98+
}
99+
public Task<TResult> continueWith(String dummy) {
100+
return delegate.continueWith(dummy);
101+
}
102+
}
103+
"""
104+
.trimIndent()
105+
)
106+
)
107+
.run()
108+
.expectClean()
109+
}
110+
111+
fun test_continueWith_withoutExecutor_fromWrongOverload_shouldFail() {
112+
lint()
113+
.files(
114+
java(taskSource()),
115+
java(
116+
"""
117+
import com.google.android.gms.tasks.Task;
118+
import java.util.concurrent.Executor;
119+
class Hello<TResult> implements Task<TResult> {
120+
private final Task<TResult> delegate;
121+
122+
Hello(Task<TResult> delegate) { this.delegate = delegate;}
123+
124+
@Override
125+
public Task<TResult> continueWith(Executor executor, String dummy) {
126+
return delegate.continueWith(dummy);
127+
}
128+
public Task<TResult> continueWith(String dummy) {
129+
return delegate.continueWith(dummy);
130+
}
131+
}
132+
"""
133+
.trimIndent()
134+
)
135+
)
136+
.run()
137+
.checkContains("Use an Executor explicitly to avoid running on the main thread")
138+
}
139+
}

0 commit comments

Comments
 (0)