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

Add a linter for implementing Dumpable without calling register.

Browse Source 
Fixes: 270355581
Test: atest DumpableNotRegisteredDetectorTest
Change-Id: I9905cd42946fa232715e9c16f1291ac2723835f2
This commit is contained in:
Caitlin Shkuratov
2023-02-21 19:08:40 +00:00
parent 3800ea7c67
commit b36471ff0e
3 changed files with 456 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -0,0 +1,127 @@
/*
* Copyright (C) 2023 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.Context
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.Location
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
import org.jetbrains.uast.UClass
/**
* Checks if any class has implemented the `Dumpable` interface but has not registered itself with
* the `DumpManager`.
*/
@Suppress("UnstableApiUsage")
class DumpableNotRegisteredDetector : Detector(), SourceCodeScanner {
private var isDumpable: Boolean = false
private var isCoreStartable: Boolean = false
private var hasRegisterCall: Boolean = false
private var classLocation: Location? = null
override fun beforeCheckFile(context: Context) {
isDumpable = false
isCoreStartable = false
hasRegisterCall = false
classLocation = null
}
override fun applicableSuperClasses(): List<String> {
return listOf(DUMPABLE_CLASS_NAME)
}
override fun getApplicableMethodNames(): List<String> {
return listOf("registerDumpable", "registerNormalDumpable", "registerCriticalDumpable")
}
override fun visitClass(context: JavaContext, declaration: UClass) {
if (declaration.isInterface || context.evaluator.isAbstract(declaration)) {
// Don't require interfaces or abstract classes to call `register` (assume the full
// implementations will call it). This also means that we correctly don't warn for the
// `Dumpable` interface itself.
return
}
classLocation = context.getNameLocation(declaration)
val superTypeClassNames = declaration.superTypes.mapNotNull { it.resolve()?.qualifiedName }
isDumpable = superTypeClassNames.contains(DUMPABLE_CLASS_NAME)
isCoreStartable = superTypeClassNames.contains(CORE_STARTABLE_CLASS_NAME)
}
override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
if (context.evaluator.isMemberInSubClassOf(method, DUMP_MANAGER_CLASS_NAME)) {
hasRegisterCall = true
}
}
override fun afterCheckFile(context: Context) {
if (!isDumpable) {
return
}
if (isDumpable && isCoreStartable) {
// CoreStartables will be automatically registered, so classes that implement
// CoreStartable do not need a `register` call.
return
}
if (!hasRegisterCall) {
context.report(
issue = ISSUE,
location = classLocation!!,
message =
"Any class implementing `Dumpable` must call " +
"`DumpManager.registerNormalDumpable` or " +
"`DumpManager.registerCriticalDumpable`",
)
}
}
companion object {
@JvmField
val ISSUE: Issue =
Issue.create(
id = "DumpableNotRegistered",
briefDescription = "Dumpable not registered with DumpManager.",
explanation =
"""
This class has implemented the `Dumpable` interface, but it has not registered \
itself with the `DumpManager`. This means that the class will never actually \
be dumped. Please call `DumpManager.registerNormalDumpable` or \
`DumpManager.registerCriticalDumpable` in the class's constructor or \
initialization method.""",
category = Category.CORRECTNESS,
priority = 8,
severity = Severity.WARNING,
implementation =
Implementation(DumpableNotRegisteredDetector::class.java, Scope.JAVA_FILE_SCOPE)
)
private const val DUMPABLE_CLASS_NAME = "com.android.systemui.Dumpable"
private const val CORE_STARTABLE_CLASS_NAME = "com.android.systemui.CoreStartable"
private const val DUMP_MANAGER_CLASS_NAME = "com.android.systemui.dump.DumpManager"
}
}

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

