New lint detector for Context.getSystemService()

Test: manual
Bug: 238923086
Change-Id: I1f7b01812225cefa84050208334dc7de2d5b4878
This commit is contained in:
Peter Kalauskas
2022-10-03 15:50:00 -07:00
parent 805815a15a
commit 26f1f09866
4 changed files with 163 additions and 0 deletions

View File

@@ -0,0 +1,75 @@
/*
* 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
/** Detects usage of Context.getSystemService() and suggests to use an injected instance instead. */
@Suppress("UnstableApiUsage")
class NonInjectedServiceDetector : Detector(), SourceCodeScanner {
override fun getApplicableMethodNames(): List<String> {
return listOf("getSystemService")
}
override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
val evaluator = context.evaluator
if (
!evaluator.isStatic(method) &&
method.name == "getSystemService" &&
method.containingClass?.qualifiedName == "android.content.Context"
) {
context.report(
ISSUE,
method,
context.getNameLocation(node),
"Use @Inject to get the handle to a system-level services instead of using " +
"Context.getSystemService()"
)
}
}
companion object {
@JvmField
val ISSUE: Issue =
Issue.create(
id = "NonInjectedService",
briefDescription =
"System-level services should be retrieved using " +
"@Inject instead of Context.getSystemService().",
explanation =
"Context.getSystemService() should be avoided because it makes testing " +
"difficult. Instead, use an injected service. For example, " +
"instead of calling Context.getSystemService(UserManager.class), " +
"use @Inject and add UserManager to the constructor",
category = Category.CORRECTNESS,
priority = 8,
severity = Severity.WARNING,
implementation =
Implementation(NonInjectedServiceDetector::class.java, Scope.JAVA_FILE_SCOPE)
)
}
}

View File

@@ -35,6 +35,7 @@ class SystemUIIssueRegistry : IssueRegistry() {
GetMainLooperViaContextDetector.ISSUE,
RegisterReceiverViaContextDetector.ISSUE,
SoftwareBitmapDetector.ISSUE,
NonInjectedServiceDetector.ISSUE,
)
override val api: Int

View File

@@ -103,6 +103,8 @@ public class Context {
public Looper getMainLooper() { return null; }
public Executor getMainExecutor() { return null; }
public Handler getMainThreadHandler() { return null; }
public final @Nullable <T> T getSystemService(@NonNull Class<T> serviceClass) { return null; }
public abstract @Nullable Object getSystemService(@ServiceName @NonNull String name);
}
"""
),

View File

@@ -0,0 +1,85 @@
/*
* 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.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
@Suppress("UnstableApiUsage")
class NonInjectedServiceDetectorTest : LintDetectorTest() {
override fun getDetector(): Detector = NonInjectedServiceDetector()
override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
override fun getIssues(): List<Issue> = listOf(NonInjectedServiceDetector.ISSUE)
@Test
fun testGetServiceWithString() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import android.content.Context;
public class TestClass1 {
public void getSystemServiceWithoutDagger(Context context) {
context.getSystemService("user");
}
}
"""
)
.indented(),
*stubs
)
.issues(NonInjectedServiceDetector.ISSUE)
.run()
.expectWarningCount(1)
.expectContains("Use @Inject to get the handle")
}
@Test
fun testGetServiceWithClass() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import android.content.Context;
import android.os.UserManager;
public class TestClass2 {
public void getSystemServiceWithoutDagger(Context context) {
context.getSystemService(UserManager.class);
}
}
"""
)
.indented(),
*stubs
)
.issues(NonInjectedServiceDetector.ISSUE)
.run()
.expectWarningCount(1)
.expectContains("Use @Inject to get the handle")
}
private val stubs = androidStubs
}