Skip to content

fix(auth)!: resolve the 12 baselined Compose lint findings #2502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
demolaf wants to merge 1 commit into from
Closed

fix(auth)!: resolve the 12 baselined Compose lint findings #2502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions auth/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ android {
warningsAsErrors = true
abortOnError = true

// Pre-existing debt only: 168 localization findings (CPRN-432) and 12 Compose
// correctness findings (CPRN-436). Every entry is suppressed; new ones still fail.
// Pre-existing debt only: 168 localization findings (CPRN-432). Every entry is
// suppressed; new ones still fail.
baseline = file("lint-baseline.xml")
}

Expand Down
132 changes: 0 additions & 132 deletions auth/lint-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,138 +12,6 @@
column="17"/>
</issue>

<issue
id="NonObservableLocale"
message="Reading locale in a non-observable way in a composable function"
errorLine1=" java.util.Locale.getDefault()"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentDefaults.kt"
line="399"
column="29"/>
</issue>

<issue
id="ComposableLambdaParameterPosition"
message="Composable lambda parameter should be the last parameter so it can be used as a trailing lambda"
errorLine1=" customLayout: (@Composable (List&lt;AuthProvider>, (AuthProvider) -> Unit) -> Unit)? = null,"
errorLine2=" ~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/method_picker/AuthMethodPicker.kt"
line="108"
column="5"/>
</issue>

<issue
id="ComposableLambdaParameterPosition"
message="Composable lambda parameter should be the last parameter so it can be used as a trailing lambda"
errorLine1=" content: (@Composable (EmailAuthContentState) -> Unit)?,"
errorLine2=" ~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthDestinations.kt"
line="127"
column="5"/>
</issue>

<issue
id="ComposableNaming"
message="Composable functions that return Unit should start with an uppercase letter"
errorLine1=" fun capture() {"
errorLine2=" ~~~~~~~">
<location
file="src/test/java/com/firebase/ui/auth/ReauthTestRequests.kt"
line="81"
column="9"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" stateDescription = localContext.getString(R.string.fui_text_field_read_only)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/components/AuthTextField.kt"
line="141"
column="44"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" context.getString(R.string.fui_error_reauth_interrupted)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt"
line="767"
column="29"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" context.getString(R.string.fui_error_reauth_no_linked_providers)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt"
line="784"
column="33"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" context.getString(R.string.fui_error_reauth_interrupted)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt"
line="796"
column="33"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" context.getString(R.string.fui_error_reauth_incomplete)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt"
line="842"
column="37"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" context.getString(R.string.fui_loading_reauth_retrying)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt"
line="851"
column="29"/>
</issue>

<issue
id="LocalContextGetResourceValueCall"
message="Querying resource values using LocalContext.current"
errorLine1=" text = context.getString(R.string.fui_reauth_password_required_notice),"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/email/SignInUI.kt"
line="245"
column="28"/>
</issue>

<issue
id="MutableCollectionMutableState"
message="Creating a MutableState object with a mutable collection type"
errorLine1=" val enrolledFactors = remember { mutableStateOf(user.multiFactor.enrolledFactors) }"
errorLine2=" ~~~~~~~~~~~~~~">
<location
file="src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentScreen.kt"
line="159"
column="38"/>
</issue>

