Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 54
ADFA-5047: Add Java code action: extract variable#1709
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.
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
Open
Daniel-ADFA
wants to merge
8
commits into
stageChoose a base branch
from
feat/ADFA-5047-java-extract-variable
base:stage
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Uh oh!
There was an error while loading. Please reload this page.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
d72b7cb
ADFA-5047: Move the extract-variable sheet into a shared :lsp:ui module
alome007 38fbc65
ADFA-5047: Add the Java extraction plan and its rewrite
alome007 50764d5
ADFA-5047: Add the Java extract variable code action
alome007 43aa22c
ADFA-5047: Fix the extract-variable findings from review
alome007 1e3b84b
ADFA-5047: Close the second round of extract-variable review findings
alome007 48c51cf
ADFA-5047: Extract the language-agnostic refactor half into :lsp:refa…
alome007 faba19b
Merge origin/stage into feat/ADFA-5047-java-extract-variable
alome007 5f76e1c
Merge branch 'feat/ADFA-5047-refactor-core' into feat/ADFA-5047-java-…
alome007 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1 idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
173 changes: 173 additions & 0 deletions
173 lsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/ExtractVariableAction.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| package com.itsaky.androidide.lsp.java.actions | ||
| import com.itsaky.androidide.actions.ActionData | ||
| import com.itsaky.androidide.actions.requireContext | ||
| import com.itsaky.androidide.actions.requireEditor | ||
| import com.itsaky.androidide.actions.requireFile | ||
| import com.itsaky.androidide.idetooltips.TooltipTag | ||
| import com.itsaky.androidide.lsp.java.refactor.ExtractionPlan | ||
| import com.itsaky.androidide.lsp.java.refactor.JAVA_KEYWORDS | ||
| import com.itsaky.androidide.lsp.java.refactor.JAVA_NAME_MESSAGES | ||
| import com.itsaky.androidide.lsp.java.refactor.buildExtractVariableRewrite | ||
| import com.itsaky.androidide.lsp.java.refactor.buildExtractionPlan | ||
| import com.itsaky.androidide.lsp.java.refactor.candidateAndScopeFor | ||
| import com.itsaky.androidide.lsp.java.refactor.toCandidateViews | ||
| import com.itsaky.androidide.lsp.models.CodeActionItem | ||
| import com.itsaky.androidide.lsp.models.CodeActionKind | ||
| import com.itsaky.androidide.lsp.models.Command | ||
| import com.itsaky.androidide.lsp.models.DocumentChange | ||
| import com.itsaky.androidide.lsp.refactor.toTextEdit | ||
| import com.itsaky.androidide.lsp.ui.ExtractVariableSelection | ||
| import com.itsaky.androidide.lsp.ui.ExtractVariableSheet | ||
| import com.itsaky.androidide.lsp.ui.findFragmentActivity | ||
| import com.itsaky.androidide.projects.FileManager | ||
| import com.itsaky.androidide.resources.R | ||
| import com.itsaky.androidide.utils.flashError | ||
| import com.itsaky.androidide.utils.flashInfo | ||
| import org.slf4j.LoggerFactory | ||
| import java.nio.file.Path | ||
| import kotlin.coroutines.cancellation.CancellationException | ||
| /** | ||
| * Extracts the expression at the cursor, or the selected one, into a local variable. | ||
| * | ||
| * The work is split so nothing heavy touches the UI thread: [execAction] runs one attributed compile | ||
| * and returns a plain-data [ExtractionPlan] covering every candidate, then [postExec] shows the shared | ||
| * sheet and turns the user's selection into a single text edit with pure offset arithmetic. | ||
| */ | ||
| class ExtractVariableAction : BaseJavaCodeAction() { | ||
| companion object { | ||
| const val ID = "ide.editor.lsp.java.extractVariable" | ||
| private val log = LoggerFactory.getLogger(ExtractVariableAction::class.java) | ||
| } | ||
| override val titleTextRes: Int = R.string.action_extract_variable | ||
| override var tooltipTag: String = TooltipTag.EDITOR_CODE_ACTIONS_EXTRACT_VARIABLE | ||
| override val id: String = ID | ||
| override var label: String = "" | ||
| // Deciding whether anything is extractable needs an attributed compile, far too costly for | ||
| // prepare() on the UI thread. BaseJavaCodeAction's file-type and module gate is all that applies; | ||
| // the action stays visible on any Java file and reports "nothing to extract" instead, as | ||
| // OrganizeImportsAction does. | ||
| override var requiresUIThread: Boolean = false | ||
| override suspend fun execAction(data: ActionData): ExtractionPlan { | ||
| val file = data.requireFile().toPath() | ||
| val cursor = data.requireEditor().cursor | ||
| val selectionStart = minOf(cursor.left, cursor.right) | ||
| val selectionEnd = maxOf(cursor.left, cursor.right) | ||
| val version = documentVersionOf(file) | ||
| // Resolving the compiler and taking its lock can both throw, and neither is inside the planner's | ||
| // own guard. DefaultActionsRegistry catches only IllegalArgumentException and this runs on a scope | ||
| // with no exception handler, so anything else would crash the app rather than fail the action. | ||
| return runCatching { | ||
| data.requireCompiler().compile(file).get { task -> | ||
| buildExtractionPlan(task, file, selectionStart, selectionEnd, version) | ||
| } | ||
| }.getOrElse { error -> | ||
| if (error is CancellationException) throw error | ||
| log.warn("Could not analyse {} for extract variable.", file, error) | ||
| ExtractionPlan.empty() | ||
| } | ||
| } | ||
| override fun postExec( | ||
| data: ActionData, | ||
| result: Any, | ||
| ) { | ||
| super.postExec(data, result) | ||
| if (result !is ExtractionPlan) return | ||
| if (result.isEmpty) { | ||
| flashInfo(R.string.msg_extract_variable_nothing_to_extract) | ||
| return | ||
| } | ||
| val context = data.requireContext() | ||
| val activity = | ||
| context.findFragmentActivity() | ||
| ?: run { | ||
| // A wiring problem rather than a user path: the editor is always hosted by one. | ||
| log.warn("No FragmentActivity for the editor context. Cannot show the extract sheet.") | ||
| flashError(R.string.msg_cannot_perform_fix) | ||
| return | ||
| } | ||
| val shown = | ||
| ExtractVariableSheet.show( | ||
| activity, | ||
| result.toCandidateViews(context), | ||
| JAVA_KEYWORDS, | ||
| JAVA_NAME_MESSAGES, | ||
| ) { selection -> applySelection(data, result, selection) } | ||
| if (!shown) { | ||
| log.warn("Fragment manager unavailable. Cannot show the extract sheet.") | ||
| } | ||
| } | ||
| /** | ||
| * Turns the user's selection into one edit and hands it to the language client. | ||
| * | ||
| * The document version is re-read here rather than trusted from the plan: the editor stays | ||
| * reachable while the sheet is open, and applying spans computed against older text would corrupt | ||
| * the file. Refusing is always safe; the user can invoke the action again. | ||
| */ | ||
| private fun applySelection( | ||
| data: ActionData, | ||
| plan: ExtractionPlan, | ||
| selection: ExtractVariableSelection, | ||
| ) { | ||
| val file = data.requireFile().toPath() | ||
| // A plan built while the document was closed carries no version to compare, so there is nothing | ||
| // to prove the text still matches: refuse rather than apply spans on trust. | ||
| if (plan.documentVersion == null || documentVersionOf(file) != plan.documentVersion) { | ||
| flashInfo(R.string.msg_extract_variable_file_changed) | ||
| return | ||
| } | ||
| val (candidate, scope) = | ||
| plan.candidateAndScopeFor(selection) ?: run { | ||
| log.warn("Selection {} does not address the plan it came from.", selection) | ||
| flashError(R.string.msg_cannot_perform_fix) | ||
| return | ||
| } | ||
| val rewrite = | ||
| buildExtractVariableRewrite( | ||
| fileText = plan.fileText, | ||
| candidateSpan = candidate.span, | ||
| declaredType = candidate.declaredType, | ||
| scope = scope, | ||
| name = selection.name, | ||
| replaceAll = selection.replaceAll, | ||
| ) ?: run { | ||
| log.warn("Could not build an extract-variable rewrite for '{}'", candidate.label) | ||
| flashError(R.string.msg_cannot_perform_fix) | ||
| return | ||
| } | ||
| val client = | ||
| data.getLanguageClient() ?: run { | ||
| log.warn("No language client set. Cannot extract variable.") | ||
| return | ||
| } | ||
| client.performCodeAction( | ||
| CodeActionItem( | ||
| title = label, | ||
| changes = listOf(DocumentChange(file = file, edits = listOf(rewrite.toTextEdit(plan.fileText)))), | ||
| kind = CodeActionKind.QuickFix, | ||
| // The rewrite is emitted fully indented. Running google-java-format here would reformat | ||
| // the whole file into the same undo step as the extraction. | ||
| command = Command("", ""), | ||
| ), | ||
| ) | ||
| } | ||
| /** Null when the document is not open, which the confirm guard treats as unverifiable and refuses. */ | ||
| private fun documentVersionOf(path: Path): Int? = FileManager.getActiveDocument(path)?.version | ||
| } | ||
1 change: 1 addition & 0 deletions
1 lsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/JavaCodeActionsMenu.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.