diff --git a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DumpableNotRegisteredDetector.kt b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DumpableNotRegisteredDetector.kt new file mode 100644 index 0000000000000..30e2a2527a93c --- /dev/null +++ b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/DumpableNotRegisteredDetector.kt @@ -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 { + return listOf(DUMPABLE_CLASS_NAME) + } + + override fun getApplicableMethodNames(): List { + 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" + } +} diff --git a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt index 254a6fb4714f4..84f70502fa45c 100644 --- a/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt +++ b/packages/SystemUI/checks/src/com/android/internal/systemui/lint/SystemUIIssueRegistry.kt @@ -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, diff --git a/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DumpableNotRegisteredDetectorTest.kt b/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DumpableNotRegisteredDetectorTest.kt new file mode 100644 index 0000000000000..3d6cbc749569a --- /dev/null +++ b/packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DumpableNotRegisteredDetectorTest.kt @@ -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 = 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, + ) + } +}