Add linter check to warn usage of registerContentObserver() synchronous API. (0bc23a89) · Commits · e / os / android_frameworks_base

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

0 → 100644

+94 −0

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

		/**
		* Checks if the synchronous APIs like registerContentObserverSync/unregisterContentObserverSync are
		* invoked for SettingsProxy or it's sub-classes, and raise a warning notifying the caller to use
		* the asynchronous/suspend APIs instead.
		*/
		@Suppress("UnstableApiUsage")
		class RegisterContentObserverSyncViaSettingsProxyDetector : Detector(), SourceCodeScanner {

		override fun getApplicableMethodNames(): List<String> {
		return SYNC_METHOD_LIST
		}

		override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {

		val evaluator = context.evaluator
		if (evaluator.isMemberInSubClassOf(method, SETTINGS_PROXY_CLASS)) {
		context.report(
		issue = SYNC_WARNING,
		location = context.getNameLocation(node),
		message =
		"`Avoid using ${method.name}()` if calling the API is not " +
		"required on the main thread. Instead use an appropriate async interface " +
		"API call for eg. `registerContentObserver()` or " +
		"`registerContentObserverAsync()`."
		)
		}
		}

		companion object {
		val SYNC_WARNING: Issue =
		Issue.create(
		id = "RegisterContentObserverSyncWarning",
		briefDescription =
		"Synchronous content observer registration API called " +
		"instead of the async APIs.`",
		// lint trims indents and converts \ to line continuations
		explanation =
		"""
		ContentObserver registration/de-registration done via \
		`SettingsProxy.registerContentObserverSync` will block the main thread \
		and may cause missed frames. Instead, use \
		`SettingsProxy.registerContentObserver()` or \
		`SettingsProxy.registerContentObserverAsync()`. These APIs will ensure \
		that the registrations/de-registrations happen sequentially on a
		background worker thread.""",
		category = Category.PERFORMANCE,
		priority = 8,
		severity = Severity.WARNING,
		implementation =
		Implementation(
		RegisterContentObserverSyncViaSettingsProxyDetector::class.java,
		Scope.JAVA_FILE_SCOPE
		)
		)

		private val SYNC_METHOD_LIST =
		listOf(
		"registerContentObserverSync",
		"unregisterContentObserverSync",
		"registerContentObserverForUserSync"
		)

		private val SETTINGS_PROXY_CLASS = "com.android.systemui.util.settings.SettingsProxy"
		}
		}

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

+2 −0

Original line number	Diff line number	Diff line
		@@ -46,10 +46,12 @@ class SystemUIIssueRegistry : IssueRegistry() {
		DemotingTestWithoutBugDetector.ISSUE,
		TestFunctionNameViolationDetector.ISSUE,
		MissingApacheLicenseDetector.ISSUE,
		RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING
		)

		override val api: Int
		get() = CURRENT_API

		override val minApi: Int
		get() = 8

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

0 → 100644

