Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
176 changes: 176 additions & 0 deletions .claude/skills/android-idioms/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
---
name: android-idioms
description: Idiomatic Kotlin for Android in this codebase — decomposing oversized lifecycle functions, scope functions (run/apply/with), when and partition instead of switch, extension functions and AndroidX KTX over verbose Java utilities, null safety instead of platform types, and companion-object constants. Use when writing any new Kotlin or converting a Java class to Kotlin.
---

<!--
~ SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
~ SPDX-License-Identifier: GPL-3.0-or-later
-->

# Android + Kotlin Idioms

The shape new Kotlin should take here, and the transformations to apply when converting
Java. Every one is behaviour-preserving. Examples are drawn from a real fragment conversion
in the wider nextcloud/android codebase — the principles transfer, but the class names
(`FileDetailSharingFragment`, `OCFile`, `fileActivity`) are from that project, not this one.

## 1. Decompose Oversized Functions

The IDE keeps the Java structure: one enormous `onViewCreated`/`setupView` that inflates,
themes, wires listeners, and kicks off loading in a single 80-line block. Split by
intent into small private functions. The lifecycle callback becomes a readable table of
contents.

```kotlin
// BEFORE: onViewCreated does everything inline (adapters, layout managers, listeners, fetch)

// AFTER
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
super.onViewCreated(view, savedInstanceState)
fileActivity ?: return
fileDataStorageManager = fileActivity?.storageManager
fileOperationsHelper = fileActivity?.fileOperationsHelper

startAnimation()
val userId = getUserId()
setupInternalShares(userId)
setupExternalShares(userId)
binding?.pickContactEmailBtn?.setOnClickListener { checkContactPermission() }
fetchSharees()
setupView()
}
```

Rules:
- One function = one reason to change. Name it for *what it accomplishes*
(`setupInternalShares`, `themeView`, `disableE2EEShareForV1`), not *how*.
- Factor duplicated blocks into a parameterized helper
(`createShareListAdapter(userId, SharesType.INTERNAL)`).
- Keep files ≤300 lines (project rule). Heavy decomposition sometimes means splitting a
god-class into collaborators — raise that with the developer rather than exceeding 300.

## 2. Scope Functions Over Repetition

Replace repeated `binding.x` / `viewThemeUtils.material.y` chains with `run`/`apply`/`with`.

```kotlin
// BEFORE
viewThemeUtils.material.themeSearchCardView(binding.searchCardWrapper);
viewThemeUtils.material.colorMaterialButtonPrimaryOutlined(binding.sendCopyBtn);
viewThemeUtils.material.colorMaterialButtonPrimaryBorderless(binding.sharesListInternalShowAll);

// AFTER
binding.run {
viewThemeUtils.material.run {
themeSearchCardView(searchCardWrapper)
colorMaterialButtonPrimaryOutlined(sendCopyBtn)
colorMaterialButtonPrimaryBorderless(sharesListInternalShowAll)
}
}
```

Use `apply {}` when configuring and returning the receiver:

```kotlin
ShareeListAdapter(fileActivity!!, ArrayList(), this, userId, user, viewThemeUtils, encrypted, type)
.apply { setHasStableIds(true) }
```

## 3. `switch` → `when` / `filter` + `partition`

Collapse a `switch` that sorts items into buckets into a declarative pipeline with a
constant `Set`.

```kotlin
// BEFORE: for-loop with switch(shareType) adding to internalShares / externalShares

// AFTER
private val externalShareTypes = setOf(
ShareType.PUBLIC_LINK, ShareType.FEDERATED_GROUP, ShareType.FEDERATED, ShareType.EMAIL
)

val (external, internal) = shares
.filter { it.shareType != null }
.partition { it.shareType in externalShareTypes }
```

## 4. Extension Functions & KTX

Import members directly and lean on AndroidX KTX instead of verbose Java utilities.

| Java / verbose | Idiomatic Kotlin |
|---|---|
| `TextUtils.isEmpty(s)` | `s.isNullOrEmpty()` |
| `BundleExtensionsKt.getParcelableArgument(b, k, T.class)` | `b.getParcelableArgument(k, T::class.java)` |
| `for (int i = 0; i < vg.getChildCount(); i++)` | `for (i in 0..<view.size)` (`androidx.core.view.size`) |
| manual getter/setter methods | Kotlin property access (`view.visibility = View.GONE`) |
| `private int x; public int getX()` (read-only to callers) | `var columnsCount = 0; private set` |
| empty override method body | `= Unit` single-expression body |
| free-standing util call | receiver extension (`externalShares.mergeDistinctByToken(publicShares)`) |

Domain-specific extensions read best as receivers on the relevant type:

```kotlin
private fun OCCapability?.isPasswordEnforced(): Boolean =
this?.filesSharingPublicPasswordEnforced?.isTrue == true &&
filesSharingPublicAskForOptionalPassword.isTrue
```

## 5. Null Safety Instead of Platform Types

