ADFA-4827: Kotlin inline variable code action (K2 LSP)

app/src/main/java/com/itsaky/androidide/fragments/sheets/ProgressSheet.java

@@ -30,68 +30,89 @@
 
 public class ProgressSheet extends BaseBottomSheetFragment {
 
- private LayoutProgressSheetBinding binding;
- private String message = "";
- private String subMessage = "";
- private boolean subMessageEnabled = false;
-
- @Override
- public void onViewCreated(@NonNull View view, Bundle savedInstanceState) {
- super.onViewCreated(view, savedInstanceState);
-
- binding.message.setText(message);
-
- final var params = (ConstraintLayout.LayoutParams) binding.message.getLayoutParams();
- if (subMessageEnabled) {
- binding.subMessage.setText(subMessage);
- binding.subMessage.setVisibility(View.VISIBLE);
- params.bottomToBottom = View.NO_ID;
- } else {
- binding.subMessage.setVisibility(View.GONE);
- params.bottomToBottom = LayoutParams.PARENT_ID;
- }
- }
-
- @Nullable
- @Override
- public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container,
- @Nullable Bundle savedInstanceState
- ) {
- binding = LayoutProgressSheetBinding.inflate(LayoutInflater.from(getContext()));
- return binding.getRoot();
- }
-
- public void setSubMessageEnabled(boolean enabled) {
- this.subMessageEnabled = enabled;
- }
-
- public void setSubMessage(String msg) {
- this.subMessage = msg;
- if (isShowing()) {
- binding.subMessage.setText(msg);
- }
- }
-
- public ProgressSheet setMessage(String message) {
- this.message = message;
- if (isShowing()) {
- binding.message.setText(message);
- }
-
- return this;
- }
-
- public ProgressSheet setProgressDrawable(Drawable drawable) {
- if (isShowing()) {
- binding.progress.setIndeterminateDrawable(drawable);
- }
- return this;
- }
-
- @Override
- public void dismiss() {
- if (isShowing()) {
- super.dismiss();
- }
- }
+	private LayoutProgressSheetBinding binding;
+	private String message = "";
+	private String subMessage = "";
+	private boolean subMessageEnabled = false;
+
+	/* A dismiss that arrived before this fragment was attached, replayed in onStart. */
+	private boolean dismissPending = false;
+
+	/**
+ * {@inheritDoc}
+ *
+ * <p>
+ * A dismiss that arrives before the enqueued {@code show()} transaction has run is remembered rather than dropped: the fragment is not attached to a fragment manager yet, so dismissing now would throw, but doing nothing would leave the sheet on screen with nothing left to close it.
+ */
+	@Override
+	public void dismiss() {
+ if (!isAdded()) {
+ dismissPending = true;
+ return;
+ }
+
+ dismissPending = false;
+ super.dismiss();
+	}
+
+	@Nullable
+	@Override
+	public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container,
+ @Nullable Bundle savedInstanceState) {
+ binding = LayoutProgressSheetBinding.inflate(LayoutInflater.from(getContext()));
+ return binding.getRoot();
+	}
+
+	@Override
+	public void onStart() {
+ super.onStart();
+ if (dismissPending) {
+ dismissPending = false;
+ dismissAllowingStateLoss();
+ }
+	}
+
+	@Override
+	public void onViewCreated(@NonNull View view, Bundle savedInstanceState) {
+ super.onViewCreated(view, savedInstanceState);
+
+ binding.message.setText(message);
+
+ final var params = (ConstraintLayout.LayoutParams) binding.message.getLayoutParams();
+ if (subMessageEnabled) {
+ binding.subMessage.setText(subMessage);
+ binding.subMessage.setVisibility(View.VISIBLE);
+ params.bottomToBottom = View.NO_ID;
+ } else {
+ binding.subMessage.setVisibility(View.GONE);
+ params.bottomToBottom = LayoutParams.PARENT_ID;
+ }
+	}
+
+	public ProgressSheet setMessage(String message) {
+ this.message = message;
+ if (isShowing()) {
+ binding.message.setText(message);
+ }
+
+ return this;
+	}
+
+	public ProgressSheet setProgressDrawable(Drawable drawable) {
+ if (isShowing()) {
+ binding.progress.setIndeterminateDrawable(drawable);
+ }
+ return this;
+	}
+
+	public void setSubMessage(String msg) {
+ this.subMessage = msg;
+ if (isShowing()) {
+ binding.subMessage.setText(msg);
+ }
+	}
+
+	public void setSubMessageEnabled(boolean enabled) {
+ this.subMessageEnabled = enabled;
+	}
 }

app/src/test/java/com/itsaky/androidide/fragments/sheets/ProgressSheetDismissTest.kt

