Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prefer Executor injection

Browse Source 
Injecting a @Main Executor makes dependencies clearer and simplifies
unit tests. We don't need to deal with loopers, MainLooper annotations,
etc.

Test: atest GetMainLooperViaContextDetectorTest
Bug: 238923086
Change-Id: Iff3bce716e4b3d9903a6056e269a3a6cde5f05c1
This commit is contained in:
Lucas Dupin
2022-08-10 17:39:22 -07:00
parent 6110cca06f
commit 3bb0be437a
3 changed files with 202 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
packages/SystemUI/checks/src/com/android/internal/systemui/lint/GetMainLooperViaContextDetector.kt Normal file
View File

@@ -0,0 +1,66 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.internal.systemui.lint
import com.android.tools.lint.detector.api.Category
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Implementation
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.android.tools.lint.detector.api.SourceCodeScanner
import com.intellij.psi.PsiMethod
import org.jetbrains.uast.UCallExpression
@Suppress("UnstableApiUsage")
class GetMainLooperViaContextDetector : Detector(), SourceCodeScanner {
override fun getApplicableMethodNames(): List<String> {
return listOf("getMainThreadHandler", "getMainLooper", "getMainExecutor")
}
override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
if (context.evaluator.isMemberInSubClassOf(method, "android.content.Context")) {
context.report(
ISSUE,
method,
context.getNameLocation(node),
"Please inject a @Main Executor instead."
)
}
}
companion object {
@JvmField
val ISSUE: Issue =
Issue.create(
id = "GetMainLooperViaContextDetector",
briefDescription = "Please use idiomatic SystemUI executors, injecting " +
"them via Dagger.",
explanation = "Injecting the @Main Executor is preferred in order to make" +
"dependencies explicit and increase testability. It's much " +
"easier to pass a FakeExecutor on your test ctor than to " +
"deal with loopers in unit tests.",
category = Category.LINT,
priority = 8,
severity = Severity.WARNING,
implementation = Implementation(GetMainLooperViaContextDetector::class.java,
Scope.JAVA_FILE_SCOPE)
)
}
}

1
packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt
View File

@@ -29,6 +29,7 @@ class SystemUIIssueRegistry : IssueRegistry() {
override val issues: List<Issue>
get() = listOf(BindServiceViaContextDetector.ISSUE,
BroadcastSentViaContextDetector.ISSUE,
GetMainLooperViaContextDetector.ISSUE,
RegisterReceiverViaContextDetector.ISSUE
)

135
packages/SystemUI/checks/tests/com/android/systemui/lint/GetMainLooperViaContextDetectorTest.kt Normal file
View File

@@ -0,0 +1,135 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.internal.systemui.lint
import com.android.tools.lint.checks.infrastructure.LintDetectorTest
import com.android.tools.lint.checks.infrastructure.TestFile
import com.android.tools.lint.checks.infrastructure.TestFiles
import com.android.tools.lint.checks.infrastructure.TestLintTask
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
import org.junit.Test
class GetMainLooperViaContextDetectorTest : LintDetectorTest() {
override fun getDetector(): Detector = GetMainLooperViaContextDetector()
override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
override fun getIssues(): List<Issue> = listOf(GetMainLooperViaContextDetector.ISSUE)
private val explanation = "Please inject a @Main Executor instead."
@Test
fun testGetMainThreadHandler() {
lint().files(
TestFiles.java(
"""
package test.pkg;
import android.content.Context;
import android.os.Handler;
public class TestClass1 {
public void test(Context context) {
Handler mainThreadHandler = context.getMainThreadHandler();
}
}
"""
).indented(),
*stubs)
.issues(GetMainLooperViaContextDetector.ISSUE)
.run()
.expectWarningCount(1)
.expectContains(explanation)
}
@Test
fun testGetMainLooper() {
lint().files(
TestFiles.java(
"""
package test.pkg;
import android.content.Context;
import android.os.Looper;
public class TestClass1 {
public void test(Context context) {
Looper mainLooper = context.getMainLooper();
}
}
"""
).indented(),
*stubs)
.issues(GetMainLooperViaContextDetector.ISSUE)
.run()
.expectWarningCount(1)
.expectContains(explanation)
}
@Test
fun testGetMainExecutor() {
lint().files(
TestFiles.java(
"""
package test.pkg;
import android.content.Context;
import java.util.concurrent.Executor;
public class TestClass1 {
public void test(Context context) {
Executor mainExecutor = context.getMainExecutor();
}
}
"""
).indented(),
*stubs)
.issues(GetMainLooperViaContextDetector.ISSUE)
.run()
.expectWarningCount(1)
.expectContains(explanation)
}
private val contextStub: TestFile = java(
"""
package android.content;
import android.os.Handler;import android.os.Looper;import java.util.concurrent.Executor;
public class Context {
public Looper getMainLooper() { return null; };
public Executor getMainExecutor() { return null; };
public Handler getMainThreadHandler() { return null; };
}
"""
)
private val looperStub: TestFile = java(
"""
package android.os;
public class Looper {}
"""
)
private val handlerStub: TestFile = java(
"""
package android.os;
public class Handler {}
"""
)
private val stubs = arrayOf(contextStub, looperStub, handlerStub)
}