ADFA-4867: Point Kotlin code actions at their own tooltip tags - #1582
Open
itsaky-adfa wants to merge 6 commits into
Open
ADFA-4867: Point Kotlin code actions at their own tooltip tags#1582itsaky-adfa wants to merge 6 commits into
itsaky-adfa wants to merge 6 commits into
Conversation
AddImportActionTest, added in ADFA-4747, calls computeImportCandidates, which the same commit removed. :lsp:kotlin test sources have not compiled since, so no unit test in the module could run. Extract the candidate resolution back out of execAction as an internal seam, mirroring OrganizeImportsAction.computeOrganizeEdit. Candidates are now keyed by fully-qualified name rather than JvmSymbol. postExec only ever used symbol.fqName, and keying by it collapses two index entries for the same class into a single chooser entry.
Tooltip content is authored per tag (ADFA-4730) and looked up by the literal string, so a wrong tag fails silently at runtime: the action shows another action's tooltip, or none at all. - Organize imports and Null-safety fixes carried no tag, orphaning the content authored for them. - Implement members used Java's editor.codeactions.overridesuper. - Import class(es) used editor.codeactions.kotlin.fiximports, which describes a different action, and one ADFA-4821 records as not implemented for Kotlin. It now uses .kotlin.addimport. - Comment and Uncomment are shared with Java, so the tag becomes a required constructor argument and each language passes its own. Not defaulted, so a new language cannot silently inherit another's tooltip. KotlinCodeActionTooltipTagTest pins all six id-to-tag pairs and asserts no Kotlin action borrows a Java tag. editor.codeactions.kotlin.addimport is new and has no content authored yet. Nothing here is visible in the app until ADFA-4868 is fixed: the code-actions submenu resolves actions under the wrong location, so no tooltip tag is read today.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 Walkthrough
WalkthroughKotlin code actions now use dedicated tooltip tags for commenting, imports, overrides, null-safety fixes, and organizing imports. Comment actions accept injected tags, menus wire language-specific values, ChangesKotlin code-action tagging
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ResolveReference
participant AddImportAction
participant ktSymbolIndex
participant CodeActionChooser
ResolveReference->>AddImportAction: provide diagnostic extras
AddImportAction->>ktSymbolIndex: query classifier candidates
ktSymbolIndex-->>AddImportAction: return candidates keyed by fqName
AddImportAction->>CodeActionChooser: display fqName edit choices
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.kt`:
- Around line 34-41: Document both public constructors of CommentLineAction in
lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.kt:34-41
and UncommentLineAction in
lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/UncommentLineAction.kt:34-41
with KDoc describing the comment/uncomment contract, the required per-language
tag parameter, and how that tag is used for tooltips.
In
`@lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt`:
- Around line 19-28: Update the expected map in KotlinCodeActionTooltipTagTest
to match KotlinCodeActionsMenu by adding the missing SurroundWithTryCatchAction
mapping, or restrict actualTags to the six regression-targeted actions. Ensure
the assertion compares equivalent action sets.
- Around line 3-5: Update KotlinCodeActionTooltipTagTest to replace JUnit 4
`@Test` and org.junit.Assert assertions with org.junit.jupiter.api test APIs and
com.google.truth.Truth.assertThat(). Add the required Jupiter and Truth test
dependencies to the lsp/kotlin module, preserving the existing test behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c6a38b0f-f38a-4b90-ab58-bf1e72ac509b
📒 Files selected for processing (10)
idetooltips/src/main/java/com/itsaky/androidide/idetooltips/TooltipTag.ktlsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.ktlsp/api/src/main/java/com/itsaky/androidide/lsp/actions/UncommentLineAction.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/JavaCodeActionsMenu.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionsMenu.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/AddImportAction.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/ImplementMembersAction.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/NullSafetyAction.ktlsp/kotlin/src/main/java/com/itsaky/androidide/lsp/kotlin/actions/OrganizeImportsAction.ktlsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt
Comment on lines
+34
to
+41
| class CommentLineAction( | ||
| lang: String, | ||
| private val targetFileExtensions: List<String>, | ||
| private val lineCommentToken: String, | ||
| tag: String, | ||
| ) : EditorActionItem { | ||
| constructor(lang: String, extension: String, lineCommentToken: String, tag: String) : | ||
| this(lang, listOf(extension), lineCommentToken, tag) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Document the new required per-language tooltip-tag API.
Both public action constructors gained a mandatory tag parameter, so their KDoc should define its per-language requirement and tooltip behavior.
lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.kt#L34-L41: document the comment action contract and requiredtag.lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/UncommentLineAction.kt#L34-L41: document the uncomment action contract and requiredtag.
📍 Affects 2 files
lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.kt#L34-L41(this comment)lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/UncommentLineAction.kt#L34-L41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.kt`
around lines 34 - 41, Document both public constructors of CommentLineAction in
lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/CommentLineAction.kt:34-41
and UncommentLineAction in
lsp/api/src/main/java/com/itsaky/androidide/lsp/actions/UncommentLineAction.kt:34-41
with KDoc describing the comment/uncomment contract, the required per-language
tag parameter, and how that tag is used for tooltips.
Source: Coding guidelines
Comment on lines
+3
to
+5
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Test |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'junit|truth' lsp/kotlin/build.gradle.kts gradle/libs.versions.tomlRepository: appdevforall/CodeOnTheGo
Length of output: 885
🏁 Script executed:
#!/bin/bash
set -e
echo "== candidate files =="
fd -a 'KotlinCodeActionTooltipTagTest.kt|lsp/kotlin/build.gradle.kts|gradle/libs.versions.toml' . | sed 's#^\./##'
echo "== target test file outline/size/content =="
wc -l lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt
sed -n '1,90p' lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt
echo "== lsp/kotlin build relevant deps/tests =="
sed -n '1,220p' lsp/kotlin/build.gradle.kts | nl -ba
echo "== repository junit/truth usage summary =="
python3 - <<'PY'
from pathlib import Path
import re
repo = next(p for p in [Path('.'), Path('/tmp/repo')] if p.exists())
files=[]
for p in Path('.').rglob('*'):
if p.is_file():
try:
text=p.read_text(errors='ignore')
except Exception:
continue
if re.search(r'import\s+(org\.junit\.jupiter|org\.junit\.Assert|org\.junit\.Test|org\.junit\.Assertions|org\.junit\.Assertions\.|com\.google\.truth|android\.test\.ext\.junit)', text):
files.append((str(p), text.count('import '), text.count('assertThat'), text.count('assertEquals'), '`@Test`'))
for fn,count_imp,count_t,eq,test in sorted(files):
if count_imp:
print(f"{fn}: imports={count_imp} truth_assert={count_t} junit_eq={eq} junit_test={test}")
PY
echo "== exact imports/usages in target file =="
rg -n 'import\s+(org\.junit|com\.google\.truth|androidx\.test\.ext\.junit)|`@Test`|assertEquals|assertTrue|assertThat' lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.ktRepository: appdevforall/CodeOnTheGo
Length of output: 2247
🏁 Script executed:
#!/bin/bash
set -e
echo "== lsp/kotlin/build.gradle.kts =="
sed -n '1,220p' lsp/kotlin/build.gradle.kts
echo "== gradle libs versions relevant entries =="
sed -n '1,45p' gradle/libs.versions.toml
sed -n '268,278p' gradle/libs.versions.toml
echo "== all junit 4 vs Jupiter usage in lsp/kotlin src/test =="
python3 - <<'PY'
from pathlib import Path
for p in Path('lsp/kotlin/src/test/java').rglob('*'):
if p.suffix not in {'.kt','.java'}:
continue
txt = p.read_text(errors='ignore')
legacy = '`@Test`' in txt and 'junit.jupiter' not in txt and 'import org.junit.Assert' in txt
jupiter = 'org.junit.jupiter' in txt
truth = 'com.google.truth' in txt or 'Truth.assertThat' in txt
if legacy or jupiter or truth:
print(f"{p}: legacy_junit4_asserts={legacy} jupiter_import={jupiter} truth_import={truth}")
PYRepository: appdevforall/CodeOnTheGo
Length of output: 8046
Use the mandated Jupiter and Truth test APIs.
lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTagTest.kt still uses JUnit 4 @Test and org.junit.Assert assertions. Add Jupiter/Truth test dependencies for this module and rewrite the tests to use org.junit.jupiter.api plus com.google.truth.Truth.assertThat().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/KotlinCodeActionTooltipTagTest.kt`
around lines 3 - 5, Update KotlinCodeActionTooltipTagTest to replace JUnit 4
`@Test` and org.junit.Assert assertions with org.junit.jupiter.api test APIs and
com.google.truth.Truth.assertThat(). Add the required Jupiter and Truth test
dependencies to the lsp/kotlin module, preserving the existing test behavior.
Source: Coding guidelines
Two changes. The Import class(es) tag becomes editor.codeactions.kotlin.importclasses, matching the label users actually see. addimport described the class name in our code rather than the user-facing concept, and Elissa names tags by the latter. Action ids move into each action's companion object as a single definition and are reused from there, following the pattern already in FieldBasedAction. The tooltip-tag test was the second place these ids were spelled out; it now reads the companion ids and the TooltipTag constants instead of literals. CommentLineAction and UncommentLineAction take a companion idFor(lang) rather than a const, since one instance is registered per language. KT_LANG in KotlinCodeActionsMenu becomes internal so the test uses the same value the menu does. Also adds the Surround with try/catch entry to the test, which ADFA-4613 registered after this branch was cut.
…uper Implement members only inserts stubs for inherited members whose effective modality is ABSTRACT (AbstractMemberStubs.kt, membersToImplement). It cannot override a concrete open member, so it is the equivalent of Java's "Implement abstract method(s)" widened to properties -- not of Java's "Override superclass methods", which offers any non-final inherited method (OverrideSuperclassMethodsAction.kt:141). The tag now says what the action does. editor.codeactions.kotlin.overridesuper is retired; editor.codeactions.kotlin.implementmembers is new and needs content authored. The label stays "Implement members" -- that is what IntelliJ and Android Studio call it, so it is what Kotlin developers expect. Java's ImplementAbstractMethodsAction borrows editor.codeactions.overridesuper for the same reason, so neither language had a correct tag for this. That half is tracked in ADFA-4869.
jatezzz
approved these changes
Jul 27, 2026
Her updated table on ADFA-4821 spells it editor.codeactions.kotlin.importclass, singular. We had importclasses. She owns the tag namespace, so match it. Every other Kotlin tag in that table already matches what this branch wires.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Jira: ADFA-4867
Tooltip content is authored per tag (ADFA-4730) and looked up by the literal tag string, so a wrong tag fails silently at runtime: the action shows another action's tooltip, or none at all. Three of the five Kotlin code actions on
stagewere wrong.Tag wiring
editor.codeactions.comment(shared with Java)editor.codeactions.kotlin.commenteditor.codeactions.uncomment(shared with Java)editor.codeactions.kotlin.uncommenteditor.codeactions.kotlin.fiximportseditor.codeactions.kotlin.addimporteditor.codeactions.kotlin.organizeimportseditor.codeactions.kotlin.nullsafetyfixeditor.codeactions.overridesuper(Java's)editor.codeactions.kotlin.overridesuperOrganize imports and Null-safety fixes shipped with no tag at all, orphaning content authored for them. Implement members was showing Java's "Override superclass methods" tooltip. Import class(es) pointed at a tag naming a different action, one ADFA-4821 records as not implemented for Kotlin.
CommentLineActionandUncommentLineActionlive inlsp/apiand serve both languages, so the tag is now a required constructor argument and each menu passes its own. Deliberately not defaulted, so a new language cannot silently inherit another language's tooltip.Review by commit
f79a1cec3- extractscomputeImportCandidatesas an internal seam, mirroringOrganizeImportsAction.computeOrganizeEdit.AddImportActionTest(added in ADFA-4747) calls this method, which the same commit removed, so:lsp:kotlintest sources have not compiled since 24 Jul and no unit test in the module could run. Candidates are now keyed by fully-qualified name instead ofJvmSymbol;postExeconly ever usedsymbol.fqName, and keying by it collapses two index entries for the same class into a single chooser entry.8436ce8dc- the tag wiring above, plusKotlinCodeActionTooltipTagTest.Each commit compiles on its own.
Notes for review
CommentLineAction.kt,UncommentLineAction.ktandJavaCodeActionsMenu.ktare the Spotless file-level ratchet reindenting each touched file in full. The behavioural change in each is a few lines.editor.codeactions.kotlin.addimportis new and has no content authored yet.EDITOR_TEXT_ACTIONSinstead ofEDITOR_CODE_ACTIONS, so no tooltip tag is read today. This PR makes the data correct; ADFA-4868 makes it reachable.Testing
KotlinCodeActionTooltipTagTest2/2 andAddImportActionTest5/5, confirmed from the JUnit XML.:lsp:kotlin,:lsp:java,:lsp:apiand:idetooltipscompile;spotlessCheckclean.