The IDE leaves `!` platform types and defensive Java null-checks. Replace with `?.`,
`?:`, and Kotlin's `require`/`requireNotNull`. A nullable `binding` (cleared in
`onDestroyView`) is the canonical Android case — guard it with `?.` / `?: return`.

```kotlin
// BEFORE
if (binding == null) return;
final LinearLayout shimmer = binding.shimmerLayout.getRoot();
shimmer.clearAnimation();

// AFTER
binding?.run {
shimmerLayout.root.run {
clearAnimation()
visibility = View.GONE
}
shareContainer.visibility = View.VISIBLE
}
```

## 6. Constants & Companion Object

Move `static final` and magic literals into a `companion object`; use `const val` for
compile-time constants. Add `@JvmStatic` to factory methods still called from Java.

```kotlin
companion object {
private const val TAG = "FileDetailSharingFragment"
private const val ARG_FILE = "FILE"
private const val MIN_SHOW_ALL_VISIBLE_ITEM_COUNT = 3
private const val INTERNAL_LINK_PATH_PRETTY = "/f/"

@JvmStatic
fun newInstance(file: OCFile?, user: User?) = FileDetailSharingFragment().apply {
arguments = Bundle().apply {
putParcelable(ARG_FILE, file)
putParcelable(ARG_USER, user)
}
}
}
```

## 7. Decompose; Do Not Suppress

If a legacy god-class cannot be split within the change's scope, say so and propose the
split — do not paper over it with `@Suppress("TooManyFunctions", "LargeClass", ...)`. Those
are detekt rule names and this repository has no detekt, so the annotation suppresses
nothing; it only tells the next reader that someone knew the file was too big and left it.

## 8. `// region` Organization

For large classes, grouping members under `// region <name>` / `// endregion` (lifecycle,
private methods, overrides, companion) aids IDE folding. This is IDE structure, not a
decorative divider. Match the surrounding file's existing style; do not introduce ASCII
banner comments (`// ==== ====`), which the project forbids.
89 changes: 89 additions & 0 deletions .claude/skills/deprecated-apis/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
---
name: deprecated-apis
description: Replacements for deprecated Android APIs — the Activity Result API instead of startActivityForResult/onActivityResult, MenuProvider instead of onCreateOptionsMenu/onOptionsItemSelected, and why a behaviour-locked conversion keeps java.util.Observable rather than silently moving to StateFlow. Use when touching activity results, fragment menus, or observer callbacks.
---

<!--
~ SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
~ SPDX-License-Identifier: GPL-3.0-or-later
-->

# Retiring Deprecated Android APIs

