Merge "New lint checks for static Settings.* usages" into tm-qpr-dev (95971855) · Commits · e / os / android_frameworks_base

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

0 → 100644

+100 −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

		private const val CLASS_SETTINGS = "android.provider.Settings"

		/**
		* Detects usage of static methods in android.provider.Settings and suggests to use an injected
		* settings provider instance instead.
		*/
		@Suppress("UnstableApiUsage")
		class StaticSettingsProviderDetector : Detector(), SourceCodeScanner {
		override fun getApplicableMethodNames(): List<String> {
		return listOf(
		"getFloat",
		"getInt",
		"getLong",
		"getString",
		"getUriFor",
		"putFloat",
		"putInt",
		"putLong",
		"putString"
		)
		}

		override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
		val evaluator = context.evaluator
		val className = method.containingClass?.qualifiedName
		if (
		className != "$CLASS_SETTINGS.Global" &&
		className != "$CLASS_SETTINGS.Secure" &&
		className != "$CLASS_SETTINGS.System"
		) {
		return
		}
		if (!evaluator.isStatic(method)) {
		return
		}

		val subclassName = className.substring(CLASS_SETTINGS.length + 1)

		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"`@Inject` a ${subclassName}Settings instead"
		)
		}

		companion object {
		@JvmField
		val ISSUE: Issue =
		Issue.create(
		id = "StaticSettingsProvider",
		briefDescription = "Static settings provider usage",
		explanation =
		"""
		Static settings provider methods, such as `Settings.Global.putInt()`, should \
		not be used because they make testing difficult. Instead, use an injected \
		settings provider. For example, instead of calling `Settings.Secure.getInt()`, \
		annotate the class constructor with `@Inject` and add `SecureSettings` to the \
		parameters.
		""",
		category = Category.CORRECTNESS,
		priority = 8,
		severity = Severity.WARNING,
		implementation =
		Implementation(
		StaticSettingsProviderDetector::class.java,
		Scope.JAVA_FILE_SCOPE
		)
		)
		}
		}

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

+1 −0

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@ class SystemUIIssueRegistry : IssueRegistry() {
		RegisterReceiverViaContextDetector.ISSUE,
		SoftwareBitmapDetector.ISSUE,
		NonInjectedServiceDetector.ISSUE,
		StaticSettingsProviderDetector.ISSUE
		)

		override val api: Int

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

+65 −0

Original line number	Diff line number	Diff line
		@@ -24,6 +24,47 @@ import org.intellij.lang.annotations.Language
		@NonNull
		private fun indentedJava(@NonNull @Language("JAVA") source: String) = java(source).indented()

		internal val commonSettingsCode =
		"""
		public static float getFloat(ContentResolver cr, String name) { return 0.0f; }
		public static long getLong(ContentResolver cr, String name) {
		return 0L;
		}
		public static int getInt(ContentResolver cr, String name) {
		return 0;
		}
		public static String getString(ContentResolver cr, String name) {
		return "";
		}
		public static float getFloat(ContentResolver cr, String name, float def) {
		return 0.0f;
		}
		public static long getLong(ContentResolver cr, String name, long def) {
		return 0L;
		}
		public static int getInt(ContentResolver cr, String name, int def) {
		return 0;
		}
		public static String getString(ContentResolver cr, String name, String def) {
		return "";
		}
		public static boolean putFloat(ContentResolver cr, String name, float value) {
		return true;
		}
		public static boolean putLong(ContentResolver cr, String name, long value) {
		return true;
		}
		public static boolean putInt(ContentResolver cr, String name, int value) {
		return true;
		}
		public static boolean putFloat(ContentResolver cr, String name) {
		return true;
		}
		public static boolean putString(ContentResolver cr, String name, String value) {
		return true;
		}
		"""

		/*
		* This file contains stubs of framework APIs and System UI classes for testing purposes only. The
		* stubs are not used in the lint detectors themselves.
		@@ -184,6 +225,30 @@ import static java.lang.annotation.RetentionPolicy.SOURCE;
		@Target({METHOD,CONSTRUCTOR,TYPE,PARAMETER})
		public @interface WorkerThread {
		}
		"""
		),
		indentedJava(
		"""
		package android.provider;

		public class Settings {
		public static final class Global {
		public static final String UNLOCK_SOUND = "unlock_sound";
		""" +
		commonSettingsCode +
		"""
		}
		public static final class Secure {
		""" +
		commonSettingsCode +
		"""
		}
		public static final class System {
		""" +
		commonSettingsCode +
		"""
		}
		}
		"""
		),
		)

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

+1 −4

Original line number	Diff line number	Diff line
		@@ -16,18 +16,15 @@

		package com.android.internal.systemui.lint

		import com.android.tools.lint.checks.infrastructure.LintDetectorTest
		import com.android.tools.lint.checks.infrastructure.TestFiles
		import com.android.tools.lint.checks.infrastructure.TestLintTask
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Issue
		import org.junit.Test

		@Suppress("UnstableApiUsage")
		class BindServiceOnMainThreadDetectorTest : LintDetectorTest() {
		class BindServiceOnMainThreadDetectorTest : SystemUILintDetectorTest() {

		override fun getDetector(): Detector = BindServiceOnMainThreadDetector()
		override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

		override fun getIssues(): List<Issue> = listOf(BindServiceOnMainThreadDetector.ISSUE)

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

+1 −4

Original line number	Diff line number	Diff line
		@@ -16,18 +16,15 @@

		package com.android.internal.systemui.lint

		import com.android.tools.lint.checks.infrastructure.LintDetectorTest
		import com.android.tools.lint.checks.infrastructure.TestFiles
		import com.android.tools.lint.checks.infrastructure.TestLintTask
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Issue
		import org.junit.Test

		@Suppress("UnstableApiUsage")
		class BroadcastSentViaContextDetectorTest : LintDetectorTest() {
		class BroadcastSentViaContextDetectorTest : SystemUILintDetectorTest() {

		override fun getDetector(): Detector = BroadcastSentViaContextDetector()
		override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

		override fun getIssues(): List<Issue> = listOf(BroadcastSentViaContextDetector.ISSUE)