Skip to content

Commit 2ddace6

Browse files
githublwmgithublwm
committed
Refactor "Add Final Intention" logic and tests for clarity.
Updated the intention logic to better handle final modifier additions for fields, variables, and parameters. Improved test coverage and added assertions for caret presence to ensure test reliability. Enhanced code readability with consistent use of `final` in method parameters and local variables.
1 parent 32696d9 commit 2ddace6

File tree

2 files changed

+57
-40
lines changed

2 files changed

+57
-40
lines changed

‎src/main/java/lwm/plugin/intention/AddFinalIntention.java‎

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,24 @@
55
import com.intellij.codeInspection.util.IntentionName;
66
import com.intellij.openapi.editor.Editor;
77
import com.intellij.openapi.project.Project;
8-
import com.intellij.psi.*;
8+
import com.intellij.psi.PsiCodeBlock;
9+
import com.intellij.psi.PsiElement;
10+
import com.intellij.psi.PsiField;
11+
import com.intellij.psi.PsiFile;
12+
import com.intellij.psi.PsiJavaFile;
13+
import com.intellij.psi.PsiLocalVariable;
14+
import com.intellij.psi.PsiMethod;
15+
import com.intellij.psi.PsiModifier;
16+
import com.intellij.psi.PsiModifierList;
17+
import com.intellij.psi.PsiModifierListOwner;
18+
import com.intellij.psi.PsiParameter;
19+
import com.intellij.psi.PsiParameterList;
20+
import com.intellij.psi.PsiVariable;
921
import com.intellij.psi.util.PsiTreeUtil;
1022
import com.intellij.psi.util.PsiUtil;
1123
import com.intellij.util.IncorrectOperationException;
1224
import org.jetbrains.annotations.NotNull;
1325

14-
import java.util.Arrays;
1526
import java.util.Collection;
1627

