Merge "Add a linter for implementing `Dumpable` without calling `register`."... (bfc087f5) · Commits · e / os / android_frameworks_base

packages/SystemUI/checks/src/com/android/internal/systemui/lint/DumpableNotRegisteredDetector.kt

0 → 100644

+127 −0

Original line number	Original line	Diff line number	Diff line
			/*
			* 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"
			}
			}

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

+1 −0

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

packages/SystemUI/checks/tests/com/android/internal/systemui/lint/DumpableNotRegisteredDetectorTest.kt

0 → 100644

+328 −0

Original line number	Original line	Diff line number	Diff line
			/*
			* 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,
			)
			}
			}