Bug 570029 - [AutoRefactor immigration #54/148] [cleanup & saveaction]
Convert fields into local variables
Change-Id: I55c5971e0c4a6d9eecdebc788b116a9a9a072107
Signed-off-by: Fabrice Tiercelin <fabrice.tiercelin@yahoo.fr>
diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
index d450e50..5e45b2f 100644
--- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
+++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
@@ -123,6 +123,10 @@
public static String NullAnnotationsCleanUp_add_nonnullbydefault_annotation;
public static String NullAnnotationsCleanUp_remove_redundant_nullness_annotation;
+ public static String SingleUsedFieldCleanUp_description;
+ public static String SingleUsedFieldCleanUp_description_old_field_declaration;
+ public static String SingleUsedFieldCleanUp_description_new_local_var_declaration;
+ public static String SingleUsedFieldCleanUp_description_uses_of_the_var;
public static String BreakLoopCleanUp_description;
public static String StaticInnerClassCleanUp_description;
public static String StringBuilderCleanUp_description;
diff --git a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
index 96ec11a..6ccd3f7 100644
--- a/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
+++ b/org.eclipse.jdt.core.manipulation/common/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
@@ -105,6 +105,10 @@
NullAnnotationsCleanUp_add_nonnullbydefault_annotation=Add missing @NonNullByDefault annotation
NullAnnotationsCleanUp_remove_redundant_nullness_annotation=Remove redundant nullness annotation
+SingleUsedFieldCleanUp_description=Convert fields into local variables if the use is only local
+SingleUsedFieldCleanUp_description_old_field_declaration=Remove field to use a local variable instead
+SingleUsedFieldCleanUp_description_new_local_var_declaration=Convert field assignment into local variable declaration
+SingleUsedFieldCleanUp_description_uses_of_the_var=Convert field call into local variable call
BreakLoopCleanUp_description=Exit loop earlier
StaticInnerClassCleanUp_description=Make inner classes static where possible
StringBuilderCleanUp_description=Replace String concatenation by StringBuilder
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
index 9403727..a82bbe3 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/dom/ASTNodes.java
@@ -122,12 +122,14 @@
import org.eclipse.jdt.core.dom.StringLiteral;
import org.eclipse.jdt.core.dom.StructuralPropertyDescriptor;
import org.eclipse.jdt.core.dom.SuperConstructorInvocation;
+import org.eclipse.jdt.core.dom.SuperFieldAccess;
import org.eclipse.jdt.core.dom.SuperMethodInvocation;
import org.eclipse.jdt.core.dom.SwitchStatement;
import org.eclipse.jdt.core.dom.ThisExpression;
import org.eclipse.jdt.core.dom.ThrowStatement;
import org.eclipse.jdt.core.dom.TryStatement;
import org.eclipse.jdt.core.dom.Type;
+import org.eclipse.jdt.core.dom.TypeDeclaration;
import org.eclipse.jdt.core.dom.UnionType;
import org.eclipse.jdt.core.dom.VariableDeclaration;
import org.eclipse.jdt.core.dom.VariableDeclarationExpression;
@@ -793,6 +795,63 @@
return null;
}
+ /**
+ * Get the field simple name.
+ *
+ * @param expression The expression
+ *
+ * @return the field simple name
+ */
+ public static SimpleName getField(final Expression expression) {
+ SimpleName simpleName= as(expression, SimpleName.class);
+
+ if (simpleName != null) {
+ return simpleName;
+ }
+
+ FieldAccess fieldName= as(expression, FieldAccess.class);
+
+ if (fieldName != null) {
+ ThisExpression thisExpression= as(fieldName.getExpression(), ThisExpression.class);
+
+ if (thisExpression != null) {
+ if (thisExpression.getQualifier() == null) {
+ return fieldName.getName();
+ }
+
+ if (thisExpression.getQualifier().isSimpleName()) {
+ SimpleName qualifier= (SimpleName) thisExpression.getQualifier();
+ TypeDeclaration visitedClass= getTypedAncestor(expression, TypeDeclaration.class);
+
+ if (visitedClass != null
+ && isSameVariable(visitedClass.getName(), qualifier)) {
+ return fieldName.getName();
+ }
+ }
+ }
+ }
+
+ SuperFieldAccess superFieldAccess= as(expression, SuperFieldAccess.class);
+
+ if (superFieldAccess != null) {
+ if (superFieldAccess.getQualifier() == null) {
+ return superFieldAccess.getName();
+ }
+
+ if (superFieldAccess.getQualifier().isSimpleName()) {
+ SimpleName qualifier= (SimpleName) superFieldAccess.getQualifier();
+ TypeDeclaration visitedClass= getTypedAncestor(expression, TypeDeclaration.class);
+
+ if (visitedClass != null
+ && isSameVariable(visitedClass.getName(), qualifier)) {
+ return superFieldAccess.getName();
+ }
+ }
+ }
+
+ return null;
+ }
+
public static boolean isLiteral(Expression expression) {
int type= expression.getNodeType();
return type == ASTNode.BOOLEAN_LITERAL || type == ASTNode.CHARACTER_LITERAL || type == ASTNode.NULL_LITERAL ||
diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
index 6ede353..b8acc2b 100644
--- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
+++ b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
@@ -999,6 +999,18 @@
public static final String VARIABLE_DECLARATION_USE_TYPE_ARGUMENTS_FOR_RAW_TYPE_REFERENCES= "cleanup.use_arguments_for_raw_type_references"; //$NON-NLS-1$
/**
+ * Refactors a field into a local variable if its use is only local.
+ * <p>
+ * Possible values: {TRUE, FALSE}
+ * <p>
+ *
+ * @see CleanUpOptionsCore#TRUE
+ * @see CleanUpOptionsCore#FALSE
+ * @since 4.19
+ */
+ public static final String SINGLE_USED_FIELD= "cleanup.single_used_field"; //$NON-NLS-1$
+
+ /**
* Add a break to avoid passive for loop iterations.
* <p>
* Possible values: {TRUE, FALSE}
diff --git a/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java b/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java
index 3109dd3..dd32470 100644
--- a/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java
+++ b/org.eclipse.jdt.ui.tests/performance/org/eclipse/jdt/ui/tests/performance/views/CleanUpPerfTest.java
@@ -81,6 +81,7 @@
import org.eclipse.jdt.internal.ui.fix.LazyLogicalCleanUp;
import org.eclipse.jdt.internal.ui.fix.MapCloningCleanUp;
import org.eclipse.jdt.internal.ui.fix.MergeConditionalBlocksCleanUp;
+import org.eclipse.jdt.internal.ui.fix.SingleUsedFieldCleanUp;
import org.eclipse.jdt.internal.ui.fix.SortMembersCleanUp;
import org.eclipse.jdt.internal.ui.fix.StringCleanUp;
import org.eclipse.jdt.internal.ui.fix.SwitchExpressionsCleanUp;
@@ -241,6 +242,22 @@
}
@Test
+ public void testSingleUsedFieldCleanUp() throws Exception {
+ CleanUpRefactoring cleanUpRefactoring= new CleanUpRefactoring();
+ addAllCUs(cleanUpRefactoring, MyTestSetup.fJProject1.getChildren());
+
+ Map<String, String> node= getNullSettings();
+
+ node.put(CleanUpConstants.SINGLE_USED_FIELD, CleanUpOptions.TRUE);
+
+ storeSettings(node);
+
+ cleanUpRefactoring.addCleanUp(new SingleUsedFieldCleanUp());
+
+ doCleanUp(cleanUpRefactoring);
+ }
+
+ @Test
public void testCodeStyleCleanUp() throws Exception {
tagAsSummary("Clean Up - Code Style", Dimension.ELAPSED_PROCESS);
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
index d5b3d96..4a38b55 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTest.java
@@ -12030,6 +12030,435 @@
}
@Test
+ public void testSingleUsedFieldInInnerClass() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public class SubClass {\n" //
+ + " private int refactorField;\n" //
+ + "\n" //
+ + " public void refactorFieldInSubClass() {\n" //
+ + " this.refactorField = 123;\n" //
+ + " System.out.println(refactorField);\n" //
+ + " }\n" //
+ + " }\n"
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public class SubClass {\n" //
+ + " public void refactorFieldInSubClass() {\n" //
+ + " int refactorField = 123;\n" //
+ + " System.out.println(refactorField);\n" //
+ + " }\n" //
+ + " }\n"
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testSingleUsedFieldWithComplexUse() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.util.List;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private short refactorFieldWithComplexUse= 42;\n" //
+ + "\n" //
+ + " public void refactorFieldWithComplexUse(boolean b, List<String> texts) {\n" //
+ + " // Keep this comment\n" //
+ + " refactorFieldWithComplexUse = 123;\n" //
+ + " if (b) {\n" //
+ + " System.out.println(refactorFieldWithComplexUse);\n" //
+ + " } else {\n" //
+ + " refactorFieldWithComplexUse = 321;\n" //
+ + "\n" //
+ + " for (String text : texts) {\n" //
+ + " System.out.println(text);\n" //
+ + " System.out.println(this.refactorFieldWithComplexUse);\n" //
+ + " }\n" //
+ + " }\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.util.List;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void refactorFieldWithComplexUse(boolean b, List<String> texts) {\n" //
+ + " // Keep this comment\n" //
+ + " short refactorFieldWithComplexUse = 123;\n" //
+ + " if (b) {\n" //
+ + " System.out.println(refactorFieldWithComplexUse);\n" //
+ + " } else {\n" //
+ + " refactorFieldWithComplexUse = 321;\n" //
+ + "\n" //
+ + " for (String text : texts) {\n" //
+ + " System.out.println(text);\n" //
+ + " System.out.println(refactorFieldWithComplexUse);\n" //
+ + " }\n" //
+ + " }\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testSingleUsedFieldArray() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private int refactorArray[];\n" //
+ + "\n" //
+ + " public void refactorArray() {\n" //
+ + " // Keep this comment\n" //
+ + " this.refactorArray = new int[]{123};\n" //
+ + " System.out.println(refactorArray);\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void refactorArray() {\n" //
+ + " // Keep this comment\n" //
+ + " int refactorArray[] = new int[]{123};\n" //
+ + " System.out.println(refactorArray);\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testSingleUsedFieldInMultiFragment() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private int refactorOneFragment, severalUses;\n" //
+ + "\n" //
+ + " public void refactorOneFragment() {\n" //
+ + " // Keep this comment\n" //
+ + " refactorOneFragment = 123;\n" //
+ + " System.out.println(refactorOneFragment);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void severalUses() {\n" //
+ + " severalUses = 123;\n" //
+ + " System.out.println(severalUses);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void severalUses(int i) {\n" //
+ + " severalUses = i;\n" //
+ + " System.out.println(severalUses);\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private int severalUses;\n" //
+ + "\n" //
+ + " public void refactorOneFragment() {\n" //
+ + " // Keep this comment\n" //
+ + " int refactorOneFragment = 123;\n" //
+ + " System.out.println(refactorOneFragment);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void severalUses() {\n" //
+ + " severalUses = 123;\n" //
+ + " System.out.println(severalUses);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void severalUses(int i) {\n" //
+ + " severalUses = i;\n" //
+ + " System.out.println(severalUses);\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testSingleUsedFieldStatic() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private static long refactorStaticField;\n" //
+ + "\n" //
+ + " public void refactorStaticField() {\n" //
+ + " // Keep this comment\n" //
+ + " refactorStaticField = 123;\n" //
+ + " System.out.println(refactorStaticField);\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void refactorStaticField() {\n" //
+ + " // Keep this comment\n" //
+ + " long refactorStaticField = 123;\n" //
+ + " System.out.println(refactorStaticField);\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testSingleUsedFieldWithSameNameAsLocalVariable() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private int refactorFieldWithSameNameAsLocalVariable;\n" //
+ + "\n" //
+ + " public void refactorFieldWithSameNameAsLocalVariable() {\n" //
+ + " refactorFieldWithSameNameAsLocalVariable = 123;\n" //
+ + " System.out.println(test1.E.this.refactorFieldWithSameNameAsLocalVariable);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void methodWithLocalVariable() {\n" //
+ + " long refactorFieldWithSameNameAsLocalVariable = 123;\n" //
+ + " System.out.println(refactorFieldWithSameNameAsLocalVariable);\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void refactorFieldWithSameNameAsLocalVariable() {\n" //
+ + " int refactorFieldWithSameNameAsLocalVariable = 123;\n" //
+ + " System.out.println(refactorFieldWithSameNameAsLocalVariable);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void methodWithLocalVariable() {\n" //
+ + " long refactorFieldWithSameNameAsLocalVariable = 123;\n" //
+ + " System.out.println(refactorFieldWithSameNameAsLocalVariable);\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testSingleUsedFieldWithSameNameAsAttribute() throws Exception {
+ // Given
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String given= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " private int out;\n" //
+ + "\n" //
+ + " public void refactorFieldWithSameNameAsAttribute() {\n" //
+ + " out = 123;\n" //
+ + " System.out.println(out);\n" //
+ + " }\n" //
+ + "}\n";
+
+ String expected= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public void refactorFieldWithSameNameAsAttribute() {\n" //
+ + " int out = 123;\n" //
+ + " System.out.println(out);\n" //
+ + " }\n" //
+ + "}\n";
+
+ // When
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", given, false, null);
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ // Then
+ assertNotEquals("The class must be changed", given, expected);
+ assertGroupCategoryUsed(new ICompilationUnit[] { cu }, new HashSet<>(Arrays.asList(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration,
+ MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var)));
+ assertRefactoringResultAsExpected(new ICompilationUnit[] { cu }, new String[] { expected });
+ }
+
+ @Test
+ public void testKeepSingleUsedField() throws Exception {
+ IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
+ String sample= "" //
+ + "package test1;\n" //
+ + "\n" //
+ + "import java.util.ArrayList;\n" //
+ + "import java.util.Arrays;\n" //
+ + "import java.util.List;\n" //
+ + "\n" //
+ + "public class E {\n" //
+ + " public int doNotRefactorPublicField;\n" //
+ + " protected int doNotRefactorProtectedField;\n" //
+ + " int doNotRefactorPackageField;\n" //
+ + " private int doNotRefactorFieldsInSeveralMethods;\n" //
+ + " private int doNotRefactorFieldInOtherField;\n" //
+ + " private int oneField = doNotRefactorFieldInOtherField;\n" //
+ + " private int doNotRefactorReadFieldBeforeAssignment;\n" //
+ + " private int doNotRefactorUnusedField;\n" //
+ + " private List<String> dynamicList= new ArrayList<>(Arrays.asList(\"foo\", \"bar\"));\n" //
+ + " private boolean doNotRefactorFieldWithActiveInitializer = dynamicList.remove(\"foo\");\n" //
+ + " private Runnable doNotRefactorObject;\n" //
+ + " @Deprecated\n" //
+ + " private int doNotRefactorFieldWithAnnotation;\n" //
+ + "\n" //
+ + " public void doNotRefactorPublicField() {\n" //
+ + " doNotRefactorPublicField = 123;\n" //
+ + " System.out.println(doNotRefactorPublicField);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorProtectedField() {\n" //
+ + " doNotRefactorProtectedField = 123;\n" //
+ + " System.out.println(doNotRefactorProtectedField);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorPackageField() {\n" //
+ + " doNotRefactorPackageField = 123;\n" //
+ + " System.out.println(doNotRefactorPackageField);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldsInSeveralMethods() {\n" //
+ + " doNotRefactorFieldsInSeveralMethods = 123;\n" //
+ + " System.out.println(doNotRefactorFieldsInSeveralMethods);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldsInSeveralMethods(int i) {\n" //
+ + " doNotRefactorFieldsInSeveralMethods = i;\n" //
+ + " System.out.println(doNotRefactorFieldsInSeveralMethods);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorReadFieldBeforeAssignment() {\n" //
+ + " System.out.println(doNotRefactorReadFieldBeforeAssignment);\n" //
+ + " doNotRefactorReadFieldBeforeAssignment = 123;\n" //
+ + " System.out.println(doNotRefactorReadFieldBeforeAssignment);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldInOtherField() {\n" //
+ + " doNotRefactorFieldInOtherField = 123;\n" //
+ + " System.out.println(doNotRefactorFieldInOtherField);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldWithActiveInitializer() {\n" //
+ + " doNotRefactorFieldWithActiveInitializer = true;\n" //
+ + " System.out.println(doNotRefactorFieldWithActiveInitializer);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorObject() {\n" //
+ + " doNotRefactorObject = new Runnable() {\n" //
+ + " @Override\n" //
+ + " public void run() {\n" //
+ + " while (true) {\n" //
+ + " System.out.println(\"Don't stop me!\");\n" //
+ + " }\n" //
+ + " }\n" //
+ + " };\n" //
+ + " doNotRefactorObject.run();\n" //
+ + " }\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldWithAnnotation() {\n" //
+ + " doNotRefactorFieldWithAnnotation = 123456;\n" //
+ + " System.out.println(doNotRefactorFieldWithAnnotation);\n" //
+ + " }\n" //
+ + "\n" //
+ + " public class SubClass {\n" //
+ + " private int subClassField = 42;\n" //
+ + "\n" //
+ + " public void doNotRefactorFieldInSubClass() {\n" //
+ + " this.subClassField = 123;\n" //
+ + " System.out.println(subClassField);\n" //
+ + " }\n" //
+ + " }\n"
+ + "\n" //
+ + " public void oneMethod() {\n" //
+ + " SubClass aSubClass = new SubClass();\n" //
+ + " System.out.println(aSubClass.subClassField);\n" //
+ + " }\n" //
+ + "}\n";
+ ICompilationUnit cu= pack.createCompilationUnit("E.java", sample, false, null);
+
+ enable(CleanUpConstants.SINGLE_USED_FIELD);
+
+ assertRefactoringHasNoChange(new ICompilationUnit[] { cu });
+ }
+
+ @Test
public void testBreakLoop() throws Exception {
IPackageFragment pack= fSourceFolder.createPackageFragment("test1", false, null);
String input= "" //
diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTestCase.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTestCase.java
index 619a39b..cf46fc8 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTestCase.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpTestCase.java
@@ -213,13 +213,13 @@
create.run(new NullProgressMonitor());
Change change= create.getChange();
- Set<GroupCategory> categories= new HashSet<>();
+ Set<GroupCategory> actualCategories= new HashSet<>();
- collectGroupCategories(categories, change);
+ collectGroupCategories(actualCategories, change);
- for (GroupCategory category : categories) {
- if (!setOfExpectedGroupCategories.contains(category.getName())) {
- fail("Should have group category: " + category.getName() + ", found instead: " + categories.stream().map(e -> e.getName()).reduce("", String::concat));
+ for (GroupCategory actualCategory : actualCategories) {
+ if (!setOfExpectedGroupCategories.contains(actualCategory.getName())) {
+ fail("Unexpected group category: " + actualCategory.getName() + ", should find: " + String.join(", ", setOfExpectedGroupCategories));
}
}
}
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java
index 1fa8f7b..46ebc74 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstantsOptions.java
@@ -25,9 +25,7 @@
import org.eclipse.jdt.internal.ui.fix.UnimplementedCodeCleanUp;
public class CleanUpConstantsOptions extends CleanUpConstants {
-
private static void setEclipseDefaultSettings(CleanUpOptions options) {
-
//Member Accesses
options.setOption(MEMBER_ACCESSES_NON_STATIC_FIELD_USE_THIS, CleanUpOptions.FALSE);
options.setOption(MEMBER_ACCESSES_NON_STATIC_FIELD_USE_THIS_ALWAYS, CleanUpOptions.FALSE);
@@ -76,6 +74,7 @@
options.setOption(PREFER_BOOLEAN_LITERAL, CleanUpOptions.FALSE);
// Optimization
+ options.setOption(SINGLE_USED_FIELD, CleanUpOptions.FALSE);
options.setOption(BREAK_LOOP, CleanUpOptions.FALSE);
options.setOption(STATIC_INNER_CLASS, CleanUpOptions.FALSE);
options.setOption(STRINGBUILDER, CleanUpOptions.FALSE);
@@ -222,6 +221,7 @@
options.setOption(PREFER_BOOLEAN_LITERAL, CleanUpOptions.FALSE);
// Optimization
+ options.setOption(SINGLE_USED_FIELD, CleanUpOptions.FALSE);
options.setOption(BREAK_LOOP, CleanUpOptions.FALSE);
options.setOption(STATIC_INNER_CLASS, CleanUpOptions.FALSE);
options.setOption(STRINGBUILDER, CleanUpOptions.FALSE);
diff --git a/org.eclipse.jdt.ui/plugin.xml b/org.eclipse.jdt.ui/plugin.xml
index ea5d5bd..169ec31 100644
--- a/org.eclipse.jdt.ui/plugin.xml
+++ b/org.eclipse.jdt.ui/plugin.xml
@@ -7102,9 +7102,14 @@
runAfter="org.eclipse.jdt.ui.cleanup.pull_up_assignment">
</cleanUp>
<cleanUp
+ class="org.eclipse.jdt.internal.ui.fix.SingleUsedFieldCleanUp"
+ id="org.eclipse.jdt.ui.cleanup.single_used_field"
+ runAfter="org.eclipse.jdt.ui.cleanup.number_suffix">
+ </cleanUp>
+ <cleanUp
class="org.eclipse.jdt.internal.ui.fix.BreakLoopCleanUp"
id="org.eclipse.jdt.ui.cleanup.break_loop"
- runAfter="org.eclipse.jdt.ui.cleanup.number_suffix">
+ runAfter="org.eclipse.jdt.ui.cleanup.single_used_field">
</cleanUp>
<cleanUp
class="org.eclipse.jdt.internal.ui.fix.StaticInnerClassCleanUp"
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/HashCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/HashCleanUp.java
index 62e88f3..a683225 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/HashCleanUp.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/HashCleanUp.java
@@ -307,7 +307,7 @@
}
if ((newHash instanceof Name || newHash instanceof FieldAccess || newHash instanceof SuperFieldAccess) && data.tempValueUsed) {
- SimpleName fieldName= getField(newHash);
+ SimpleName fieldName= ASTNodes.getField(newHash);
if (fieldName != null
&& !ASTNodes.isSameVariable(data.primeId, fieldName)
@@ -324,7 +324,7 @@
TypeDeclaration topLevelClass= ASTNodes.getTypedAncestor(innerClass, TypeDeclaration.class);
if (ASTNodes.usesGivenSignature(specificMethod, Float.class.getCanonicalName(), "floatToIntBits", float.class.getSimpleName())) { //$NON-NLS-1$
- SimpleName fieldName= getField((Expression) specificMethod.arguments().get(0));
+ SimpleName fieldName= ASTNodes.getField((Expression) specificMethod.arguments().get(0));
if (fieldName != null
&& !ASTNodes.isSameVariable(fieldName, data.primeId)
@@ -341,7 +341,7 @@
|| ASTNodes.usesGivenSignature(specificMethod, Arrays.class.getCanonicalName(), HASH_CODE_METHOD, Object[].class.getCanonicalName())
|| ASTNodes.usesGivenSignature(specificMethod, Arrays.class.getCanonicalName(), HASH_CODE_METHOD, long[].class.getCanonicalName())
|| ASTNodes.usesGivenSignature(specificMethod, Arrays.class.getCanonicalName(), HASH_CODE_METHOD, short[].class.getCanonicalName())) {
- SimpleName fieldName= getField((Expression) specificMethod.arguments().get(0));
+ SimpleName fieldName= ASTNodes.getField((Expression) specificMethod.arguments().get(0));
if (fieldName != null
&& !ASTNodes.isSameVariable(fieldName, data.primeId)
@@ -405,56 +405,6 @@
return false;
}
- private SimpleName getField(final Expression expression) {
- SimpleName simpleName= ASTNodes.as(expression, SimpleName.class);
-
- if (simpleName != null) {
- return simpleName;
- }
-
- FieldAccess fieldName= ASTNodes.as(expression, FieldAccess.class);
-
- if (fieldName != null) {
- ThisExpression thisExpression= ASTNodes.as(fieldName.getExpression(), ThisExpression.class);
-
- if (thisExpression != null) {
- if (thisExpression.getQualifier() == null) {
- return fieldName.getName();
- }
-
- if (thisExpression.getQualifier().isSimpleName()) {
- SimpleName qualifier= (SimpleName) thisExpression.getQualifier();
- TypeDeclaration visitedClass= ASTNodes.getTypedAncestor(expression, TypeDeclaration.class);
-
- if (visitedClass != null
- && ASTNodes.isSameVariable(visitedClass.getName(), qualifier)) {
- return fieldName.getName();
- }
- }
- }
- }
-
- SuperFieldAccess superFieldAccess= ASTNodes.as(expression, SuperFieldAccess.class);
-
- if (superFieldAccess != null) {
- if (superFieldAccess.getQualifier() == null) {
- return superFieldAccess.getName();
- }
-
- if (superFieldAccess.getQualifier().isSimpleName()) {
- SimpleName qualifier= (SimpleName) superFieldAccess.getQualifier();
- TypeDeclaration visitedClass= ASTNodes.getTypedAncestor(expression, TypeDeclaration.class);
-
- if (visitedClass != null
- && ASTNodes.isSameVariable(visitedClass.getName(), qualifier)) {
- return superFieldAccess.getName();
- }
- }
- }
-
- return null;
- }
-
private boolean isGreatNumberValid(final CollectedData data, final CastExpression newHash) {
OrderedInfixExpression<Expression, InfixExpression> orderedBitwise= ASTNodes.orderedInfix(newHash.getExpression(), Expression.class, InfixExpression.class);
@@ -462,7 +412,7 @@
&& orderedBitwise != null
&& ASTNodes.hasType(newHash.getExpression(), long.class.getSimpleName(), double.class.getSimpleName())
&& InfixExpression.Operator.XOR.equals(orderedBitwise.getOperator())) {
- SimpleName field= getField(orderedBitwise.getFirstOperand());
+ SimpleName field= ASTNodes.getField(orderedBitwise.getFirstOperand());
InfixExpression moveExpression= orderedBitwise.getSecondOperand();
if (field != null
@@ -470,7 +420,7 @@
&& !ASTNodes.isSameVariable(field, data.primeId)
&& !ASTNodes.isSameVariable(field, data.resultId)
&& ASTNodes.hasOperator(moveExpression, InfixExpression.Operator.RIGHT_SHIFT_UNSIGNED)) {
- SimpleName againFieldName= getField(moveExpression.getLeftOperand());
+ SimpleName againFieldName= ASTNodes.getField(moveExpression.getLeftOperand());
Long hash= ASTNodes.getIntegerLiteral(moveExpression.getRightOperand());
if (Long.valueOf(32).equals(hash)
@@ -493,7 +443,7 @@
}
private boolean isBooleanValid(final CollectedData data, final ConditionalExpression newHash) {
- SimpleName booleanField= getField(newHash.getExpression());
+ SimpleName booleanField= ASTNodes.getField(newHash.getExpression());
Long hashForTrue= ASTNodes.getIntegerLiteral(newHash.getThenExpression());
Long hashForFalse= ASTNodes.getIntegerLiteral(newHash.getElseExpression());
@@ -516,7 +466,7 @@
if (orderedIsFieldNull != null
&& Arrays.asList(InfixExpression.Operator.EQUALS, InfixExpression.Operator.NOT_EQUALS).contains(orderedIsFieldNull.getOperator())) {
- SimpleName field= getField(orderedIsFieldNull.getFirstOperand());
+ SimpleName field= ASTNodes.getField(orderedIsFieldNull.getFirstOperand());
if (field != null) {
Long zero;
@@ -535,7 +485,7 @@
&& hashOnField.getExpression() != null
&& HASH_CODE_METHOD.equals(hashOnField.getName().getIdentifier())
&& (hashOnField.arguments() == null || hashOnField.arguments().isEmpty())) {
- SimpleName fieldToHash= getField(hashOnField.getExpression());
+ SimpleName fieldToHash= ASTNodes.getField(hashOnField.getExpression());
if (fieldToHash != null
&& ASTNodes.isSameVariable(field, fieldToHash)) {
@@ -550,7 +500,7 @@
}
private boolean isGivenVariable(final Expression expression, final SimpleName varId) {
- SimpleName field= getField(expression);
+ SimpleName field= ASTNodes.getField(expression);
return field != null && ASTNodes.isSameVariable(varId, field);
}
});
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/SingleUsedFieldCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/SingleUsedFieldCleanUp.java
new file mode 100644
index 0000000..c6fb298
--- /dev/null
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/SingleUsedFieldCleanUp.java
@@ -0,0 +1,439 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Fabrice TIERCELIN and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Fabrice TIERCELIN - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.internal.ui.fix;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.eclipse.core.runtime.CoreException;
+
+import org.eclipse.text.edits.TextEditGroup;
+
+import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.dom.AST;
+import org.eclipse.jdt.core.dom.ASTNode;
+import org.eclipse.jdt.core.dom.ASTVisitor;
+import org.eclipse.jdt.core.dom.Assignment;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.Dimension;
+import org.eclipse.jdt.core.dom.Expression;
+import org.eclipse.jdt.core.dom.FieldAccess;
+import org.eclipse.jdt.core.dom.FieldDeclaration;
+import org.eclipse.jdt.core.dom.IExtendedModifier;
+import org.eclipse.jdt.core.dom.MethodDeclaration;
+import org.eclipse.jdt.core.dom.Modifier;
+import org.eclipse.jdt.core.dom.QualifiedName;
+import org.eclipse.jdt.core.dom.SimpleName;
+import org.eclipse.jdt.core.dom.SingleVariableDeclaration;
+import org.eclipse.jdt.core.dom.Statement;
+import org.eclipse.jdt.core.dom.ThisExpression;
+import org.eclipse.jdt.core.dom.Type;
+import org.eclipse.jdt.core.dom.TypeDeclaration;
+import org.eclipse.jdt.core.dom.VariableDeclarationExpression;
+import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
+import org.eclipse.jdt.core.dom.VariableDeclarationStatement;
+import org.eclipse.jdt.core.dom.rewrite.ASTRewrite;
+
+import org.eclipse.jdt.internal.corext.dom.ASTNodes;
+import org.eclipse.jdt.internal.corext.fix.CleanUpConstants;
+import org.eclipse.jdt.internal.corext.fix.CompilationUnitRewriteOperationsFix;
+import org.eclipse.jdt.internal.corext.fix.CompilationUnitRewriteOperationsFix.CompilationUnitRewriteOperation;
+import org.eclipse.jdt.internal.corext.fix.LinkedProposalModel;
+import org.eclipse.jdt.internal.corext.refactoring.structure.CompilationUnitRewrite;
+
+import org.eclipse.jdt.ui.cleanup.CleanUpRequirements;
+import org.eclipse.jdt.ui.cleanup.ICleanUpFix;
+import org.eclipse.jdt.ui.text.java.IProblemLocation;
+
+/**
+ * A fix that refactors a field into a local variable if its use is only local:
+ * <ul>
+ * <li>The previous value should not be read,</li>
+ * <li>The field should be private,</li>
+ * <li>The field should not be final,</li>
+ * <li>The field should be primitive,</li>
+ * <li>The field should not have annotations.</li>
+ * </ul>
+ */
+public class SingleUsedFieldCleanUp extends AbstractMultiFix {
+ private static final class FieldUseVisitor extends ASTVisitor {
+ private final SimpleName field;
+ private final Set<SimpleName> occurrences= new LinkedHashSet<>();
+
+ private FieldUseVisitor(final SimpleName field) {
+ this.field= field;
+ }
+
+ @Override
+ public boolean visit(final SimpleName aVariable) {
+ if (field != aVariable
+ && ASTNodes.isSameVariable(field, aVariable)) {
+ occurrences.add(aVariable);
+ }
+
+ return true;
+ }
+
+ private Set<SimpleName> getOccurrences() {
+ return occurrences;
+ }
+ }
+
+ public SingleUsedFieldCleanUp() {
+ this(Collections.emptyMap());
+ }
+
+ public SingleUsedFieldCleanUp(final Map<String, String> options) {
+ super(options);
+ }
+
+ @Override
+ public CleanUpRequirements getRequirements() {
+ boolean requireAST= isEnabled(CleanUpConstants.SINGLE_USED_FIELD);
+ return new CleanUpRequirements(requireAST, false, false, null);
+ }
+
+ @Override
+ public String[] getStepDescriptions() {
+ if (isEnabled(CleanUpConstants.SINGLE_USED_FIELD)) {
+ return new String[] { MultiFixMessages.SingleUsedFieldCleanUp_description };
+ }
+
+ return new String[0];
+ }
+
+ @Override
+ public String getPreview() {
+ StringBuilder bld= new StringBuilder();
+ bld.append("public class MyClass {\n"); //$NON-NLS-1$
+
+ if (!isEnabled(CleanUpConstants.SINGLE_USED_FIELD)) {
+ bld.append(" private long singleUsedField;\n"); //$NON-NLS-1$
+ bld.append("\n"); //$NON-NLS-1$
+ }
+
+ bld.append(" public void myMethod() {\n"); //$NON-NLS-1$
+
+ if (isEnabled(CleanUpConstants.SINGLE_USED_FIELD)) {
+ bld.append(" long singleUsedField = 123;\n"); //$NON-NLS-1$
+ } else {
+ bld.append(" singleUsedField = 123;\n"); //$NON-NLS-1$
+ }
+
+ bld.append(" System.out.println(singleUsedField);\n"); //$NON-NLS-1$
+ bld.append(" }\n"); //$NON-NLS-1$
+ bld.append("}\n"); //$NON-NLS-1$
+
+ if (isEnabled(CleanUpConstants.SINGLE_USED_FIELD)) {
+ bld.append("\n\n"); //$NON-NLS-1$
+ }
+
+ return bld.toString();
+ }
+
+ @Override
+ protected ICleanUpFix createFix(final CompilationUnit unit) throws CoreException {
+ if (!isEnabled(CleanUpConstants.SINGLE_USED_FIELD)) {
+ return null;
+ }
+
+ final List<CompilationUnitRewriteOperation> rewriteOperations= new ArrayList<>();
+
+ unit.accept(new ASTVisitor() {
+ @Override
+ public boolean visit(final TypeDeclaration visited) {
+ for (FieldDeclaration field : visited.getFields()) {
+ if (!maybeReplaceFieldByLocalVariable(visited, field)) {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ private boolean maybeReplaceFieldByLocalVariable(final TypeDeclaration visited, final FieldDeclaration field) {
+ if (Modifier.isPrivate(field.getModifiers())
+ && !Modifier.isFinal(field.getModifiers())
+ && !hasAnnotation(field)
+ && field.getType().isPrimitiveType()) {
+ for (Object object : field.fragments()) {
+ VariableDeclarationFragment fragment= (VariableDeclarationFragment) object;
+
+ if (!maybeReplaceFragmentByLocalVariable(visited, field, fragment)) {
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ private boolean maybeReplaceFragmentByLocalVariable(final TypeDeclaration visited, final FieldDeclaration field,
+ final VariableDeclarationFragment fragment) {
+ if (fragment.getInitializer() != null && !ASTNodes.isPassiveWithoutFallingThrough(fragment.getInitializer())) {
+ return true;
+ }
+
+ FieldUseVisitor fieldUseVisitor= new FieldUseVisitor(fragment.getName());
+ visited.getRoot().accept(fieldUseVisitor);
+ Set<SimpleName> occurrences= fieldUseVisitor.getOccurrences();
+
+ MethodDeclaration oneMethodDeclaration= null;
+
+ for (SimpleName occurrence : occurrences) {
+ MethodDeclaration currentMethodDeclaration= ASTNodes.getTypedAncestor(occurrence, MethodDeclaration.class);
+
+ if (isVariableDeclaration(occurrence)
+ || isExternalField(occurrence)
+ || currentMethodDeclaration == null
+ || oneMethodDeclaration != null && currentMethodDeclaration != oneMethodDeclaration) {
+ return true;
+ }
+
+ oneMethodDeclaration= currentMethodDeclaration;
+ }
+
+ if (oneMethodDeclaration == null) {
+ return true;
+ }
+
+ boolean isReassigned= isAlwaysErased(occurrences);
+
+ if (isReassigned) {
+ SimpleName reassignment= findReassignment(occurrences);
+
+ if (reassignment != null) {
+ ASTNode parent= reassignment.getParent();
+
+ if (parent instanceof FieldAccess
+ && reassignment.getLocationInParent() == FieldAccess.NAME_PROPERTY
+ && parent.getLocationInParent() == Assignment.LEFT_HAND_SIDE_PROPERTY) {
+ parent= parent.getParent();
+ }
+
+ if (parent instanceof Assignment) {
+ rewriteOperations.add(new SingleUsedFieldOperation(field, fragment, reassignment, occurrences));
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ private SimpleName findReassignment(final Set<SimpleName> occurrences) {
+ for (SimpleName reassignment : occurrences) {
+ if (isReassigned(reassignment) && isReassignmentForAll(reassignment, occurrences)) {
+ return reassignment;
+ }
+ }
+
+ return null;
+ }
+
+ private boolean isReassignmentForAll(final SimpleName reassignment, final Set<SimpleName> occurrences) {
+ for (SimpleName occurrence : occurrences) {
+ if (reassignment != occurrence) {
+ Statement statement= ASTNodes.getTypedAncestor(occurrence, Statement.class);
+ boolean isReassigned= false;
+
+ while (statement != null) {
+ Assignment assignment= ASTNodes.asExpression(statement, Assignment.class);
+
+ if (assignment != null
+ && ASTNodes.hasOperator(assignment, Assignment.Operator.ASSIGN)) {
+ SimpleName field= ASTNodes.getField(assignment.getLeftHandSide());
+
+ if (field == reassignment) {
+ isReassigned= true;
+ break;
+ }
+ }
+
+ statement= ASTNodes.getPreviousStatement(statement);
+ }
+
+ if (!isReassigned) {
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ private boolean isAlwaysErased(final Set<SimpleName> occurrences) {
+ for (SimpleName occurrence : occurrences) {
+ if (!isReassigned(occurrence)) {
+ Statement statement= ASTNodes.getTypedAncestor(occurrence, Statement.class);
+ boolean isReassigned= false;
+
+ while (statement != null) {
+ statement= ASTNodes.getPreviousStatement(statement);
+ Assignment assignment= ASTNodes.asExpression(statement, Assignment.class);
+
+ if (assignment != null
+ && ASTNodes.hasOperator(assignment, Assignment.Operator.ASSIGN)) {
+ SimpleName field= ASTNodes.getField(assignment.getLeftHandSide());
+
+ if (ASTNodes.areSameVariables(field, occurrence)) {
+ isReassigned= true;
+ break;
+ }
+ }
+ }
+
+ if (!isReassigned) {
+ return false;
+ }
+ }
+ }
+
+ return true;
+ }
+
+ private boolean isReassigned(final SimpleName occurrence) {
+ Expression expression= occurrence;
+
+ if (expression.getParent() instanceof FieldAccess) {
+ expression= (FieldAccess) expression.getParent();
+ }
+
+ return expression.getParent() instanceof Assignment
+ && expression.getLocationInParent() == Assignment.LEFT_HAND_SIDE_PROPERTY
+ && ASTNodes.hasOperator((Assignment) expression.getParent(), Assignment.Operator.ASSIGN);
+ }
+
+ private boolean isExternalField(final SimpleName occurrence) {
+ FieldAccess fieldAccess= ASTNodes.as(occurrence, FieldAccess.class);
+
+ if (fieldAccess != null) {
+ ThisExpression thisExpression= ASTNodes.as(fieldAccess.getExpression(), ThisExpression.class);
+
+ if (thisExpression == null || thisExpression.getQualifier() != null) {
+ return true;
+ }
+ }
+
+ return ASTNodes.is(occurrence, QualifiedName.class);
+ }
+
+ private boolean isVariableDeclaration(final SimpleName occurrence) {
+ switch (occurrence.getParent().getNodeType()) {
+ case ASTNode.SINGLE_VARIABLE_DECLARATION:
+ case ASTNode.VARIABLE_DECLARATION_STATEMENT:
+ return occurrence.getLocationInParent() == SingleVariableDeclaration.NAME_PROPERTY;
+
+ case ASTNode.VARIABLE_DECLARATION_EXPRESSION:
+ return occurrence.getLocationInParent() == VariableDeclarationExpression.FRAGMENTS_PROPERTY;
+
+ case ASTNode.VARIABLE_DECLARATION_FRAGMENT:
+ return occurrence.getLocationInParent() == VariableDeclarationFragment.NAME_PROPERTY;
+
+ default:
+ return false;
+ }
+ }
+
+ private boolean hasAnnotation(final FieldDeclaration field) {
+ List<IExtendedModifier> modifiers= field.modifiers();
+ return modifiers.stream().anyMatch(IExtendedModifier::isAnnotation);
+ }
+ });
+
+ if (rewriteOperations.isEmpty()) {
+ return null;
+ }
+
+ return new CompilationUnitRewriteOperationsFix(MultiFixMessages.SingleUsedFieldCleanUp_description, unit,
+ rewriteOperations.toArray(new CompilationUnitRewriteOperation[0]));
+ }
+
+ @Override
+ public boolean canFix(final ICompilationUnit compilationUnit, final IProblemLocation problem) {
+ return false;
+ }
+
+ @Override
+ protected ICleanUpFix createFix(final CompilationUnit unit, final IProblemLocation[] problems) throws CoreException {
+ return null;
+ }
+
+ private static class SingleUsedFieldOperation extends CompilationUnitRewriteOperation {
+ private final FieldDeclaration field;
+ private final VariableDeclarationFragment fragment;
+ private final SimpleName reassignment;
+ private final Set<SimpleName> occurrences;
+
+ public SingleUsedFieldOperation(final FieldDeclaration field, final VariableDeclarationFragment fragment, final SimpleName reassignment, Set<SimpleName> occurrences) {
+ this.field= field;
+ this.fragment= fragment;
+ this.reassignment= reassignment;
+ this.occurrences= occurrences;
+ }
+
+ @Override
+ public void rewriteAST(final CompilationUnitRewrite cuRewrite, final LinkedProposalModel linkedModel) throws CoreException {
+ ASTRewrite rewrite= cuRewrite.getASTRewrite();
+ AST ast= cuRewrite.getRoot().getAST();
+ TextEditGroup groupOldFieldDeclaration= createTextEditGroup(MultiFixMessages.SingleUsedFieldCleanUp_description_old_field_declaration, cuRewrite);
+ TextEditGroup groupNewLocalVar= createTextEditGroup(MultiFixMessages.SingleUsedFieldCleanUp_description_new_local_var_declaration, cuRewrite);
+ TextEditGroup groupUsesOfTheVar= createTextEditGroup(MultiFixMessages.SingleUsedFieldCleanUp_description_uses_of_the_var, cuRewrite);
+
+ boolean isFieldKept= field.fragments().size() != 1;
+
+ if (isFieldKept) {
+ rewrite.remove(fragment, groupOldFieldDeclaration);
+ ASTNodes.replaceButKeepComment(rewrite, field.getType(), rewrite.createCopyTarget(field.getType()), groupOldFieldDeclaration);
+ } else {
+ rewrite.remove(field, groupOldFieldDeclaration);
+ }
+
+ Assignment reassignmentAssignment= ASTNodes.getTypedAncestor(reassignment, Assignment.class);
+
+ VariableDeclarationFragment newFragment= ast.newVariableDeclarationFragment();
+ newFragment.setName(ASTNodes.createMoveTarget(rewrite, reassignment));
+ newFragment.setInitializer(ASTNodes.createMoveTarget(rewrite, reassignmentAssignment.getRightHandSide()));
+ List<Dimension> extraDimensions= fragment.extraDimensions();
+ List<Dimension> newExtraDimensions= newFragment.extraDimensions();
+ newExtraDimensions.addAll(ASTNodes.createMoveTarget(rewrite, extraDimensions));
+
+ VariableDeclarationStatement newDeclareStatement= ast.newVariableDeclarationStatement(newFragment);
+ newDeclareStatement.setType(isFieldKept ? ASTNodes.createMoveTarget(rewrite, field.getType()) : (Type) rewrite.createCopyTarget(field.getType()));
+ List<IExtendedModifier> modifiers= field.modifiers();
+ List<IExtendedModifier> newModifiers= newDeclareStatement.modifiers();
+
+ for (IExtendedModifier iExtendedModifier : modifiers) {
+ Modifier modifier= (Modifier) iExtendedModifier;
+
+ if (!modifier.isPrivate() && !modifier.isStatic()) {
+ newModifiers.add(isFieldKept ? ASTNodes.createMoveTarget(rewrite, modifier) : (Modifier) rewrite.createCopyTarget(modifier));
+ }
+ }
+
+ ASTNodes.replaceButKeepComment(rewrite, ASTNodes.getTypedAncestor(reassignmentAssignment, Statement.class),
+ newDeclareStatement, groupNewLocalVar);
+
+ for (SimpleName occurrence : occurrences) {
+ if (occurrence != reassignment && occurrence.getParent() instanceof FieldAccess) {
+ ASTNodes.replaceButKeepComment(rewrite, occurrence.getParent(), occurrence, groupUsesOfTheVar);
+ }
+ }
+ }
+ }
+}
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java
index 85a5bd6..ed9a74f 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.java
@@ -81,6 +81,7 @@
public static String OptimizationTabPage_GroupName_Optimization;
+ public static String OptimizationTabPage_CheckboxName_SingleUsedField;
public static String OptimizationTabPage_CheckboxName_BreakLoop;
public static String OptimizationTabPage_CheckboxName_StaticInnerClass;
public static String OptimizationTabPage_CheckboxName_StringBuilder;
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties
index 88299df..ad13753 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties
@@ -60,6 +60,7 @@
CodeStyleTabPage_CheckboxName_SimplifyLambdaExpressionAndMethodRefSyntax=Simplify &lambda expression and method reference syntax
OptimizationTabPage_GroupName_Optimization=Optimization
+OptimizationTabPage_CheckboxName_SingleUsedField=Convert fields into local variables if the use is only local
OptimizationTabPage_CheckboxName_BreakLoop=Exit &loop earlier
OptimizationTabPage_CheckboxName_StaticInnerClass=Make inner classes static where possible
OptimizationTabPage_CheckboxName_StringBuilder=Replace &String concatenation by StringBuilder
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/OptimizationTabPage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/OptimizationTabPage.java
index fecf995..a166f7a 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/OptimizationTabPage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/OptimizationTabPage.java
@@ -29,6 +29,7 @@
import org.eclipse.jdt.internal.ui.fix.PrimitiveParsingCleanUp;
import org.eclipse.jdt.internal.ui.fix.PrimitiveSerializationCleanUp;
import org.eclipse.jdt.internal.ui.fix.StaticInnerClassCleanUp;
+import org.eclipse.jdt.internal.ui.fix.SingleUsedFieldCleanUp;
import org.eclipse.jdt.internal.ui.fix.StringBuilderCleanUp;
public final class OptimizationTabPage extends AbstractCleanUpTabPage {
@@ -37,6 +38,7 @@
@Override
protected AbstractCleanUp[] createPreviewCleanUps(Map<String, String> values) {
return new AbstractCleanUp[] {
+ new SingleUsedFieldCleanUp(values),
new BreakLoopCleanUp(values),
new StaticInnerClassCleanUp(values),
new StringBuilderCleanUp(values),
@@ -53,6 +55,9 @@
protected void doCreatePreferences(Composite composite, int numColumns) {
Group optimizationGroup= createGroup(numColumns, composite, CleanUpMessages.OptimizationTabPage_GroupName_Optimization);
+ final CheckboxPreference singleUsedFieldPref= createCheckboxPref(optimizationGroup, numColumns, CleanUpMessages.OptimizationTabPage_CheckboxName_SingleUsedField, CleanUpConstants.SINGLE_USED_FIELD, CleanUpModifyDialog.FALSE_TRUE);
+ registerPreference(singleUsedFieldPref);
+
final CheckboxPreference breakLoopPref= createCheckboxPref(optimizationGroup, numColumns, CleanUpMessages.OptimizationTabPage_CheckboxName_BreakLoop, CleanUpConstants.BREAK_LOOP, CleanUpModifyDialog.FALSE_TRUE);
registerPreference(breakLoopPref);
