Merge changes from topic "collect-as-state" into main (22efac91) · Commits · e / os / android_frameworks_base

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

0 → 100644

+97 −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.client.api.UElementHandler
		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 org.jetbrains.uast.UElement
		import org.jetbrains.uast.UFile
		import org.jetbrains.uast.UImportStatement

		class CollectAsStateDetector : Detector(), SourceCodeScanner {

		override fun getApplicableUastTypes(): List<Class<out UElement>> {
		return listOf(UFile::class.java)
		}

		override fun createUastHandler(context: JavaContext): UElementHandler {
		return object : UElementHandler() {
		override fun visitFile(node: UFile) {
		node.imports.forEach { importStatement ->
		visitImportStatement(context, importStatement)
		}
		}
		}
		}

		private fun visitImportStatement(
		context: JavaContext,
		importStatement: UImportStatement,
		) {
		val importText = importStatement.importReference?.asSourceString() ?: return
		if (ILLEGAL_IMPORT == importText) {
		context.report(
		issue = ISSUE,
		scope = importStatement,
		location = context.getLocation(importStatement),
		message = "collectAsState considered harmful",
		)
		}
		}

		companion object {
		@JvmField
		val ISSUE =
		Issue.create(
		id = "OverlyEagerCollectAsState",
		briefDescription = "collectAsState considered harmful",
		explanation =
		"""
		go/sysui-compose#collect-as-state

		Don't use collectAsState as it will set up a coroutine that keeps collecting from a
		flow until its coroutine scope becomes inactive. This prevents the work from being
		properly paused while the surrounding lifecycle becomes paused or stopped and is
		therefore considered harmful.

		Instead, use Flow.collectAsStateWithLifecycle(initial: T) or
		StateFlow.collectAsStateWithLifecycle(). These APIs correctly pause the collection
		coroutine while the lifecycle drops below the specified minActiveState (which
		defaults to STARTED meaning that it will pause when the Compose-hosting window
		becomes invisible).
		"""
		.trimIndent(),
		category = Category.PERFORMANCE,
		priority = 8,
		severity = Severity.ERROR,
		implementation =
		Implementation(
		CollectAsStateDetector::class.java,
		Scope.JAVA_FILE_SCOPE,
		),
		)

		private val ILLEGAL_IMPORT = "androidx.compose.runtime.collectAsState"
		}
		}

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,
		CollectAsStateDetector.ISSUE,
		DumpableNotRegisteredDetector.ISSUE,
		FlowDetector.SHARED_FLOW_CREATION,
		SlowUserQueryDetector.ISSUE_SLOW_USER_ID_QUERY,

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

0 → 100644

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

		class CollectAsStateDetectorTest : SystemUILintDetectorTest() {

		override fun getDetector(): Detector {
		return CollectAsStateDetector()
		}

		override fun getIssues(): List<Issue> {
		return listOf(
		CollectAsStateDetector.ISSUE,
		)
		}

		@Test
		fun testViolation() {
		lint()
		.files(COLLECT_AS_STATE_STUB, COLLECT_WITH_LIFECYCLE_AS_STATE_STUB, GOOD_FILE, BAD_FILE)
		.issues(CollectAsStateDetector.ISSUE)
		.run()
		.expect(
		"""
		src/com/android/internal/systemui/lint/Bad.kt:3: Error: collectAsState considered harmful [OverlyEagerCollectAsState]
		import androidx.compose.runtime.collectAsState
		~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
		1 errors, 0 warnings
		"""
		.trimIndent()
		)
		}

		@Test
		fun testNoViolation() {
		lint()
		.files(COLLECT_AS_STATE_STUB, COLLECT_WITH_LIFECYCLE_AS_STATE_STUB, GOOD_FILE)
		.issues(CollectAsStateDetector.ISSUE)
		.run()
		.expectClean()
		}

		companion object {
		private val COLLECT_AS_STATE_STUB =
		TestFiles.kotlin(
		"""
		package androidx.compose.runtime

		fun collectAsState() {}
		"""
		.trimIndent()
		)
		private val COLLECT_WITH_LIFECYCLE_AS_STATE_STUB =
		TestFiles.kotlin(
		"""
		package androidx.lifecycle.compose

		fun collectAsStateWithLifecycle() {}
		"""
		.trimIndent()
		)

		private val BAD_FILE =
		TestFiles.kotlin(
		"""
		package com.android.internal.systemui.lint

		import androidx.compose.runtime.collectAsState

		class Bad
		"""
		.trimIndent()
		)

		private val GOOD_FILE =
		TestFiles.kotlin(
		"""
		package com.android.internal.systemui.lint

		import androidx.lifecycle.compose.collectAsStateWithLifecycle

		class Good
		"""
		.trimIndent()
		)
		}
		}