@@ -0,0 +1,84 @@
+/*
+ * This file is part of AndroidIDE.
+ *
+ * AndroidIDE is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * AndroidIDE is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with AndroidIDE. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.itsaky.androidide.fragments.sheets
+
+import android.os.Looper
+import androidx.appcompat.app.AppCompatActivity
+import com.google.common.truth.Truth.assertThat
+import com.itsaky.androidide.app.BaseApplication
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.Robolectric
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.Shadows.shadowOf
+import org.robolectric.annotation.Config
+
+/**
+ * `DialogFragment.show` only enqueues the add transaction, so a dismiss issued in the same
+ * main-thread pass lands before the sheet exists. Callers that show a progress sheet around work
+ * that can finish synchronously - `IDELanguageClientImpl.performCodeAction` - rely on that dismiss
+ * being honoured; dropping it strands the sheet on screen with nothing left to close it.
+ */
+@RunWith(RobolectricTestRunner::class)
+@Config(application = ProgressSheetDismissTest.TestApp::class)
+class ProgressSheetDismissTest {
+	open class TestApp : BaseApplication()
+
+	@Test
+	fun `dismiss issued before the show transaction runs still closes the sheet`() {
+ val activity = Robolectric.buildActivity(AppCompatActivity::class.java).setup().get()
+ val manager = activity.supportFragmentManager
+
+ val sheet = ProgressSheet()
+ sheet.isCancelable = false
+ sheet.show(manager, TAG)
+ sheet.dismiss()
+
+ shadowOf(Looper.getMainLooper()).idle()
+ manager.executePendingTransactions()
+
+ assertThat(manager.findFragmentByTag(TAG)).isNull()
+ assertThat(sheet.isShowing).isFalse()
+	}
+
+	@Test
+	fun `dismiss issued once the sheet is on screen closes it`() {
+ val activity = Robolectric.buildActivity(AppCompatActivity::class.java).setup().get()
+ val manager = activity.supportFragmentManager
+
+ val sheet = ProgressSheet()
+ sheet.isCancelable = false
+ sheet.show(manager, TAG)
+
+ shadowOf(Looper.getMainLooper()).idle()
+ manager.executePendingTransactions()
+ assertThat(sheet.isShowing).isTrue()
+
+ sheet.dismiss()
+
+ shadowOf(Looper.getMainLooper()).idle()
+ manager.executePendingTransactions()
+
+ assertThat(manager.findFragmentByTag(TAG)).isNull()
+ assertThat(sheet.isShowing).isFalse()
+	}
+
+	private companion object {
+ const val TAG = "progress_sheet_test"
+	}
+}

docs/adr/0013-refactoring-ui-lives-in-the-owning-lsp-module.md

@@ -1,4 +1,4 @@
-# 0012. Refactoring UI lives in the owning LSP module
+# 0013. Refactoring UI lives in the owning LSP module
 
 - **Status:** Proposed
 - **Date:** 2026-08-03

docs/adr/0014-refactorings-decline-rather-than-rewrite.md

@@ -1,12 +1,12 @@
-# 0013. Interactive refactorings decline rather than rewrite unselected code
+# 0014. Interactive refactorings decline rather than rewrite unselected code
 
 - **Status:** Proposed
 - **Date:** 2026-08-10
 - **Deciders:** Code On The Go team
 
 ## Context
 
-The K2 Kotlin LSP is growing a family of interactive refactorings: extract variable (ADFA-4826), extract method (ADFA-5080), inline variable (ADFA-4827), semantic rename (ADFA-4825). [ADR 0012](0012-refactoring-ui-lives-in-the-owning-lsp-module.md) settles where their UI lives and that analysis produces plain data. It says nothing about how capable they should be.
+The K2 Kotlin LSP is growing a family of interactive refactorings: extract variable (ADFA-4826), extract method (ADFA-5080), inline variable (ADFA-4827), semantic rename (ADFA-4825). [ADR 0013](0013-refactoring-ui-lives-in-the-owning-lsp-module.md) settles where their UI lives and that analysis produces plain data. It says nothing about how capable they should be.
 
 That question turns out to dominate the requirements. Designing extract method surfaced a run of cases where the transformation the user asked for cannot be performed by *moving* their code - it also needs the moved code's interior edited, or a guess about intent:
 
@@ -29,8 +29,11 @@ Concretely:
 - **Prefer a stricter rule to a cleverer one** when strictness costs capability and cleverness costs certainty. Extract method refuses a reassigned outer `var` even when the write is provably dead, because proving it needs liveness analysis.
 - **Never emit code that does not compile, and avoid emitting code that warns.** The two modifiers extract method *does* add - `suspend` and `@Composable` - are required precisely because omitting them breaks compilation.
 - **A refusal is a backlog item, not a dead end.** Where the refused case is common, file it: ADFA-5082 tracks the reassigned-`var` output.
