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
130 changes: 130 additions & 0 deletions .claude/skills/android-java-to-kotlin/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
---
name: android-java-to-kotlin
description: >
Use when finishing a Java-to-Kotlin conversion in an Android project, when the user
mentions "java to kotlin", "j2k", "convert java", "migrate java to kotlin", "finish the
conversion", "make it idiomatic", or when a freshly IDE-converted .kt file needs to be
turned into clean, modern, idiomatic Kotlin. The developer first runs the IDE converter
(Android Studio: Code > Convert Java File to Kotlin File), then this skill drives the
second pass: idiomatic cleanup, fail-fast control flow, coroutines/lifecycleScope,
modern Android APIs, function decomposition, and a behaviour-locking test.
license: AGPL-3.0-or-later
SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
SPDX-License-Identifier: AGPL-3.0-or-later
metadata:
author: Nextcloud Android
version: "1.0.0"
---

# Android Java to Kotlin Conversion (Second Pass)

The IDE converter produces Kotlin that *compiles* but is not *idiomatic*: platform types
everywhere, one giant function per lifecycle callback, nested `if`/`else`, `new Thread`,
`switch`, `TextUtils.isEmpty`, magic numbers. This skill drives the disciplined second
pass that turns that output into clean, modern, testable Kotlin — **without changing
observable behaviour** — and then writes a test that proves behaviour is unchanged.

## The Two-Person Workflow

This skill assumes a hand-off:

1. **Developer** runs the mechanical IDE conversion (`Code > Convert Java File to Kotlin
File`, or ⌥⇧⌘K), producing a `.kt` file that compiles but is not idiomatic.
2. **You (Claude)** drive everything after that:

Step 0 Baseline → Step 1 Idiomatic pass → Step 2 Behaviour-locking test → Step 3
Verify. If Step 3 fails or behaviour drifts, loop back to Step 1.

If you are handed a `.java` file instead, first apply a faithful 1:1 translation to reach
the same starting point, then continue.

## The Prime Directive: Behaviour Must Not Change

Every transformation in this skill is **behaviour-preserving**. You are refactoring for
readability, safety, and modern API usage — not adding features or fixing bugs. If you
spot a real bug, surface it to the developer; do not silently "fix" it inside a
conversion. The behaviour-locking test in Step 3 exists to keep you honest.

## Step 0: Establish Baseline

Before editing anything:

1. Read the whole `.kt` file (and, if you can, the original `.java` via
`git show <rev>:<path>.java` or the IDE's local history) to understand *what it does*.
2. Write down the **public/observable surface** you must keep intact:
- Lifecycle callbacks (`onCreate`, `onViewCreated`, `onSaveInstanceState`, …) and their
ordering of side-effects.
- Any Parcelable/Bundle keys, intent extras, and `newInstance(...)` factory shapes.
3. Note threading: which work runs off the main thread today (`Thread`, `AsyncTask`,
`runOnUiThread`, executors) — this maps to coroutines in Step 2.

## Step 1: Idiomatic Pass

Apply, in this order, then re-check the invariants:

1. **Kill platform types.** Give every `!` platform type an explicit nullable/non-null
type based on the Java source and call sites.
2. **Fail fast.** Replace nested `if`/`else` pyramids and null-checks with guard clauses
and `require`/`requireNotNull`/`?: return`. See [FAIL-FAST.md](references/FAIL-FAST.md).
3. **Decompose.** Break each oversized lifecycle callback / `setupView`-style method into
small, single-purpose private functions named for their intent. See
[ANDROID-IDIOMS.md](references/ANDROID-IDIOMS.md).
4. **Modern concurrency.** Replace `new Thread`, `AsyncTask`, `runOnUiThread`, and
callback pairs with `lifecycleScope` + `suspend` + `withContext`. See
[CONCURRENCY.md](references/CONCURRENCY.md).
5. **Modern Android + Kotlin idioms.** Scope functions (`run`/`apply`/`let`), extension
functions, `when`/`partition`/`filter` over `switch`, string templates, `isNullOrEmpty`,
view/KTX extensions. See [ANDROID-IDIOMS.md](references/ANDROID-IDIOMS.md). Retire
deprecated Android APIs (`onActivityResult`, options-menu overrides,
`java.util.Observable`) — see [DEPRECATED-APIS.md](references/DEPRECATED-APIS.md).
6. **Project conventions.** SPDX header, no magic numbers (`companion object` +
`const val`), resources not hardcoded strings, ≤300 lines/file, ≤120 cols. See
[PROJECT-CONVENTIONS.md](references/PROJECT-CONVENTIONS.md).
7. **Testability seams.** Extract pure logic (URL building, permission math, partitioning)
into functions you can unit-test; mark with `@VisibleForTesting` where they must stay
`internal`/`private`-ish but be reachable from tests.

Do NOT expand scope. Unrelated files stay untouched (AGENTS.md / AI policy).

## Step 2: Write a Behaviour-Locking Test (Mandatory)

A conversion is not complete until a test proves behaviour is unchanged. See
[TESTING.md](references/TESTING.md) for the decision tree. In short:

- **Pure functions you extracted** (e.g. link builders, `isReshareForbidden`,
partitioning) → fast JVM unit tests in `app/src/test/` (JUnit4 + mockito-kotlin).
- **Fragment/Activity/DB behaviour** → instrumented test in `app/src/androidTest/`
extending the project's base test class, or a Robolectric test where the project uses it.
- Prefer testing the **seams you just created**. If the Java original had no test, your
new test is the characterization test that locks current behaviour.

Every test file gets the SPDX header and follows the project's test conventions.

## Step 3: Verify

Run and report real output — never claim green without evidence:

```bash
# Formatting + static analysis on the changed files
./gradlew spotlessKotlinCheck detektGplayDebug lintGplayDebug spotbugsGplayDebug

# Unit tests
./gradlew jacocoTestGplayDebugUnitTest

# Instrumented tests (if you wrote one), scoped to the class
./gradlew createGplayDebugCoverageReport -Pcoverage=true \
-Pandroid.testInstrumentationRunnerArguments.class=<fully.qualified.TestClass>
```

## Worked Example

[assets/worked-example.md](assets/worked-example.md) is a real before/after from
`FileDetailSharingFragment` (871-line Java fragment → idiomatic Kotlin) showing each
transformation class in context. Read it when you need a concrete pattern.

## Batch Conversion

Convert one file at a time, leaf dependencies first. Report progress per file. Warn the
developer before a change set grows into several thousand lines — split into focused PRs
(AGENTS.md). Each commit needs `Assisted-by: <agent>:<model>`; only the human adds
`Signed-off-by`.
163 changes: 163 additions & 0 deletions .claude/skills/android-java-to-kotlin/assets/worked-example.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
# Worked Example: FileDetailSharingFragment

A real conversion of an 871-line Java fragment
(`com.owncloud.android.ui.fragment.FileDetailSharingFragment`) into idiomatic Kotlin. Each
section shows one transformation class from the skill, in context. Behaviour is unchanged
throughout; the only new capability (`@VisibleForTesting createInternalLink`) is a
testability seam that returns the same URL the Java produced.

## A. SPDX Header Rewrite

```diff
-/*
- * Nextcloud Android client application
- *
- * @author Andy Scherzinger
- * ...
- * Copyright (C) 2018 Andy Scherzinger
- * SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only
- */
+/*
+ * Nextcloud - Android Client
+ *
+ * SPDX-FileCopyrightText: 2026 Alper Ozturk <alper.ozturk@nextcloud.com>
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
```

## B. Fail-Fast Preconditions

```diff
-if (file == null) throw new IllegalArgumentException("File may not be null");
-if (user == null) throw new IllegalArgumentException("Account may not be null");
-fileActivity = (FileActivity) getActivity();
-if (fileActivity == null) throw new IllegalArgumentException("FileActivity may not be null");
+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" }
```

## C. Decomposition of `onViewCreated`

The Java `onViewCreated` inlined adapter creation (duplicated for internal/external),
layout managers, listeners, and the fetch kick-off. It became a readable sequence plus
extracted helpers `getUserId()`, `setupInternalShares()`, `setupExternalShares()`,
`createShareListAdapter(userId, type)`, `startAnimation()`. The two near-identical adapter
blocks collapsed into one parameterized factory:

```kotlin
private fun createShareListAdapter(userId: String, type: SharesType): ShareeListAdapter =
ShareeListAdapter(
fileActivity!!, ArrayList(), this, userId, user, viewThemeUtils,
(file?.isEncrypted == true), type
).apply { setHasStableIds(true) }
```

## D. `new Thread` + `runOnUiThread` → `lifecycleScope` + `suspend`

```diff
-private void fetchE2EECounter(Runnable onComplete) {
- new Thread(() -> {
- try { ... fileDataStorageManager.saveFile(file); }
- catch (Exception e) { Log_OC.e(TAG, "..." + e.getMessage()); }
- Activity a = getActivity();
- if (a != null) a.runOnUiThread(onComplete);
- }).start();
-}
+private suspend fun fetchE2EECounter(): Boolean = withContext(Dispatchers.IO) {
+ return@withContext try {
+ val client = clientFactory.create(user)
+ val metadata = RefreshFolderOperation.getDecryptedFolderMetadata(true, file, client, user, requireContext())
+ if (metadata is DecryptedFolderMetadataFile) {
+ file?.setE2eCounter(metadata.metadata.counter)
+ fileDataStorageManager?.saveFile(file)
+ }
+ true
+ } catch (e: Exception) {
+ Log_OC.e(TAG, "Error refreshing E2E counter: " + e.message)
+ false
+ }
+}
```

Caller now `lifecycleScope.launch { if (!fetchE2EECounter()) return@launch; withContext(Main){ ... } }`.
The callback-pair `fetchSharees(onSuccess, onError)` was likewise turned into a `suspend`
call returning `Boolean`.

## E. `switch` Bucketing → `partition` + Set

```diff
-for (OCShare share : shares) {
- if (share.getShareType() != null) {
- switch (share.getShareType()) {
- case PUBLIC_LINK: case FEDERATED_GROUP: case FEDERATED: case EMAIL:
- externalShares.add(share); break;
- default: internalShares.add(share); break;
- }
- }
-}
+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 }
```

## F. Nested `if` Cursor Handling → Guard Clauses

The three-level-nested `handleContactResult` became flat sequential guards, each showing
the snackbar + log and returning, with `cursor.close()` preserved on every path. See
[FAIL-FAST.md](../references/FAIL-FAST.md) section "Deeply Nested".

## G. Scope Functions & KTX

```diff
-final LinearLayout shimmerLayout = binding.shimmerLayout.getRoot();
-shimmerLayout.clearAnimation();
-shimmerLayout.setVisibility(View.GONE);
-binding.shareContainer.setVisibility(View.VISIBLE);
+binding?.run {
+ shimmerLayout.root.run { clearAnimation(); visibility = View.GONE }
+ shareContainer.visibility = View.VISIBLE
+}
```

```diff
-for (int i = 0; i < viewGroup.getChildCount(); i++) { toggleSearchViewEnable(viewGroup.getChildAt(i), enable); }
+for (i in 0..<view.size) { toggleSearchViewEnable(view.getChildAt(i), enable) } // androidx.core.view.size
```

## H. Constants + Companion + `@JvmStatic`

```diff
-private static final String TAG = "FileDetailSharingFragment";
-ViewExtensionsKt.setVisibleIf(binding.sharesListInternalShowAll, adapter.shares.size() > 3);
+companion object {
+ private const val TAG = "FileDetailSharingFragment"
+ private const val MIN_SHOW_ALL_VISIBLE_ITEM_COUNT = 3
+ @JvmStatic fun newInstance(file: OCFile?, user: User?) = FileDetailSharingFragment().apply { ... }
+}
+binding.sharesListInternalShowAll.setVisibleIf(internalShares.size > MIN_SHOW_ALL_VISIBLE_ITEM_COUNT)
```

## I. Testability Seam

`createInternalLink` was inlined string concatenation in Java. It was extracted to a pure
`@VisibleForTesting internal fun createInternalLink(user, file, capabilities): String` that
returns the identical URL — enabling the unit test in
[TESTING.md](../references/TESTING.md) without a device.

## J. Detekt Suppression (Last Resort)

The class is genuinely large and can't be fully split within one conversion, so a
file-level suppression documents the debt honestly:

```kotlin
@Suppress("TooManyFunctions", "LargeClass", "TooGenericExceptionCaught", "ReturnCount")
class FileDetailSharingFragment : Fragment(), ... {
```

Prefer real decomposition; use this only when splitting is out of scope, and tell the
developer.
Loading
Loading