1728
/**
@@ -29,15 +40,18 @@ public class AddFinalIntention implements IntentionAction {
2940
}
3041

3142
@Override
32-
public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
43+
public boolean isAvailable(@NotNull final Project project, final Editor editor, final PsiFile file) {
3344
if (!(file instanceof PsiJavaFile)) {
3445
return false;
3546
}
3647
final PsiElement element = file.findElementAt(editor.getCaretModel().getOffset());
3748
if (element == null) {
3849
return false;
3950
}
40-
51+
final PsiField field = PsiTreeUtil.getParentOfType(element, PsiField.class, false);
52+
if (field != null) {
53+
return true;
54+
}
4155
if (element instanceof PsiVariable) {
4256
return true;
4357
}
@@ -46,40 +60,40 @@ public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file
4660
}
4761

4862
@Override
49-
public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException {
63+
public void invoke(@NotNull final Project project, final Editor editor, final PsiFile file) throws IncorrectOperationException {
5064
final PsiElement element = file.findElementAt(editor.getCaretModel().getOffset());
5165
if (element == null) {
5266
return;
5367
}
5468

55-
PsiVariable specificVariable = PsiTreeUtil.getParentOfType(element, PsiVariable.class, false);
69+
final PsiVariable specificVariable = PsiTreeUtil.getParentOfType(element, PsiVariable.class, false);
5670
if (specificVariable != null) {
5771
addFinalModifierIfNotPresent(specificVariable);
58-
return;
72+
return;
5973
}
6074

6175
final PsiMethod containingMethod = PsiTreeUtil.getParentOfType(element, PsiMethod.class);
6276
if (containingMethod != null) {
6377
// Add final to parameters
6478
final PsiParameterList parameterList = containingMethod.getParameterList();
65-
for (PsiParameter parameter : parameterList.getParameters()) {
79+
for (final PsiParameter parameter : parameterList.getParameters()) {
6680
addFinalModifierIfNotPresent(parameter);
6781
}
6882

6983
// Add final to local variables
7084
final PsiCodeBlock methodBody = containingMethod.getBody();
7185
if (methodBody != null) {
72-
Collection<PsiLocalVariable> localVariables = PsiTreeUtil.collectElementsOfType(methodBody, PsiLocalVariable.class);
73-
for (PsiLocalVariable localVariable : localVariables) {
86+
final Collection<PsiLocalVariable> localVariables = PsiTreeUtil.collectElementsOfType(methodBody, PsiLocalVariable.class);
87+
for (final PsiLocalVariable localVariable : localVariables) {
7488
addFinalModifierIfNotPresent(localVariable);
7589
}
7690
}
7791
}
7892
}
7993

80-
private void addFinalModifierIfNotPresent(PsiModifierListOwner element) {
94+
private void addFinalModifierIfNotPresent(final PsiModifierListOwner element) {
8195
if (element != null) {
82-
PsiModifierList modifierList = element.getModifierList();
96+
final PsiModifierList modifierList = element.getModifierList();
8397
if (modifierList != null && !modifierList.hasExplicitModifier(PsiModifier.FINAL)) {
8498
PsiUtil.setModifierProperty(element, PsiModifier.FINAL, true);
8599
}

‎src/test/java/lwm/plugin/intention/AddFinalIntentionTest.java‎

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,29 @@ public class AddFinalIntentionTest extends LightJavaCodeInsightFixtureTestCase {
99

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

12-
private void doTest(String before, String after) {
13-
myFixture.configureByText("Test.java", before.replace("<caret>", "<caret>")); // Ensure caret is processed
12+
private void doTest(final String before, final String after) {
13+
assertTrue("Test code must contain <caret>", before.contains("<caret>"));
14+
myFixture.configureByText("Test.java", before);
1415
final IntentionAction intention = myFixture.findSingleIntention(INTENTION_TEXT);
1516
assertNotNull("Intention '" + INTENTION_TEXT + "' not found", intention);
1617
myFixture.launchAction(intention);
1718
myFixture.checkResult(after);
1819
}
1920

20-
private void doTestNotAvailable(String content) {
21-
myFixture.configureByText("Test.java", content.replace("<caret>", "<caret>")); // Ensure caret is processed
22-
List<IntentionAction> intentions = myFixture.getAvailableIntentions();
23-
List<String> intentionTexts = intentions.stream()
24-
.map(IntentionAction::getText)
25-
.collect(Collectors.toList());
21+
private void doTestNotAvailable(final String content) {
22+
assertTrue("Test code must contain <caret>", content.contains("<caret>"));
23+
myFixture.configureByText("Test.java", content);
24+
final List<IntentionAction> intentions = myFixture.getAvailableIntentions();
25+
final List<String> intentionTexts = intentions.stream()
26+
.map(IntentionAction::getText)
27+
.collect(Collectors.toList());
2628
assertFalse("Intention '" + INTENTION_TEXT + "' should not be available. Available: " + intentionTexts,
2729
intentionTexts.contains(INTENTION_TEXT));
2830
}
29-
30-
private void doTestNoChange(String contentWithCaret) {
31-
String contentWithoutCaret = contentWithCaret.replace("<caret>", "");
31+
32+
private void doTestNoChange(final String contentWithCaret) {
33+
assertTrue("Test code must contain <caret>", contentWithCaret.contains("<caret>"));
34+
final String contentWithoutCaret = contentWithCaret.replace("<caret>", "");
3235
myFixture.configureByText("Test.java", contentWithCaret);
3336
// Attempt to find the intention. It should be available.
3437
final IntentionAction intention = myFixture.findSingleIntention(INTENTION_TEXT);
@@ -38,23 +41,23 @@ private void doTestNoChange(String contentWithCaret) {
3841
}
3942

4043
public void testAddFinalToSingleParameter() {
41-
String before = "class Test {\n" +
44+
final String before = "class Test {\n" +
4245
" void method(String pa<caret>ram1, int param2) {}\n" +
4346
"}";
44-
String after = "class Test {\n" +
47+
final String after = "class Test {\n" +
4548
" void method(final String param1, int param2) {}\n" +
4649
"}";
4750
doTest(before, after);
4851
}
4952

5053
public void testAddFinalToSingleLocalVariable() {
51-
String before = "class Test {\n" +
54+
final String before = "class Test {\n" +
5255
" void method() {\n" +
5356
" String l<caret>ocal1 = \"hello\";\n" +
5457
" int local2 = 10;\n" +
5558
" }\n" +
5659
"}";
57-
String after = "class Test {\n" +
60+
final String after = "class Test {\n" +
5861
" void method() {\n" +
5962
" final String local1 = \"hello\";\n" +
6063
" int local2 = 10;\n" +
@@ -65,17 +68,17 @@ public void testAddFinalToSingleLocalVariable() {
6568

6669
public void testAddFinalToAllInMethodScope() {
6770
// Caret is removed in 'after' string by checkResult implicitly if not present.
68-
String before = "class Test {\n" +
71+
final String before = "class Test {\n" +
6972
" void method(String param1, int param2) {\n" +
7073
" <caret>\n" +
7174
" String local1 = \"hello\";\n" +
7275
" int local2 = 10;\n" +
7376
" final String alreadyFinal = \"done\";\n" +
7477
" }\n" +
7578
"}";
76-
String after = "class Test {\n" +
79+
final String after = "class Test {\n" +
7780
" void method(final String param1, final int param2) {\n" +
78-
" \n" +
81+
" \n" +
7982
" final String local1 = \"hello\";\n" +
8083
" final int local2 = 10;\n" +
8184
" final String alreadyFinal = \"done\";\n" +
@@ -85,52 +88,52 @@ public void testAddFinalToAllInMethodScope() {
8588
}
8689

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

9598
public void testNotAvailableOnAlreadyFinalLocalVariable() {
96-
String content = "class Test {\n" +
99+
final String content = "class Test {\n" +
97100
" void method() {\n" +
98101
" final String lo<caret>cal1 = \"hello\";\n" +
99102
" }\n" +
100103
"}";
101104
// As per current isAvailable & invoke logic, intention is available but makes no change.
102105
doTestNoChange(content);
103106
}
104-
107+
105108
public void testNotAvailableOnClassDeclaration() {
106-
String content = "class Te<caret>st {\n" +
109+
final String content = "class Te<caret>st {\n" +
107110
" void method(String param1) {}\n" +
108111
"}";
109112
doTestNotAvailable(content);
110113
}
111114

112115
public void testNotAvailableInImportStatement() {
113-
String content = "import ja<caret>va.util.List;\n" +
116+
final String content = "import ja<caret>va.util.List;\n" +
114117
"class Test {\n" +
115118
" void method(String param1) {}\n" +
116119
"}";
117120
doTestNotAvailable(content);
118121
}
119-
122+
120123
public void testNotAvailableOnFieldName() {
121124
// The current isAvailable will make it available for fields.
122125
// The invoke method will make the field final.
123-
String before = "class Test {\n" +
126+
final String before = "class Test {\n" +
124127
" String myFi<caret>eld = \"value\";\n" +
125128
"}";
126-
String after = "class Test {\n" +
129+
final String after = "class Test {\n" +
127130
" final String myField = \"value\";\n" +
128131
"}";
129132
doTest(before, after);
130133
}
131134

132135
public void testNotAvailableOnAlreadyFinalFieldName() {
133-
String content = "class Test {\n" +
136+
final String content = "class Test {\n" +
134137
" final String myFi<caret>eld = \"value\";\n" +
135138
"}";
136139
// Intention should be available (based on isAvailable) but do nothing (based on invoke)
@@ -141,6 +144,6 @@ public void testNotAvailableOnAlreadyFinalFieldName() {
141144
@Override
142145
protected String getTestDataPath() {
143146
// Not using testData files for these tests, so path can be empty or root.
144-
return "";
147+
return "";
145148
}
146149
}

0 commit comments

Comments
 (0)