@@ -32,6 +32,7 @@ class SystemUIIssueRegistry : IssueRegistry() {
BindServiceOnMainThreadDetector.ISSUE,
BroadcastSentViaContextDetector.ISSUE,
CleanArchitectureDependencyViolationDetector.ISSUE,
DumpableNotRegisteredDetector.ISSUE,
SlowUserQueryDetector.ISSUE_SLOW_USER_ID_QUERY,
SlowUserQueryDetector.ISSUE_SLOW_USER_INFO_QUERY,
NonInjectedMainThreadDetector.ISSUE,

328
packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DumpableNotRegisteredDetectorTest.kt Normal file
View File

@@ -0,0 +1,328 @@
/*
* Copyright (C) 2023 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.TestFiles
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
import org.junit.Test
@Suppress("UnstableApiUsage")
class DumpableNotRegisteredDetectorTest : SystemUILintDetectorTest() {
override fun getDetector(): Detector = DumpableNotRegisteredDetector()
override fun getIssues(): List<Issue> = listOf(DumpableNotRegisteredDetector.ISSUE)
@Test
fun classIsNotDumpable_noViolation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
class SomeClass() {
}
""".trimIndent()
),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expectClean()
}
@Test
fun classIsDumpable_andRegisterIsCalled_noViolation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
import com.android.systemui.dump.DumpManager;
public class SomeClass implements Dumpable {
SomeClass(DumpManager dumpManager) {
dumpManager.registerDumpable(this);
}
@Override
void dump(PrintWriter pw, String[] args) {
pw.println("testDump");
}
}
""".trimIndent()
),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expectClean()
}
@Test
fun classIsDumpable_andRegisterNormalIsCalled_noViolation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
import com.android.systemui.dump.DumpManager;
public class SomeClass implements Dumpable {
SomeClass(DumpManager dumpManager) {
dumpManager.registerNormalDumpable(this);
}
@Override
void dump(PrintWriter pw, String[] args) {
pw.println("testDump");
}
}
""".trimIndent()
),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expectClean()
}
@Test
fun classIsDumpable_andRegisterCriticalIsCalled_noViolation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
import com.android.systemui.dump.DumpManager;
public class SomeClass implements Dumpable {
SomeClass(DumpManager dumpManager) {
dumpManager.registerCriticalDumpable(this);
}
@Override
void dump(PrintWriter pw, String[] args) {
pw.println("testDump");
}
}
""".trimIndent()
),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expectClean()
}
@Test
fun classIsDumpable_noRegister_violation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
public class SomeClass implements Dumpable {
@Override
public void dump() {
}
}
"""
)
.indented(),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expect(
("""
src/test/pkg/SomeClass.java:5: Warning: Any class implementing Dumpable must call DumpManager.registerNormalDumpable or DumpManager.registerCriticalDumpable [DumpableNotRegistered]
public class SomeClass implements Dumpable {
~~~~~~~~~
0 errors, 1 warnings
""")
.trimIndent()
)
}
@Test
fun classIsDumpable_usesNotDumpManagerMethod_violation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
import com.android.systemui.OtherRegistrationObject;
public class SomeClass implements Dumpable {
public SomeClass(OtherRegistrationObject otherRegistrationObject) {
otherRegistrationObject.registerDumpable(this);
}
@Override
public void dump() {
}
}
"""
)
.indented(),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expect(
("""
src/test/pkg/SomeClass.java:6: Warning: Any class implementing Dumpable must call DumpManager.registerNormalDumpable or DumpManager.registerCriticalDumpable [DumpableNotRegistered]
public class SomeClass implements Dumpable {
~~~~~~~~~
0 errors, 1 warnings
""")
.trimIndent()
)
}
@Test
fun classIsDumpableAndCoreStartable_noRegister_noViolation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
import com.android.systemui.CoreStartable;
public class SomeClass implements Dumpable, CoreStartable {
@Override
public void start() {
}
@Override
public void dump() {
}
}
"""
)
.indented(),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expectClean()
}
@Test
fun classIsAbstract_noRegister_noViolation() {
lint()
.files(
TestFiles.java(
"""
package test.pkg;
import com.android.systemui.Dumpable;
public abstract class SomeClass implements Dumpable {
void abstractMethodHere();
@Override
public void dump() {
}
}
"""
)
.indented(),
*stubs,
)
.issues(DumpableNotRegisteredDetector.ISSUE)
.run()
.expectClean()
}
companion object {
private val DUMPABLE_STUB =
TestFiles.java(
"""
package com.android.systemui;
import com.android.systemui.dump.DumpManager;
import java.io.PrintWriter;
public interface Dumpable {
void dump();
}
"""
)
.indented()
private val DUMP_MANAGER_STUB =
TestFiles.java(
"""
package com.android.systemui.dump;
public interface DumpManager {
void registerDumpable(Dumpable module);
void registerNormalDumpable(Dumpable module);
void registerCriticalDumpable(Dumpable module);
}
"""
)
.indented()
private val OTHER_REGISTRATION_OBJECT_STUB =
TestFiles.java(
"""
package com.android.systemui;
public interface OtherRegistrationObject {
void registerDumpable(Dumpable module);
}
"""
)
.indented()
private val CORE_STARTABLE_STUB =
TestFiles.java(
"""
package com.android.systemui;
public interface CoreStartable {
void start();
}
"""
)
.indented()
private val stubs =
arrayOf(
DUMPABLE_STUB,
DUMP_MANAGER_STUB,
OTHER_REGISTRATION_OBJECT_STUB,
CORE_STARTABLE_STUB,
)
}
}