Merge "Update linter formatting, add new tests" into tm-qpr-dev (b1eebad6) · Commits · e / os / android_frameworks_base

packages/SystemUI/checks/src/com/android/internal/systemui/lint/GetMainLooperViaContextDetector.kt→packages/SystemUI/checks/src/com/android/internal/systemui/lint/BindServiceOnMainThreadDetector.kt

+95 −0

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.internal.systemui.lint

		import com.android.SdkConstants.CLASS_CONTEXT
		import com.android.tools.lint.detector.api.Category
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Implementation
		@@ -25,42 +26,70 @@ 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 com.intellij.psi.PsiModifierListOwner
		import org.jetbrains.uast.UCallExpression
		import org.jetbrains.uast.UClass
		import org.jetbrains.uast.UMethod
		import org.jetbrains.uast.getParentOfType

		/**
		* Warns if {@code Context.bindService}, {@code Context.bindServiceAsUser}, or {@code
		* Context.unbindService} is not called on a {@code WorkerThread}
		*/
		@Suppress("UnstableApiUsage")
		class GetMainLooperViaContextDetector : Detector(), SourceCodeScanner {
		class BindServiceOnMainThreadDetector : Detector(), SourceCodeScanner {

		override fun getApplicableMethodNames(): List<String> {
		return listOf("getMainThreadHandler", "getMainLooper", "getMainExecutor")
		return listOf("bindService", "bindServiceAsUser", "unbindService")
		}

		private fun hasWorkerThreadAnnotation(
		context: JavaContext,
		annotated: PsiModifierListOwner?
		): Boolean {
		return context.evaluator.getAnnotations(annotated, inHierarchy = true).any { uAnnotation ->
		uAnnotation.qualifiedName == "androidx.annotation.WorkerThread"
		}
		}

		override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
		if (context.evaluator.isMemberInSubClassOf(method, "android.content.Context")) {
		if (context.evaluator.isMemberInSubClassOf(method, CLASS_CONTEXT)) {
		if (
		!hasWorkerThreadAnnotation(context, node.getParentOfType(UMethod::class.java)) &&
		!hasWorkerThreadAnnotation(context, node.getParentOfType(UClass::class.java))
		) {
		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"Please inject a @Main Executor instead."
		context.getLocation(node),
		"This method should be annotated with `@WorkerThread` because " +
		"it calls ${method.name}",
		)
		}
		}
		}

		companion object {
		@JvmField
		val ISSUE: Issue =
		Issue.create(
		id = "GetMainLooperViaContextDetector",
		briefDescription = "Please use idiomatic SystemUI executors, injecting " +
		"them via Dagger.",
		explanation = "Injecting the @Main Executor is preferred in order to make" +
		"dependencies explicit and increase testability. It's much " +
		"easier to pass a FakeExecutor on your test ctor than to " +
		"deal with loopers in unit tests.",
		category = Category.LINT,
		id = "BindServiceOnMainThread",
		briefDescription = "Service bound or unbound on main thread",
		explanation =
		"""
		Binding and unbinding services are synchronous calls to `ActivityManager`. \
		They usually take multiple milliseconds to complete. If called on the main \
		thread, it will likely cause missed frames. To fix it, use a `@Background \
		Executor` and annotate the calling method with `@WorkerThread`.
		""",
		category = Category.PERFORMANCE,
		priority = 8,
		severity = Severity.WARNING,
		implementation = Implementation(GetMainLooperViaContextDetector::class.java,
		Scope.JAVA_FILE_SCOPE)
		implementation =
		Implementation(
		BindServiceOnMainThreadDetector::class.java,
		Scope.JAVA_FILE_SCOPE
		)
		)
		}
		}

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