packages/SystemUI/compose/features/src/com/android/systemui/bouncer/ui/composable/BouncerContent.kt

+13 −12

Original line number	Diff line number	Diff line
		@@ -56,7 +56,6 @@ import androidx.compose.material3.windowsizeclass.WindowHeightSizeClass
		import androidx.compose.runtime.Composable
		import androidx.compose.runtime.DisposableEffect
		import androidx.compose.runtime.LaunchedEffect
		import androidx.compose.runtime.collectAsState
		import androidx.compose.runtime.getValue
		import androidx.compose.runtime.mutableStateOf
		import androidx.compose.runtime.remember
		@@ -77,6 +76,7 @@ import androidx.compose.ui.unit.LayoutDirection
		import androidx.compose.ui.unit.dp
		import androidx.compose.ui.unit.sp
		import androidx.compose.ui.unit.times
		import androidx.lifecycle.compose.collectAsStateWithLifecycle
		import com.android.compose.PlatformButton
		import com.android.compose.animation.Easings
		import com.android.compose.animation.scene.ElementKey
		@@ -111,7 +111,7 @@ fun BouncerContent(
		dialogFactory: BouncerDialogFactory,
		modifier: Modifier = Modifier,
		) {
		val isSideBySideSupported by viewModel.isSideBySideSupported.collectAsState()
		val isSideBySideSupported by viewModel.isSideBySideSupported.collectAsStateWithLifecycle()
		val layout = calculateLayout(isSideBySideSupported = isSideBySideSupported)

		Box(
		@@ -220,7 +220,7 @@ private fun SplitLayout(
		viewModel: BouncerViewModel,
		modifier: Modifier = Modifier,
		) {
		val authMethod by viewModel.authMethodViewModel.collectAsState()
		val authMethod by viewModel.authMethodViewModel.collectAsStateWithLifecycle()

		Row(
		modifier =
		@@ -316,7 +316,7 @@ private fun BesideUserSwitcherLayout(
		val (isSwapped, setSwapped) = rememberSaveable(isLeftToRight) { mutableStateOf(!isLeftToRight) }
		val isHeightExpanded =
		LocalWindowSizeClass.current.heightSizeClass == WindowHeightSizeClass.Expanded
		val authMethod by viewModel.authMethodViewModel.collectAsState()
		val authMethod by viewModel.authMethodViewModel.collectAsStateWithLifecycle()

		Row(
		modifier =
		@@ -480,7 +480,7 @@ private fun FoldAware(
		modifier: Modifier = Modifier,
		) {
		val foldPosture: FoldPosture by foldPosture()
		val isSplitAroundTheFoldRequired by viewModel.isFoldSplitRequired.collectAsState()
		val isSplitAroundTheFoldRequired by viewModel.isFoldSplitRequired.collectAsStateWithLifecycle()
		val isSplitAroundTheFold = foldPosture == FoldPosture.Tabletop && isSplitAroundTheFoldRequired
		val currentSceneKey =
		if (isSplitAroundTheFold) SceneKeys.SplitSceneKey else SceneKeys.ContiguousSceneKey
		@@ -562,7 +562,7 @@ private fun StatusMessage(
		viewModel: BouncerMessageViewModel,
		modifier: Modifier = Modifier,
		) {
		val message: MessageViewModel? by viewModel.message.collectAsState()
		val message: MessageViewModel? by viewModel.message.collectAsStateWithLifecycle()

		DisposableEffect(Unit) {
		viewModel.onShown()
		@@ -612,7 +612,7 @@ private fun OutputArea(
		modifier: Modifier = Modifier,
		) {
		val authMethodViewModel: AuthMethodBouncerViewModel? by
		viewModel.authMethodViewModel.collectAsState()
		viewModel.authMethodViewModel.collectAsStateWithLifecycle()

		when (val nonNullViewModel = authMethodViewModel) {
		is PinBouncerViewModel ->
		@@ -642,7 +642,7 @@ private fun InputArea(
		modifier: Modifier = Modifier,
		) {
		val authMethodViewModel: AuthMethodBouncerViewModel? by
		viewModel.authMethodViewModel.collectAsState()
		viewModel.authMethodViewModel.collectAsStateWithLifecycle()

		when (val nonNullViewModel = authMethodViewModel) {
		is PinBouncerViewModel -> {
		@@ -668,7 +668,8 @@ private fun ActionArea(
		viewModel: BouncerViewModel,
		modifier: Modifier = Modifier,
		) {
		val actionButton: BouncerActionButtonModel? by viewModel.actionButton.collectAsState()
		val actionButton: BouncerActionButtonModel? by
		viewModel.actionButton.collectAsStateWithLifecycle()
		val appearFadeInAnimatable = remember { Animatable(0f) }
		val appearMoveAnimatable = remember { Animatable(0f) }
		val appearAnimationInitialOffset = with(LocalDensity.current) { 80.dp.toPx() }
		@@ -735,7 +736,7 @@ private fun Dialog(
		bouncerViewModel: BouncerViewModel,
		dialogFactory: BouncerDialogFactory,
		) {
		val dialogViewModel by bouncerViewModel.dialogViewModel.collectAsState()
		val dialogViewModel by bouncerViewModel.dialogViewModel.collectAsStateWithLifecycle()
		var dialog: AlertDialog? by remember { mutableStateOf(null) }

		dialogViewModel?.let { viewModel ->
		@@ -772,8 +773,8 @@ private fun UserSwitcher(
		return
		}

		val selectedUserImage by viewModel.selectedUserImage.collectAsState(null)
		val dropdownItems by viewModel.userSwitcherDropdown.collectAsState(emptyList())
		val selectedUserImage by viewModel.selectedUserImage.collectAsStateWithLifecycle(null)
		val dropdownItems by viewModel.userSwitcherDropdown.collectAsStateWithLifecycle(emptyList())

		Column(
		horizontalAlignment = Alignment.CenterHorizontally,

packages/SystemUI/compose/features/src/com/android/systemui/bouncer/ui/composable/PasswordBouncer.kt

+9 −7

Original line number	Diff line number	Diff line
		@@ -27,7 +27,6 @@ import androidx.compose.material3.TextField
		import androidx.compose.runtime.Composable
		import androidx.compose.runtime.DisposableEffect
		import androidx.compose.runtime.LaunchedEffect
		import androidx.compose.runtime.collectAsState
		import androidx.compose.runtime.getValue
		import androidx.compose.runtime.remember
		import androidx.compose.ui.ExperimentalComposeUiApi
		@@ -49,6 +48,7 @@ import androidx.compose.ui.text.input.KeyboardType
		import androidx.compose.ui.text.input.PasswordVisualTransformation
		import androidx.compose.ui.text.style.TextAlign
		import androidx.compose.ui.unit.dp
		import androidx.lifecycle.compose.collectAsStateWithLifecycle
		import com.android.compose.PlatformIconButton
		import com.android.systemui.bouncer.ui.viewmodel.PasswordBouncerViewModel
		import com.android.systemui.common.ui.compose.SelectedUserAwareInputConnection
		@@ -62,18 +62,20 @@ internal fun PasswordBouncer(
		modifier: Modifier = Modifier,
		) {
		val focusRequester = remember { FocusRequester() }
		val isTextFieldFocusRequested by viewModel.isTextFieldFocusRequested.collectAsState()
		val isTextFieldFocusRequested by
		viewModel.isTextFieldFocusRequested.collectAsStateWithLifecycle()
		LaunchedEffect(isTextFieldFocusRequested) {
		if (isTextFieldFocusRequested) {
		focusRequester.requestFocus()
		}
		}

		val password: String by viewModel.password.collectAsState()
		val isInputEnabled: Boolean by viewModel.isInputEnabled.collectAsState()
		val animateFailure: Boolean by viewModel.animateFailure.collectAsState()
		val isImeSwitcherButtonVisible by viewModel.isImeSwitcherButtonVisible.collectAsState()
		val selectedUserId by viewModel.selectedUserId.collectAsState()
		val password: String by viewModel.password.collectAsStateWithLifecycle()
		val isInputEnabled: Boolean by viewModel.isInputEnabled.collectAsStateWithLifecycle()
		val animateFailure: Boolean by viewModel.animateFailure.collectAsStateWithLifecycle()
		val isImeSwitcherButtonVisible by
		viewModel.isImeSwitcherButtonVisible.collectAsStateWithLifecycle()
		val selectedUserId by viewModel.selectedUserId.collectAsStateWithLifecycle()

		DisposableEffect(Unit) { onDispose { viewModel.onHidden() } }