+211 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2024 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

		/** Test class for [RegisterContentObserverSyncViaSettingsProxyDetector]. */
		class RegisterContentObserverSyncViaSettingsProxyDetectorTest : SystemUILintDetectorTest() {
		override fun getDetector(): Detector = RegisterContentObserverSyncViaSettingsProxyDetector()

		override fun getIssues(): List<Issue> =
		listOf(RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING)

		@Test
		fun testRegisterContentObserverSync_throwError() {
		lint()
		.files(
		TestFiles.java(
		"""
		package test.pkg;
		import com.android.systemui.util.settings.SecureSettings;
		public class TestClass {
		public void register(SecureSettings secureSettings) {
		secureSettings.
		registerContentObserverSync(Settings.Global.getUriFor(Settings.Global.DEVICE_PROVISIONED),
		false, mSettingObserver);
		}
		}
		"""
		)
		.indented(),
		*stubs
		)
		.issues(RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING)
		.run()
		.expect(
		"""
		src/test/pkg/TestClass.java:6: Warning: Avoid using registerContentObserverSync() if calling the API is not required on the main thread. Instead use an appropriate async interface API call for eg. registerContentObserver() or registerContentObserverAsync(). [RegisterContentObserverSyncWarning]
		registerContentObserverSync(Settings.Global.getUriFor(Settings.Global.DEVICE_PROVISIONED),
		~~~~~~~~~~~~~~~~~~~~~~~~~~~
		0 errors, 1 warnings
		"""
		.trimIndent()
		)
		}

		@Test
		fun testRegisterContentObserverForUserSync_throwError() {
		lint()
		.files(
		TestFiles.java(
		"""
		package test.pkg;
		import com.android.systemui.util.settings.SecureSettings;
		public class TestClass {
		public void register(SecureSettings secureSettings) {
		secureSettings.
		registerContentObserverForUserSync(Settings.Global.getUriFor(Settings.Global.DEVICE_PROVISIONED),
		false, mSettingObserver);
		}
		}
		"""
		)
		.indented(),
		*stubs
		)
		.issues(RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING)
		.run()
		.expect(
		"""
		src/test/pkg/TestClass.java:6: Warning: Avoid using registerContentObserverForUserSync() if calling the API is not required on the main thread. Instead use an appropriate async interface API call for eg. registerContentObserver() or registerContentObserverAsync(). [RegisterContentObserverSyncWarning]
		registerContentObserverForUserSync(Settings.Global.getUriFor(Settings.Global.DEVICE_PROVISIONED),
		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
		0 errors, 1 warnings
		"""
		.trimIndent()
		)
		}

		@Test
		fun testSuppressRegisterContentObserverSync() {
		lint()
		.files(
		TestFiles.java(
		"""
		package test.pkg;
		import com.android.systemui.util.settings.SecureSettings;
		public class TestClass {
		@SuppressWarnings("RegisterContentObserverSyncWarning")
		public void register(SecureSettings secureSettings) {
		secureSettings.
		registerContentObserverForUserSync(Settings.Global.getUriFor(Settings.Global.DEVICE_PROVISIONED),
		false, mSettingObserver);
		}
		}
		"""
		)
		.indented(),
		*stubs
		)
		.issues(RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING)
		.run()
		.expectClean()
		}

		@Test
		fun testNoopIfNoCall() {
		lint()
		.files(
		TestFiles.java(
		"""
		package test.pkg;
		import com.android.systemui.util.settings.SecureSettings;
		public class TestClass {
		public void register(SecureSettings secureSettings) {
		}
		}
		"""
		)
		.indented(),
		*stubs
		)
		.issues(RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING)
		.run()
		.expectClean()
		}

		@Test
		fun testUnRegisterContentObserverSync_throwError() {
		lint()
		.files(
		TestFiles.java(
		"""
		package test.pkg;
		import com.android.systemui.util.settings.SecureSettings;
		public class TestClass {
		public void register(SecureSettings secureSettings) {
		secureSettings.
		unregisterContentObserverSync(mSettingObserver);
		}
		}
		"""
		)
		.indented(),
		*stubs
		)
		.issues(RegisterContentObserverSyncViaSettingsProxyDetector.SYNC_WARNING)
		.run()
		.expect(
		"""
		src/test/pkg/TestClass.java:6: Warning: Avoid using unregisterContentObserverSync() if calling the API is not required on the main thread. Instead use an appropriate async interface API call for eg. registerContentObserver() or registerContentObserverAsync(). [RegisterContentObserverSyncWarning]
		unregisterContentObserverSync(mSettingObserver);
		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
		0 errors, 1 warnings
		"""
		.trimIndent()
		)
		}

		private companion object {
		private val SETTINGS_PROXY_STUB =
		kotlin(
		"""
		package com.android.systemui.util.settings
		interface SettingsProxy {
		fun registerContentObserverSync() {}
		fun unregisterContentObserverSync() {}
		}
		"""
		)
		.indented()

		private val USER_SETTINGS_PROXY_STUB =
		kotlin(
		"""
		package com.android.systemui.util.settings
		interface UserSettingsProxy : SettingsProxy {
		fun registerContentObserverForUserSync() {}
		}
		"""
		)
		.indented()

		private val SECURE_SETTINGS_STUB =
		kotlin(
		"""
		package com.android.systemui.util.settings
		interface SecureSettings : UserSettingsProxy {}
		"""
		)
		.indented()
		}

		private val stubs = arrayOf(SETTINGS_PROXY_STUB, USER_SETTINGS_PROXY_STUB, SECURE_SETTINGS_STUB)
		}