+13 −12

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.internal.systemui.lint

		import com.android.SdkConstants.CLASS_CONTEXT
		import com.android.tools.lint.detector.api.Category
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Implementation
		@@ -48,14 +49,14 @@ class BroadcastSentViaContextDetector : Detector(), SourceCodeScanner {
		return
		}

		val evaulator = context.evaluator
		if (evaulator.isMemberInSubClassOf(method, "android.content.Context")) {
		val evaluator = context.evaluator
		if (evaluator.isMemberInSubClassOf(method, CLASS_CONTEXT)) {
		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"Please don't call sendBroadcast/sendBroadcastAsUser directly on " +
		"Context, use com.android.systemui.broadcast.BroadcastSender instead."
		"`Context.${method.name}()` should be replaced with " +
		"`BroadcastSender.${method.name}()`"
		)
		}
		}
		@@ -65,14 +66,14 @@ class BroadcastSentViaContextDetector : Detector(), SourceCodeScanner {
		val ISSUE: Issue =
		Issue.create(
		id = "BroadcastSentViaContext",
		briefDescription = "Broadcast sent via Context instead of BroadcastSender.",
		explanation =
		"Broadcast was sent via " +
		"Context.sendBroadcast/Context.sendBroadcastAsUser. Please use " +
		"BroadcastSender.sendBroadcast/BroadcastSender.sendBroadcastAsUser " +
		"which will schedule dispatch of broadcasts on background thread. " +
		"Sending broadcasts on main thread causes jank due to synchronous " +
		"Binder calls.",
		briefDescription = "Broadcast sent via `Context` instead of `BroadcastSender`",
		// lint trims indents and converts \ to line continuations
		explanation = """
		Broadcasts sent via `Context.sendBroadcast()` or \
		`Context.sendBroadcastAsUser()` will block the main thread and may cause \
		missed frames. Instead, use `BroadcastSender.sendBroadcast()` or \
		`BroadcastSender.sendBroadcastAsUser()` which will schedule and dispatch \
		broadcasts on a background worker thread.""",
		category = Category.PERFORMANCE,
		priority = 8,
		severity = Severity.WARNING,

packages/SystemUI/checks/src/com/android/internal/systemui/lint/BindServiceViaContextDetector.kt→packages/SystemUI/checks/src/com/android/internal/systemui/lint/NonInjectedMainThreadDetector.kt

+18 −16

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.internal.systemui.lint

		import com.android.SdkConstants.CLASS_CONTEXT
		import com.android.tools.lint.detector.api.Category
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Implementation
		@@ -28,20 +29,19 @@ import com.intellij.psi.PsiMethod
		import org.jetbrains.uast.UCallExpression

		@Suppress("UnstableApiUsage")
		class BindServiceViaContextDetector : Detector(), SourceCodeScanner {
		class NonInjectedMainThreadDetector : Detector(), SourceCodeScanner {

		override fun getApplicableMethodNames(): List<String> {
		return listOf("bindService", "bindServiceAsUser", "unbindService")
		return listOf("getMainThreadHandler", "getMainLooper", "getMainExecutor")
		}

		override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
		if (context.evaluator.isMemberInSubClassOf(method, "android.content.Context")) {
		if (context.evaluator.isMemberInSubClassOf(method, CLASS_CONTEXT)) {
		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"Binding or unbinding services are synchronous calls, please make " +
		"sure you're on a @Background Executor."
		"Replace with injected `@Main Executor`."
		)
		}
		}
		@@ -50,18 +50,20 @@ class BindServiceViaContextDetector : Detector(), SourceCodeScanner {
		@JvmField
		val ISSUE: Issue =
		Issue.create(
		id = "BindServiceViaContextDetector",
		briefDescription = "Service bound/unbound via Context, please make sure " +
		"you're on a background thread.",
		id = "NonInjectedMainThread",
		briefDescription = "Main thread usage without dependency injection",
		explanation =
		"Binding or unbinding services are synchronous calls to ActivityManager, " +
		"they usually take multiple milliseconds to complete and will make" +
		"the caller drop frames. Make sure you're on a @Background Executor.",
		category = Category.PERFORMANCE,
		"""
		Main thread should be injected using the `@Main Executor` instead \
		of using the accessors in `Context`. This is to make the \
		dependencies explicit and increase testability. It's much easier \
		to pass a `FakeExecutor` on test constructors than it is to deal \
		with loopers in unit tests.""",
		category = Category.LINT,
		priority = 8,
		severity = Severity.WARNING,
		implementation =
		Implementation(BindServiceViaContextDetector::class.java, Scope.JAVA_FILE_SCOPE)
		Implementation(NonInjectedMainThreadDetector::class.java, Scope.JAVA_FILE_SCOPE)
		)
		}
		}

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

+23 −11

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.internal.systemui.lint

		import com.android.SdkConstants.CLASS_CONTEXT
		import com.android.tools.lint.detector.api.Category
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Implementation
		@@ -32,7 +33,7 @@ import org.jetbrains.uast.UCallExpression
		class NonInjectedServiceDetector : Detector(), SourceCodeScanner {

		override fun getApplicableMethodNames(): List<String> {
		return listOf("getSystemService")
		return listOf("getSystemService", "get")
		}

		override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
		@@ -40,14 +41,25 @@ class NonInjectedServiceDetector : Detector(), SourceCodeScanner {
		if (
		!evaluator.isStatic(method) &&
		method.name == "getSystemService" &&
		method.containingClass?.qualifiedName == "android.content.Context"
		method.containingClass?.qualifiedName == CLASS_CONTEXT
		) {
		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"Use @Inject to get the handle to a system-level services instead of using " +
		"Context.getSystemService()"
		"Use `@Inject` to get system-level service handles instead of " +
		"`Context.getSystemService()`"
		)
		} else if (
		evaluator.isStatic(method) &&
		method.name == "get" &&
		method.containingClass?.qualifiedName == "android.accounts.AccountManager"
		) {
		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"Replace `AccountManager.get()` with an injected instance of `AccountManager`"
		)
		}
		}
		@@ -57,14 +69,14 @@ class NonInjectedServiceDetector : Detector(), SourceCodeScanner {
		val ISSUE: Issue =
		Issue.create(
		id = "NonInjectedService",
		briefDescription =
		"System-level services should be retrieved using " +
		"@Inject instead of Context.getSystemService().",
		briefDescription = "System service not injected",
		explanation =
		"Context.getSystemService() should be avoided because it makes testing " +
		"difficult. Instead, use an injected service. For example, " +
		"instead of calling Context.getSystemService(UserManager.class), " +
		"use @Inject and add UserManager to the constructor",
		"""
		`Context.getSystemService()` should be avoided because it makes testing \
		difficult. Instead, use an injected service. For example, instead of calling \
		`Context.getSystemService(UserManager.class)` in a class, annotate the class' \
		constructor with `@Inject` and add `UserManager` to the parameters.
		""",
		category = Category.CORRECTNESS,
		priority = 8,
		severity = Severity.WARNING,

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

+13 −10

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.internal.systemui.lint

		import com.android.SdkConstants.CLASS_CONTEXT
		import com.android.tools.lint.detector.api.Category
		import com.android.tools.lint.detector.api.Detector
		import com.android.tools.lint.detector.api.Implementation
		@@ -35,12 +36,12 @@ class RegisterReceiverViaContextDetector : Detector(), SourceCodeScanner {
		}

		override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
		if (context.evaluator.isMemberInSubClassOf(method, "android.content.Context")) {
		if (context.evaluator.isMemberInSubClassOf(method, CLASS_CONTEXT)) {
		context.report(
		ISSUE,
		method,
		context.getNameLocation(node),
		"BroadcastReceivers should be registered via BroadcastDispatcher."
		"Register `BroadcastReceiver` using `BroadcastDispatcher` instead of `Context`"
		)
		}
		}
		@@ -49,14 +50,16 @@ class RegisterReceiverViaContextDetector : Detector(), SourceCodeScanner {
		@JvmField
		val ISSUE: Issue =
		Issue.create(
		id = "RegisterReceiverViaContextDetector",
		briefDescription = "Broadcast registrations via Context are blocking " +
		"calls. Please use BroadcastDispatcher.",
		explanation =
		"Context#registerReceiver is a blocking call to the system server, " +
		"making it very likely that you'll drop a frame. Please use " +
		"BroadcastDispatcher instead (or move this call to a " +
		"@Background Executor.)",
		id = "RegisterReceiverViaContext",
		briefDescription = "Blocking broadcast registration",
		// lint trims indents and converts \ to line continuations
		explanation = """
		`Context.registerReceiver()` is a blocking call to the system server, \
		making it very likely that you'll drop a frame. Please use \
		`BroadcastDispatcher` instead, which registers the receiver on a \
		background thread. `BroadcastDispatcher` also improves our visibility \
		into ANRs.""",
		moreInfo = "go/identifying-broadcast-threads",
		category = Category.PERFORMANCE,
		priority = 8,
		severity = Severity.WARNING,