Skip to content

Refactor and test AddFinalIntention #1

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.

Sign up for GitHub

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

Merged
githublwm merged 2 commits into from
May 22, 2025
Merged

Refactor and test AddFinalIntention #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32696d9
Refactor and test AddFinalIntention
google-labs-jules[bot] May 22, 2025
2ddace6
Refactor "Add Final Intention" logic and tests for clarity.
githublwm May 22, 2025
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
84 changes: 62 additions & 22 deletions src/main/java/lwm/plugin/intention/AddFinalIntention.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,25 @@
import com.intellij.codeInspection.util.IntentionName;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.PsiCodeBlock;
import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiField;
import com.intellij.psi.PsiFile;
import com.intellij.psi.PsiJavaFile;
import com.intellij.psi.PsiLocalVariable;
import com.intellij.psi.PsiMethod;
import com.intellij.psi.PsiModifier;
import com.intellij.psi.PsiModifierList;
import com.intellij.psi.PsiModifierListOwner;
import com.intellij.psi.PsiParameter;
import com.intellij.psi.PsiParameterList;
import com.intellij.psi.PsiVariable;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.IncorrectOperationException;
import org.jetbrains.annotations.NotNull;

import java.util.Arrays;
import java.util.Collection;

/**
* @author longwm
Expand All @@ -28,36 +40,64 @@ public class AddFinalIntention implements IntentionAction {
}

@Override
public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
return file instanceof PsiJavaFile;
public boolean isAvailable(@NotNull final Project project, final Editor editor, final PsiFile file) {
if (!(file instanceof PsiJavaFile)) {
return false;
}
final PsiElement element = file.findElementAt(editor.getCaretModel().getOffset());
if (element == null) {
return false;
}
final PsiField field = PsiTreeUtil.getParentOfType(element, PsiField.class, false);
if (field != null) {
return true;
}
if (element instanceof PsiVariable) {
return true;
}

return PsiTreeUtil.getParentOfType(element, PsiMethod.class) != null;
}

@Override
public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException {
public void invoke(@NotNull final Project project, final Editor editor, final PsiFile file) throws IncorrectOperationException {
final PsiElement element = file.findElementAt(editor.getCaretModel().getOffset());
if (element == null) {
return;
}

final PsiMethod method = PsiTreeUtil.getParentOfType(element, PsiMethod.class);
if (method != null) {
final PsiParameterList parameterList = method.getParameterList();
Arrays.stream(parameterList.getParameters())
.forEach(psiParameter -> PsiUtil.setModifierProperty(psiParameter, PsiModifier.FINAL, true));
final PsiCodeBlock methodBody = method.getBody();
final PsiVariable specificVariable = PsiTreeUtil.getParentOfType(element, PsiVariable.class, false);
if (specificVariable != null) {
addFinalModifierIfNotPresent(specificVariable);
return;
}

final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(element, PsiMethod.class);
if (containingMethod != null) {
// Add final to parameters
final PsiParameterList parameterList = containingMethod.getParameterList();
for (final PsiParameter parameter : parameterList.getParameters()) {
addFinalModifierIfNotPresent(parameter);
}

// Add final to local variables
final PsiCodeBlock methodBody = containingMethod.getBody();
if (methodBody != null) {
final PsiStatement[] statements = methodBody.getStatements();
Arrays.stream(statements)
.filter(PsiDeclarationStatement.class::isInstance)
.map(psiStatement -> ((PsiDeclarationStatement) psiStatement).getDeclaredElements())
.flatMap(Arrays::stream)
.filter(PsiLocalVariable.class::isInstance)
.forEach(psiElement -> PsiUtil.setModifierProperty((PsiLocalVariable) psiElement, PsiModifier.FINAL, true));
final Collection<PsiLocalVariable> localVariables = PsiTreeUtil.collectElementsOfType(methodBody, PsiLocalVariable.class);
for (final PsiLocalVariable localVariable : localVariables) {
addFinalModifierIfNotPresent(localVariable);
}
}
}
}

final PsiVariable psiVariable = PsiTreeUtil.getParentOfType(element, PsiVariable.class);
if (psiVariable != null) {
PsiUtil.setModifierProperty(psiVariable, PsiModifier.FINAL, true);
private void addFinalModifierIfNotPresent(final PsiModifierListOwner element) {
if (element != null) {
final PsiModifierList modifierList = element.getModifierList();
if (modifierList != null && !modifierList.hasExplicitModifier(PsiModifier.FINAL)) {
PsiUtil.setModifierProperty(element, PsiModifier.FINAL, true);
}
}

}

@Override
Expand Down
149 changes: 149 additions & 0 deletions src/test/java/lwm/plugin/intention/AddFinalIntentionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package lwm.plugin.intention;

import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase;
import java.util.List;
import java.util.stream.Collectors;

public class AddFinalIntentionTest extends LightJavaCodeInsightFixtureTestCase {

private static final String INTENTION_TEXT = "Add final modifier(添加final修饰)";

private void doTest(final String before, final String after) {
assertTrue("Test code must contain <caret>", before.contains("<caret>"));
myFixture.configureByText("Test.java", before);
final IntentionAction intention = myFixture.findSingleIntention(INTENTION_TEXT);
assertNotNull("Intention '" + INTENTION_TEXT + "' not found", intention);
myFixture.launchAction(intention);
myFixture.checkResult(after);
}

private void doTestNotAvailable(final String content) {
assertTrue("Test code must contain <caret>", content.contains("<caret>"));
myFixture.configureByText("Test.java", content);
final List<IntentionAction> intentions = myFixture.getAvailableIntentions();
final List<String> intentionTexts = intentions.stream()
.map(IntentionAction::getText)
.collect(Collectors.toList());
assertFalse("Intention '" + INTENTION_TEXT + "' should not be available. Available: " + intentionTexts,
intentionTexts.contains(INTENTION_TEXT));
}

private void doTestNoChange(final String contentWithCaret) {
assertTrue("Test code must contain <caret>", contentWithCaret.contains("<caret>"));
final String contentWithoutCaret = contentWithCaret.replace("<caret>", "");
myFixture.configureByText("Test.java", contentWithCaret);
// Attempt to find the intention. It should be available.
final IntentionAction intention = myFixture.findSingleIntention(INTENTION_TEXT);
assertNotNull("Intention '" + INTENTION_TEXT + "' should be available but was not found.", intention);
myFixture.launchAction(intention);
myFixture.checkResult(contentWithoutCaret); // Ensure no change happened
}

public void testAddFinalToSingleParameter() {
final String before = "class Test {\n" +
" void method(String pa<caret>ram1, int param2) {}\n" +
"}";
final String after = "class Test {\n" +
" void method(final String param1, int param2) {}\n" +
"}";
doTest(before, after);
}

public void testAddFinalToSingleLocalVariable() {
final String before = "class Test {\n" +
" void method() {\n" +
" String l<caret>ocal1 = \"hello\";\n" +
" int local2 = 10;\n" +
" }\n" +
"}";
final String after = "class Test {\n" +
" void method() {\n" +
" final String local1 = \"hello\";\n" +
" int local2 = 10;\n" +
" }\n" +
"}";
doTest(before, after);
}

public void testAddFinalToAllInMethodScope() {
// Caret is removed in 'after' string by checkResult implicitly if not present.
final String before = "class Test {\n" +
" void method(String param1, int param2) {\n" +
" <caret>\n" +
" String local1 = \"hello\";\n" +
" int local2 = 10;\n" +
" final String alreadyFinal = \"done\";\n" +
" }\n" +
"}";
final String after = "class Test {\n" +
" void method(final String param1, final int param2) {\n" +
" \n" +
" final String local1 = \"hello\";\n" +
" final int local2 = 10;\n" +
" final String alreadyFinal = \"done\";\n" +
" }\n" +
"}";
doTest(before, after);
}

public void testNotAvailableOnAlreadyFinalParameter() {
final String content = "class Test {\n" +
" void method(final String pa<caret>ram1) {}\n" +
"}";
// As per current isAvailable & invoke logic, intention is available but makes no change.
doTestNoChange(content);
}

public void testNotAvailableOnAlreadyFinalLocalVariable() {
final String content = "class Test {\n" +
" void method() {\n" +
" final String lo<caret>cal1 = \"hello\";\n" +
" }\n" +
"}";
// As per current isAvailable & invoke logic, intention is available but makes no change.
doTestNoChange(content);
}

public void testNotAvailableOnClassDeclaration() {
final String content = "class Te<caret>st {\n" +
" void method(String param1) {}\n" +
"}";
doTestNotAvailable(content);
}

public void testNotAvailableInImportStatement() {
final String content = "import ja<caret>va.util.List;\n" +
"class Test {\n" +
" void method(String param1) {}\n" +
"}";
doTestNotAvailable(content);
}

public void testNotAvailableOnFieldName() {
// The current isAvailable will make it available for fields.
// The invoke method will make the field final.
final String before = "class Test {\n" +
" String myFi<caret>eld = \"value\";\n" +
"}";
final String after = "class Test {\n" +
" final String myField = \"value\";\n" +
"}";
doTest(before, after);
}

public void testNotAvailableOnAlreadyFinalFieldName() {
final String content = "class Test {\n" +
" final String myFi<caret>eld = \"value\";\n" +
"}";
// Intention should be available (based on isAvailable) but do nothing (based on invoke)
doTestNoChange(content);
}


@Override
protected String getTestDataPath() {
// Not using testData files for these tests, so path can be empty or root.
return "";
}
}
Loading