+- **Where part of the request is sound, apply that part and say so.** A refactoring has three outcomes, not two: apply, refuse, or **apply partially**. Inline variable (ADFA-4827) is the case that needs the third - a variable whose value is reassigned partway through can be inlined at the references before the write and nowhere after it, so refusing the whole thing would discard a sound transformation of the earlier half. A partial application must report both counts and what it left behind, and it must leave the file compiling on its own, exactly as a full application does. It is not a licence to apply the doubtful part and hope.
 
-This applies to the whole refactoring family, not just extract method. Inline variable and rename inherit it.
+This applies to the whole refactoring family, not just extract method. Inline variable and rename inherit it - inline variable adding the third outcome above, rename presumed to need only the first two until its design says otherwise.
+
+**Out of scope of this decision.** Whether a refactoring may duplicate an expression that is evaluated more than once. Inline variable does, without checking for side effects (see [kotlin-inline-variable.md](../features/kotlin-inline-variable.md)): the emitted code compiles, carries no warning, and is the user's own expression unedited, so nothing above forbids it. Kotlin offers no way to prove purity, so a check would be a heuristic rather than a stricter rule, and this ADR prefers strictness to cleverness in both directions.
 
 ## Consequences
 
@@ -40,12 +43,14 @@ This applies to the whole refactoring family, not just extract method. Inline va
 - Refusal reasons are cheap to specify, cheap to test (one case each) and cheap to QA, where a clever transformation needs its own test matrix and its own failure modes.
 - The rules are stateable in a sentence each, which is what makes the feature docs reviewable by someone who has not read the implementation.
 - Excluding cases by construction keeps the analysis pass small, which matters when it runs on a phone.
+- Partial application recovers capability that an all-or-nothing rule would throw away, without weakening the compile-and-do-not-warn guarantee: the sound part is applied and the doubtful part is simply not touched.
 
 **Negative / costs**
 
 - The refactorings are visibly less capable than a desktop IDE's. Two of extract method's refusals - a reassigned outer `var` (the accumulator loop) and an enclosing `with`/`apply` receiver (pervasive in Android code) - will be hit routinely.
 - The quality of the *messages* becomes load-bearing. A generic refusal reads as a broken feature, so this decision spends translated strings: roughly seven for extract method alone.
 - Users arriving from IntelliJ will read some refusals as regressions rather than as design.
+- A partial application is harder to *report* than either other outcome, and harder to QA: the message has to convey two counts and a surviving declaration in one flash, and every "how many were left behind" case is its own test. A partial result the user misreads as a complete one is the failure mode to watch.
 - The line is a judgement, not a formalism. "Editing the interior of the moved code" is clear in the cases above but will need re-application, case by case, in each future refactoring.
 
 ## Alternatives considered
@@ -57,7 +62,8 @@ This applies to the whole refactoring family, not just extract method. Inline va
 
 ## Related
 
-- [ADR 0012](0012-refactoring-ui-lives-in-the-owning-lsp-module.md) - where refactoring UI lives; this ADR answers *how capable it is*
+- [ADR 0013](0013-refactoring-ui-lives-in-the-owning-lsp-module.md) - where refactoring UI lives; this ADR answers *how capable it is*
 - [ADR 0010](0010-navigation-resolves-via-analysis-api.md) - the K2 Analysis API as the Kotlin semantic source of truth
 - [kotlin-extract-method.md](../features/kotlin-extract-method.md) - R7 to R10 and R14 are this decision applied case by case
 - [kotlin-extract-variable.md](../features/kotlin-extract-variable.md) - the shared vocabulary and primitives
+- [kotlin-inline-variable.md](../features/kotlin-inline-variable.md) - R6 to R9 are this decision applied to a subtractive refactoring, and the origin of the third outcome

docs/adr/README.md

@@ -26,5 +26,5 @@ Format is lightweight **MADR / Nygard**: Context → Decision → Consequences 
 | [0010](0010-navigation-resolves-via-analysis-api.md) | Kotlin navigation resolves via the Analysis API, not the symbol index | Proposed |
 | [0011](0011-command-analysis-priority.md) | User-invoked commands get their own analysis priority | Proposed |
 | [0012](0012-volatile-build-metadata-out-of-abis.md) | Keep volatile build metadata out of module ABIs | Proposed |
-| [0013](0012-refactoring-ui-lives-in-the-owning-lsp-module.md) | Refactoring UI lives in the owning LSP module | Proposed |
-| [0014](0013-refactorings-decline-rather-than-rewrite.md) | Interactive refactorings decline rather than rewrite unselected code | Proposed |
+| [0013](0013-refactoring-ui-lives-in-the-owning-lsp-module.md) | Refactoring UI lives in the owning LSP module | Proposed |
+| [0014](0014-refactorings-decline-rather-than-rewrite.md) | Interactive refactorings decline rather than rewrite unselected code | Proposed |