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

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

0 → 100644

+127 −0

Original line number	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	Diff line number	Diff line
		@@ -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,

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

0 → 100644

+328 −0

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