Never introduce these deprecated APIs in new code, and replace them when you touch code
that uses them — a Java→Kotlin conversion is the right moment, since the IDE converter
leaves them untouched. Each replacement below is behaviour-preserving. Examples are real
conversions from the nextcloud/android client (PRs #16878, #16792), not from this repository.

## `startActivityForResult` / `onActivityResult` → Activity Result API

The request-code + `onActivityResult` protocol is deprecated. Register a launcher at
construction time and receive the result in its callback.

```kotlin
// BEFORE
startActivityForResult(action, SELECT_LOCATION_REQUEST_CODE)
override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
if (requestCode == SELECT_LOCATION_REQUEST_CODE && data != null) { handle(data) }
}

// AFTER
private val folderPickerLauncher = registerForActivityResult(
ActivityResultContracts.StartActivityForResult()
) { result ->
if (result.resultCode == Activity.RESULT_OK) {
handle(result.data)
}
}

// launch it:
folderPickerLauncher.launch(intent)
```

Type-safe, no manual request-code bookkeeping, and it survives process death because
registration is declarative. Register during initialization (a field initializer or
`onCreate`/`onViewCreated`) — never inside a click handler, or the registration is lost.

## `onCreateOptionsMenu` / `onOptionsItemSelected` → `MenuProvider`

`setHasOptionsMenu(true)` plus the two menu overrides are deprecated on `Fragment`. Add a
`MenuProvider` bound to the view lifecycle instead.

```kotlin
// AFTER
val menuHost: MenuHost = requireActivity()
menuHost.addMenuProvider(object : MenuProvider {
override fun onCreateMenu(menu: Menu, inflater: MenuInflater) =
inflater.inflate(R.menu.gallery_menu, menu)

override fun onMenuItemSelected(item: MenuItem): Boolean = when (item.itemId) {
R.id.action_select_all -> { selectAll(); true }
else -> false
}
}, viewLifecycleOwner, Lifecycle.State.RESUMED)
```

Passing `viewLifecycleOwner` + `Lifecycle.State.RESUMED` auto-adds and removes the menu as
the view's lifecycle changes — no leak, no manual `setHasOptionsMenu`. (PR #16878)

## `java.util.Observable` — Keep or Migrate?

`java.util.Observable` / `Observer` are deprecated (Java 9+). But a conversion is
behaviour-locked, so **do not** silently swap the notification mechanism — existing Java
observers rely on `setChanged()` / `notifyObservers()`. PR #16792 deliberately KEPT it:

```kotlin
class UploadsStorageManager(...) : Observable() {
fun notifyObserversNow() {
Handler(Looper.getMainLooper()).post {
setChanged()
notifyObservers()
}
}
}
```

Migrating to `StateFlow` / `SharedFlow` changes the observation contract and every call
site — that is a separate, opt-in refactor, not part of a 1:1 conversion. Note the
deprecation, propose the flow migration as a follow-up, and keep the current mechanism
unless the developer scopes the larger change.
120 changes: 120 additions & 0 deletions .claude/skills/fail-fast/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
---
name: fail-fast
description: Guard clauses instead of nested if/else in Kotlin — require/requireNotNull/check matched to the original exception type, flattening nested pyramids into sequential early returns, "?: return" chains for nullables, resource cleanup on every early-return path, and when not to invert a branch. Use for any code with preconditions, nullable values, or nested conditionals.
---

<!--
~ SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
~ SPDX-License-Identifier: GPL-3.0-or-later
-->

# Fail Fast: Guard Clauses Over Nested `if`/`else`

Write new conditionals this way, and invert Java's nested-`if` pyramids into it when you
touch them: guard clauses that return (or throw) early, leaving the happy path at the lowest
indentation. Behaviour is identical — the branches are the same, only the shape changes.
Examples come from the wider nextcloud/android codebase; the class names are not from this
repository.

## Precondition Checks → `require` / `requireNotNull`

`requireNotNull` returns the smart-cast non-null value AND throws
`IllegalArgumentException` with the message — exactly matching the Java `if (x == null)
throw new IllegalArgumentException(...)`.

```kotlin
// BEFORE
if (file == null) throw IllegalArgumentException("File may not be null");
if (user == null) throw IllegalArgumentException("Account may not be null");
fileActivity = (FileActivity) getActivity();
if (fileActivity == null) throw IllegalArgumentException("FileActivity may not be null");

// AFTER
fileActivity = activity as? FileActivity
requireNotNull(file) { "File may not be null" }
requireNotNull(user) { "Account may not be null" }
requireNotNull(fileActivity) { "FileActivity may not be null" }
```

Use `require(condition) { msg }` for boolean preconditions:

```kotlin
require(activity is FileActivity) { "Calling activity must be of type FileActivity" }
```

`check`/`checkNotNull` are the `IllegalStateException` equivalents — use them when the Java
threw `IllegalStateException`. Match the original exception type; that is observable
behaviour.

## Early Return Over Nested Success Path

```kotlin
// BEFORE
private void checkShareViaUser() {
if (!MDMConfig.INSTANCE.shareViaUser(requireContext())) {
binding.searchContainer.setVisibility(View.GONE);
}
}

// AFTER
private fun checkShareViaUser() {
if (shareViaUser(requireContext())) return
binding?.searchContainer?.visibility = View.GONE
}
```

## Deeply Nested `if`/`else` → Sequential Guards

The most valuable transformation. A cursor-handling method nested three levels deep
becomes a flat sequence of guard clauses, each handling one failure and returning.

```kotlin
// BEFORE: if (cursor != null) { if (moveToFirst()) { if (columnIndex != -1) {...} else ... } else ... } else ...

// AFTER
private fun handleContactResult(contactUri: Uri) {
val cursor = fileActivity?.contentResolver?.query(contactUri, projection, null, null, null)
if (cursor == null) {
DisplayUtils.showSnackMessage(this, R.string.email_pick_failed)
Log_OC.e(TAG, "Failed to pick email address as Cursor is null.")
return
}
if (!cursor.moveToFirst()) {
DisplayUtils.showSnackMessage(this, R.string.email_pick_failed)
Log_OC.e(TAG, "Failed to pick email address as no Email found.")
return
}
val columnIndex = cursor.getColumnIndex(ContactsContract.CommonDataKinds.Email.ADDRESS)
if (columnIndex == -1) {
DisplayUtils.showSnackMessage(this, R.string.email_pick_failed)
Log_OC.e(TAG, "Failed to pick email address.")
cursor.close()
return
}
val email = cursor.getString(columnIndex)
// ... happy path at base indentation
cursor.close()
}
```

Watch the cleanup: if the Java relied on falling through to a single `cursor.close()`,
each early return must still close it (or wrap in `use {}`). Missing that changes
behaviour (resource leak) — verify it.

## Nullable-Guard Idioms

```kotlin
val activity = fileActivity ?: return
val clientRepository = activity.clientRepository ?: return
val remotePath = file?.remotePath ?: return
```

Each `?: return` collapses one Java `if (x == null) return;`. Chain them at the top of the
function so the body works with non-null smart-cast locals.

## When NOT to Invert

- Do not turn a genuine two-branch decision (both branches do real work) into a guard if
it obscures the symmetry — a `when`/`if-else` expression is clearer there.
- Do not change the *order* of side-effects while inverting; the snackbar/log calls above
must fire in the same cases as before.
Loading
Loading