<issue
id="StringFormatInvalid"
message="Format string &apos;`fui_signed_in_as`&apos; is not a valid format string so it should not be passed to `String.format`"
Expand Down
6 changes: 4 additions & 2 deletions auth/src/main/java/com/firebase/ui/auth/ui/components/AuthTextField.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.stateDescription
import androidx.compose.ui.text.input.PasswordVisualTransformation
Expand Down Expand Up @@ -113,7 +114,8 @@ fun AuthTextField(
visibilityToggleModifier: Modifier = Modifier,
) {
var passwordVisible by remember { mutableStateOf(false) }
val localContext = LocalContext.current
// semantics {} is not a composable scope, so the description is resolved out here.
val readOnlyStateDescription = stringResource(R.string.fui_text_field_read_only)

// Automatically set the correct keyboard type based on validator or field type
val resolvedKeyboardOptions = remember(validator, isSecureTextField, keyboardOptions) {
Expand All @@ -138,7 +140,7 @@ fun AuthTextField(
.then(
if (readOnly) {
Modifier.semantics {
stateDescription = localContext.getString(R.string.fui_text_field_read_only)
stateDescription = readOnlyStateDescription
}
} else {
Modifier
Expand Down
4 changes: 2 additions & 2 deletions auth/src/main/java/com/firebase/ui/auth/ui/method_picker/AuthMethodPicker.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ class MethodPickerTermsConfiguration(
* @param providers The list of providers to display.
* @param logo An optional logo to display.
* @param onProviderSelected A callback when a provider is selected.
* @param customLayout An optional custom layout composable for the provider buttons.
* @param termsOfServiceUrl The URL for the Terms of Service.
* @param privacyPolicyUrl The URL for the Privacy Policy.
* @param lastSignInPreference The last sign-in preference to show a "Continue as..." button.
Expand All @@ -93,6 +92,7 @@ class MethodPickerTermsConfiguration(
* @param onContinueAsSelected A callback when the "Continue as..." button is selected, with the
* provider and saved identifier (email address). Falls back to [onProviderSelected]
* if not provided.
* @param customLayout An optional custom layout composable for the provider buttons.
*
* @since 10.0.0
*/
Expand All @@ -105,9 +105,9 @@ fun AuthMethodPicker(
termsOfServiceUrl: String? = null,
privacyPolicyUrl: String? = null,
lastSignInPreference: SignInPreferenceManager.SignInPreference? = null,
customLayout: (@Composable (List<AuthProvider>, (AuthProvider) -> Unit) -> Unit)? = null,
termsConfiguration: MethodPickerTermsConfiguration? = null,
onContinueAsSelected: ((AuthProvider, String?) -> Unit)? = null,
customLayout: (@Composable (List<AuthProvider>, (AuthProvider) -> Unit) -> Unit)? = null,
) {
val continueAsHandler: (AuthProvider, String?) -> Unit =
onContinueAsSelected ?: { provider, _ -> onProviderSelected(provider) }
Expand Down
19 changes: 14 additions & 5 deletions auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.unit.dp
import androidx.navigation3.runtime.entryProvider
Expand Down Expand Up @@ -182,6 +183,14 @@ fun FirebaseAuthScreen(
val coroutineScope = rememberCoroutineScope()
val stringProvider = remember(context) { DefaultAuthUIStringProvider(context) }

// The reauth effects below run outside composition, so they cannot call stringResource
// themselves.
val reauthInterruptedMessage = stringResource(R.string.fui_error_reauth_interrupted)
val reauthNoLinkedProvidersMessage =
stringResource(R.string.fui_error_reauth_no_linked_providers)
val reauthIncompleteMessage = stringResource(R.string.fui_error_reauth_incomplete)
val reauthRetryingMessage = stringResource(R.string.fui_loading_reauth_retrying)

val observedAuthState by remember(authUI) { authUI.authStateFlow() }
.collectAsState(initial = null as AuthState?)
val rawAuthState = observedAuthState ?: AuthState.Idle
Expand Down Expand Up @@ -764,7 +773,7 @@ fun FirebaseAuthScreen(
authUI.updateAuthState(
AuthState.Error(
AuthException.UnknownException(
context.getString(R.string.fui_error_reauth_interrupted)
reauthInterruptedMessage
)
)
)
Expand All @@ -781,7 +790,7 @@ fun FirebaseAuthScreen(
required,
AuthState.Error(
AuthException.UnknownException(
context.getString(R.string.fui_error_reauth_no_linked_providers)
reauthNoLinkedProvidersMessage
)
),
)
Expand All @@ -793,7 +802,7 @@ fun FirebaseAuthScreen(
required,
AuthState.Error(
AuthException.UnknownException(
context.getString(R.string.fui_error_reauth_interrupted)
reauthInterruptedMessage
)
),
)
Expand Down Expand Up @@ -839,7 +848,7 @@ fun FirebaseAuthScreen(
AuthState.Reauthentication.AttemptFailed(
request,
AuthException.UnknownException(
context.getString(R.string.fui_error_reauth_incomplete)
reauthIncompleteMessage
),
)
}
Expand All @@ -848,7 +857,7 @@ fun FirebaseAuthScreen(
// A Success here would claim the pending operation had already succeeded.
val terminal = if (request.hasPendingOperation) {
AuthState.Loading(
context.getString(R.string.fui_loading_reauth_retrying)
reauthRetryingMessage
)
} else {
AuthState.Success(result = null, user = request.user)
Expand Down
2 changes: 1 addition & 1 deletion auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthDestinations.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ internal fun EmailAuthStep(
context: Context,
configuration: AuthUIConfiguration,
authUI: FirebaseAuthUI,
content: (@Composable (EmailAuthContentState) -> Unit)?,
navigateToStep: (AuthRoute.Email.Step) -> Unit,
isStepBelow: (NavKey?) -> Boolean,
onCancel: () -> Unit,
Expand All @@ -136,6 +135,7 @@ internal fun EmailAuthStep(
onError: (AuthException) -> Unit = {},
/** Passed through to [EmailAuthScreen]: where a consumed notification leaves the flow. */
onNotificationConsumed: (() -> Unit)? = null,
content: (@Composable (EmailAuthContentState) -> Unit)? = null,
) {
if (!configuration.isEmailStepOffered(step)) {
LaunchedEffect(entryKey) {
Expand Down
3 changes: 2 additions & 1 deletion auth/src/main/java/com/firebase/ui/auth/ui/screens/email/SignInUI.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.heading
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.style.TextAlign
Expand Down Expand Up @@ -242,7 +243,7 @@ fun SignInUI(
modifier = Modifier
.align(Alignment.Start)
.testTag(FirebaseAuthTestTags.SignIn.REAUTH_PASSWORD_NOTICE),
text = context.getString(R.string.fui_reauth_password_required_notice),
text = stringResource(R.string.fui_reauth_password_required_notice),
style = MaterialTheme.typography.bodySmall,
)
Spacer(modifier = Modifier.height(8.dp))
Expand Down
17 changes: 13 additions & 4 deletions auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentDefaults.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalLocale
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.text.input.KeyboardType
import androidx.compose.ui.text.style.TextAlign
Expand Down Expand Up @@ -361,6 +362,15 @@ private fun EnrolledFactorItem(
enabled: Boolean,
stringProvider: AuthUIStringProvider
) {
// LocalLocale is the Activity's configured locale. Locale.getDefault() and
// intl.Locale.current both read the process global, which disagrees with it when the host
// installs a per-context locale override — rendering the date in a different language from
// every stringResource around it.
val locale = LocalLocale.current
val enrollmentDateFormat = remember(locale) {
java.text.SimpleDateFormat("MMM dd, yyyy", locale.platformLocale)
}
Comment on lines +370 to +372

@gemini-code-assist gemini-code-assist Bot Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While using LocalLocale.current is a great improvement for localization, the date format string "MMM dd, yyyy" is hardcoded. This format is not suitable for all locales (e.g., many European locales use dd.MM.yyyy).

To properly internationalize the date, you should use a locale-sensitive date format provided by java.text.DateFormat. This will ensure the date is displayed in a format that is familiar to users in different regions.

Suggested change
val enrollmentDateFormat = remember(locale) {
java.text.SimpleDateFormat("MMM dd, yyyy", locale.platformLocale)
}
val enrollmentDateFormat = remember(locale) {
java.text.DateFormat.getDateInstance(java.text.DateFormat.MEDIUM, locale.platformLocale)
}


Card(
modifier = Modifier.fillMaxWidth(),
colors = CardDefaults.cardColors(
Expand Down Expand Up @@ -394,10 +404,9 @@ private fun EnrolledFactorItem(
)
Text(
text = stringProvider.enrolledOnDateLabel(
java.text.SimpleDateFormat(
"MMM dd, yyyy",
java.util.Locale.getDefault()
).format(java.util.Date(factorInfo.enrollmentTimestamp * 1000))
enrollmentDateFormat.format(
java.util.Date(factorInfo.enrollmentTimestamp * 1000)
)
),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant
Expand Down
11 changes: 8 additions & 3 deletions auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import com.firebase.ui.auth.mfa.TotpEnrollmentHandler
import com.firebase.ui.auth.util.CountryUtils
import com.google.firebase.auth.FirebaseAuth
import com.google.firebase.auth.FirebaseUser
import com.google.firebase.auth.MultiFactorInfo
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch

Expand Down Expand Up @@ -156,7 +157,11 @@ internal fun MfaEnrollmentScreenInternal(
val isLoading = remember { mutableStateOf(false) }
val error = remember { mutableStateOf<String?>(null) }
val lastException = remember { mutableStateOf<Exception?>(null) }
val enrolledFactors = remember { mutableStateOf(user.multiFactor.enrolledFactors) }
// Snapshot the SDK's list: it hands back a mutable one, and holding that instance would
// both trip MutableCollectionMutableState and skip recomposition if it mutated in place.
val enrolledFactors = remember {
mutableStateOf<List<MultiFactorInfo>>(user.multiFactor.enrolledFactors.toList())
}

// The SMS steps read only the terms and privacy URLs off this, so a host that supplied no
// configuration gets a stand-in. Its provider is arbitrary — a configuration must declare at
Expand Down Expand Up @@ -269,7 +274,7 @@ internal fun MfaEnrollmentScreenInternal(
try {
user.multiFactor.unenroll(factorInfo).addOnCompleteListener { task ->
if (task.isSuccessful) {
enrolledFactors.value = user.multiFactor.enrolledFactors
enrolledFactors.value = user.multiFactor.enrolledFactors.toList()
error.value = null
} else {
error.value = task.exception?.message
Expand Down Expand Up @@ -361,7 +366,7 @@ internal fun MfaEnrollmentScreenInternal(
null -> throw IllegalStateException("No factor selected")
}

enrolledFactors.value = user.multiFactor.enrolledFactors
enrolledFactors.value = user.multiFactor.enrolledFactors.toList()

onComplete()
error.value = null
Expand Down
2 changes: 1 addition & 1 deletion auth/src/test/java/com/firebase/ui/auth/ReauthTestRequests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ internal class ReauthScopeProbe {

/** Call from inside a content slot. */
@Composable
fun capture() {
fun Capture() {
scope = LocalAuthFlowScope.current
}

Expand Down
2 changes: 1 addition & 1 deletion auth/src/test/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreenEmailRecoveryTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ class FirebaseAuthScreenEmailRecoveryTest {
onSignInFailure = {},
onSignInCancelled = {},
// One provider, so the sheet opens at the email step and the picker never composes.
emailContent = { probe.capture() },
emailContent = { probe.Capture() },
)
}
composeTestRule.waitForIdle()
Expand Down
Loading