Bug 531441 - Add redundant modifier cleanup actionI20180425-0725I20180425-0505

Change-Id: I0e94fc38c5aa3fcc297aa38809a8cd486e48457e
Signed-off-by: Karsten Thoms <karsten.thoms@itemis.de>

10 files changed, 303 insertions, 37 deletions

diff --git a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpStressTest.java b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpStressTest.java
index dcd42a8715..121d3a9c7c 100644
--- a/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpStressTest.java
+++ b/org.eclipse.jdt.ui.tests/ui/org/eclipse/jdt/ui/tests/quickfix/CleanUpStressTest.java
@@ -1066,9 +1066,8 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append(" * An interface to define how a test suite should be loaded.\n");
         buf.append(" */\n");
         buf.append("public interface TestSuiteLoader {\n");
-        buf.append("    abstract public Class load(String suiteClassName)\n");
-        buf.append("            throws ClassNotFoundException;\n");
-        buf.append("    abstract public Class reload(Class aClass) throws ClassNotFoundException;\n");
+        buf.append("    Class load(String suiteClassName) throws ClassNotFoundException;\n");
+        buf.append("    Class reload(Class aClass) throws ClassNotFoundException;\n");
         buf.append("}");
         fExpectedChangesAllTests.put("junit.runner.TestSuiteLoader.java", buf.toString());
         buf= new StringBuffer();
@@ -1455,11 +1454,11 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append("    /**\n");
         buf.append("     * Counts the number of test cases that will be run by this test.\n");
         buf.append("     */\n");
-        buf.append("    public abstract int countTestCases();\n");
+        buf.append("    int countTestCases();\n");
         buf.append("    /**\n");
         buf.append("     * Runs a test and collects its result in a TestResult instance.\n");
         buf.append("     */\n");
-        buf.append("    public abstract void run(TestResult result);\n");
+        buf.append("    void run(TestResult result);\n");
         buf.append("}");
         fExpectedChangesAllTests.put("junit.framework.Test.java", buf.toString());
         buf= new StringBuffer();
@@ -2772,19 +2771,19 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append("    /**\n");
         buf.append("     * An error occurred.\n");
         buf.append("     */\n");
-        buf.append("    public void addError(Test test, Throwable t);\n");
+        buf.append("    void addError(Test test, Throwable t);\n");
         buf.append("    /**\n");
         buf.append("     * A failure occurred.\n");
         buf.append("     */\n");
-        buf.append("    public void addFailure(Test test, AssertionFailedError t);\n");
+        buf.append("    void addFailure(Test test, AssertionFailedError t);\n");
         buf.append("    /**\n");
         buf.append("     * A test ended.\n");
         buf.append("     */\n");
-        buf.append("    public void endTest(Test test);\n");
+        buf.append("    void endTest(Test test);\n");
         buf.append("    /**\n");
         buf.append("     * A test started.\n");
         buf.append("     */\n");
-        buf.append("    public void startTest(Test test);\n");
+        buf.append("    void startTest(Test test);\n");
         buf.append("}");
         fExpectedChangesAllTests.put("junit.framework.TestListener.java", buf.toString());
         buf= new StringBuffer();
@@ -2868,7 +2867,7 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append("    /**\n");
         buf.append("     * Run the the following method protected.\n");
         buf.append("     */\n");
-        buf.append("    public abstract void protect() throws Throwable;\n");
+        buf.append("    void protect() throws Throwable;\n");
         buf.append("}");
         fExpectedChangesAllTests.put("junit.framework.Protectable.java", buf.toString());
         buf= new StringBuffer();
@@ -2882,37 +2881,37 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append("    /**\n");
         buf.append("     * Adds a money to this money.\n");
         buf.append("     */\n");
-        buf.append("    public abstract IMoney add(IMoney m);\n");
+        buf.append("    IMoney add(IMoney m);\n");
         buf.append("    /**\n");
         buf.append("     * Adds a simple Money to this money. This is a helper method for\n");
         buf.append("     * implementing double dispatch\n");
         buf.append("     */\n");
-        buf.append("    public abstract IMoney addMoney(Money m);\n");
+        buf.append("    IMoney addMoney(Money m);\n");
         buf.append("    /**\n");
         buf.append("     * Adds a MoneyBag to this money. This is a helper method for implementing\n");
         buf.append("     * double dispatch\n");
         buf.append("     */\n");
-        buf.append("    public abstract IMoney addMoneyBag(MoneyBag s);\n");
+        buf.append("    IMoney addMoneyBag(MoneyBag s);\n");
         buf.append("    /**\n");
         buf.append("     * Append this to a MoneyBag m.\n");
         buf.append("     */\n");
-        buf.append("    public abstract void appendTo(MoneyBag m);\n");
+        buf.append("    void appendTo(MoneyBag m);\n");
         buf.append("    /**\n");
         buf.append("     * Tests whether this money is zero\n");
         buf.append("     */\n");
-        buf.append("    public abstract boolean isZero();\n");
+        buf.append("    boolean isZero();\n");
         buf.append("    /**\n");
         buf.append("     * Multiplies a money by the given factor.\n");
         buf.append("     */\n");
-        buf.append("    public abstract IMoney multiply(int factor);\n");
+        buf.append("    IMoney multiply(int factor);\n");
         buf.append("    /**\n");
         buf.append("     * Negates this money.\n");
         buf.append("     */\n");
-        buf.append("    public abstract IMoney negate();\n");
+        buf.append("    IMoney negate();\n");
         buf.append("    /**\n");
         buf.append("     * Subtracts a money from this money.\n");
         buf.append("     */\n");
-        buf.append("    public abstract IMoney subtract(IMoney m);\n");
+        buf.append("    IMoney subtract(IMoney m);\n");
         buf.append("}");
         fExpectedChangesAllTests.put("junit.samples.money.IMoney.java", buf.toString());
         buf= new StringBuffer();
@@ -3161,15 +3160,15 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append(" */\n");
         buf.append("public interface TestRunListener {\n");
         buf.append("    /* test status constants */\n");
-        buf.append("    public static final int STATUS_ERROR = 1;\n");
-        buf.append("    public static final int STATUS_FAILURE = 2;\n");
-        buf.append("\n");
-        buf.append("    public void testEnded(String testName);\n");
-        buf.append("    public void testFailed(int status, String testName, String trace);\n");
-        buf.append("    public void testRunEnded(long elapsedTime);\n");
-        buf.append("    public void testRunStarted(String testSuiteName, int testCount);\n");
-        buf.append("    public void testRunStopped(long elapsedTime);\n");
-        buf.append("    public void testStarted(String testName);\n");
+        buf.append("    int STATUS_ERROR = 1;\n");
+        buf.append("    int STATUS_FAILURE = 2;\n");
+        buf.append("\n");
+        buf.append("    void testEnded(String testName);\n");
+        buf.append("    void testFailed(int status, String testName, String trace);\n");
+        buf.append("    void testRunEnded(long elapsedTime);\n");
+        buf.append("    void testRunStarted(String testSuiteName, int testCount);\n");
+        buf.append("    void testRunStopped(long elapsedTime);\n");
+        buf.append("    void testStarted(String testName);\n");
         buf.append("}\n");
         buf.append("");
         fExpectedChangesAllTests.put("junit.runner.TestRunListener.java", buf.toString());
@@ -4340,8 +4339,8 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append(" * of the JDK 1.1.7 compatibility.\n");
         buf.append(" */\n");
         buf.append("public class Sorter {\n");
-        buf.append("    public static interface Swapper {\n");
-        buf.append("        public void swap(Vector values, int left, int right);\n");
+        buf.append("    public interface Swapper {\n");
+        buf.append("        void swap(Vector values, int left, int right);\n");
         buf.append("    }\n");
         buf.append("\n");
         buf.append("    public static void sortStrings(final Vector values, int left, int right,\n");
@@ -4523,7 +4522,7 @@ public class CleanUpStressTest extends CleanUpTestCase {
         buf.append("    /**\n");
         buf.append("     * Returns an enumeration of Strings with qualified class names\n");
         buf.append("     */\n");
-        buf.append("    public Enumeration collectTests();\n");
+        buf.append("    Enumeration collectTests();\n");
         buf.append("}\n");
         buf.append("");
         fExpectedChangesAllTests.put("junit.runner.TestCollector.java", buf.toString());
@@ -5299,6 +5298,8 @@ public class CleanUpStressTest extends CleanUpTestCase {
 
 		enable(CleanUpConstants.SORT_MEMBERS);
 		enable(CleanUpConstants.SORT_MEMBERS_ALL);
+		
+		enable(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS);
 
 		ICompilationUnit[] units= cus.toArray(new ICompilationUnit[cus.size()]);
 		performRefactoring(units);
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 56ee976601..2a87bc2b0c 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
@@ -9110,4 +9110,50 @@ public class CleanUpTest extends CleanUpTestCase {
 		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1, cu2 }, new String[] { expected1, expected2 });
 	}
 
+	public void testRemoveRedundantModifiers () throws Exception {
+		IPackageFragment pack1= fSourceFolder.createPackageFragment("test", false, null);
+		StringBuffer buf= new StringBuffer();
+		buf.append("package test;\n");
+		buf.append("public interface IFoo {\n");
+		buf.append("  public static final int MAGIC_NUMBER = 646;\n");
+		buf.append("  public abstract int foo ();\n");
+		buf.append("  abstract void func ();\n");
+		buf.append("  public int bar (int bazz);\n");
+		buf.append("}\n");
+		ICompilationUnit cu1= pack1.createCompilationUnit("IFoo.java", buf.toString(), false, null);
+
+		buf= new StringBuffer();
+		buf.append("package test;\n");
+		buf.append("public interface IFoo {\n");
+		buf.append("  int MAGIC_NUMBER = 646;\n");
+		buf.append("  int foo ();\n");
+		buf.append("  void func ();\n");
+		buf.append("  int bar (int bazz);\n");
+		buf.append("}\n");
+		String expected1 = buf.toString();
+
+		buf= new StringBuffer();
+		buf.append("package test;\n");
+		buf.append("public final class Sealed {\n");
+		buf.append("  public final void foo () {};\n");
+		buf.append("  \n");
+		buf.append("  static interface INested {\n");
+		buf.append("  }\n");
+		buf.append("}\n");
+		ICompilationUnit cu2= pack1.createCompilationUnit("Sealed.java", buf.toString(), false, null);
+		
+		buf= new StringBuffer();
+		buf.append("package test;\n");
+		buf.append("public final class Sealed {\n");
+		buf.append("  public void foo () {};\n");
+		buf.append("  \n");
+		buf.append("  interface INested {\n");
+		buf.append("  }\n");
+		buf.append("}\n");
+		String expected2 = buf.toString();
+		
+		enable(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS);
+		assertRefactoringResultAsExpected(new ICompilationUnit[] { cu1, cu2 }, new String[] { expected1, expected2 });
+
+	}
 }
diff --git a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
index 3d4a6b7055..f1be85832c 100644
--- a/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
+++ b/org.eclipse.jdt.ui/core extension/org/eclipse/jdt/internal/corext/fix/CleanUpConstants.java
@@ -857,6 +857,18 @@ public class CleanUpConstants {
 	 */
 	public static final String REMOVE_REDUNDANT_TYPE_ARGUMENTS= "cleanup.remove_redundant_type_arguments"; //$NON-NLS-1$
 
+	/**
+	 * Removes redundant modifiers.<br>
+	 * <br>
+	 * Possible values: {TRUE, FALSE}<br>
+	 * <br>
+	 *
+	 * @see CleanUpOptions#TRUE
+	 * @see CleanUpOptions#FALSE
+	 * @since 3.14
+	 */
+	public static final String REMOVE_REDUNDANT_MODIFIERS= "cleanup.remove_redundant_modifiers"; //$NON-NLS-1$
+
 
 	/**
 	 * Controls whether missing annotations should be added to the code. For detailed settings use:<br>
@@ -1220,6 +1232,7 @@ public class CleanUpConstants {
 		options.setOption(REMOVE_UNNECESSARY_NLS_TAGS, CleanUpOptions.TRUE);
 		options.setOption(INSERT_INFERRED_TYPE_ARGUMENTS, CleanUpOptions.FALSE);
 		options.setOption(REMOVE_REDUNDANT_TYPE_ARGUMENTS, CleanUpOptions.FALSE);
+		options.setOption(REMOVE_REDUNDANT_MODIFIERS, CleanUpOptions.FALSE);
 
 		//Missing Code
 		options.setOption(ADD_MISSING_ANNOTATIONS, CleanUpOptions.TRUE);
@@ -1307,6 +1320,7 @@ public class CleanUpConstants {
 		options.setOption(REMOVE_UNNECESSARY_NLS_TAGS, CleanUpOptions.FALSE);
 		options.setOption(INSERT_INFERRED_TYPE_ARGUMENTS, CleanUpOptions.FALSE);
 		options.setOption(REMOVE_REDUNDANT_TYPE_ARGUMENTS, CleanUpOptions.FALSE);
+		options.setOption(REMOVE_REDUNDANT_MODIFIERS, CleanUpOptions.FALSE);
 
 		//Missing Code
 		options.setOption(ADD_MISSING_ANNOTATIONS, CleanUpOptions.TRUE);
diff --git a/org.eclipse.jdt.ui/plugin.xml b/org.eclipse.jdt.ui/plugin.xml
index 4b323a4e18..13b4b01060 100644
--- a/org.eclipse.jdt.ui/plugin.xml
+++ b/org.eclipse.jdt.ui/plugin.xml
@@ -6923,9 +6923,14 @@
             runAfter="org.eclipse.jdt.ui.cleanup.potential_programming_problems">
       </cleanUp>
       <cleanUp
+            class="org.eclipse.jdt.internal.ui.fix.RedundantModifiersCleanUp"
+            id="org.eclipse.jdt.ui.cleanup.unnecessary_modifiers"
+            runAfter="org.eclipse.jdt.ui.cleanup.unnecessary_code">
+      </cleanUp>
+      <cleanUp
             class="org.eclipse.jdt.internal.ui.fix.StringCleanUp"
             id="org.eclipse.jdt.ui.cleanup.strings"
-            runAfter="org.eclipse.jdt.ui.cleanup.unnecessary_code">
+            runAfter="org.eclipse.jdt.ui.cleanup.unnecessary_modifiers">
       </cleanUp>
       <cleanUp
             class="org.eclipse.jdt.internal.ui.fix.UnimplementedCodeCleanUp"
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
index 3effab65b3..f6b524aff4 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.java
@@ -105,6 +105,8 @@ public class MultiFixMessages extends NLS {
 	public static String TypeParametersCleanUp_InsertInferredTypeArguments_description;
 	public static String TypeParametersCleanUp_RemoveUnnecessaryTypeArguments_description;
 
+	public static String RedundantModifiersCleanup_description;
+
 	static {
 		// initialize resource bundle
 		NLS.initializeMessages(BUNDLE_NAME, MultiFixMessages.class);
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
index fd45e412f8..f42d2d8149 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/MultiFixMessages.properties
@@ -90,3 +90,5 @@ NullAnnotationsCleanUp_add_nonnullbydefault_annotation=Add missing @NonNullByDef
 NullAnnotationsCleanUp_remove_redundant_nullness_annotation=Remove redundant nullness annotation
 TypeParametersCleanUp_InsertInferredTypeArguments_description=Insert inferred type arguments
 TypeParametersCleanUp_RemoveUnnecessaryTypeArguments_description=Remove redundant type arguments
+
+RedundantModifiersCleanup_description = Remove redundant modifiers
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/RedundantModifiersCleanUp.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/RedundantModifiersCleanUp.java
new file mode 100644
index 0000000000..8702a272e6
--- /dev/null
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/fix/RedundantModifiersCleanUp.java
@@ -0,0 +1,187 @@
+/*******************************************************************************
+ * Copyright (c) 2018 itemis AG (http://www.itemis.eu) and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Karsten Thoms (itemis) - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.jdt.internal.ui.fix;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import org.eclipse.core.runtime.CoreException;
+
+import org.eclipse.jdt.core.ICompilationUnit;
+import org.eclipse.jdt.core.dom.ASTNode;
+import org.eclipse.jdt.core.dom.ASTVisitor;
+import org.eclipse.jdt.core.dom.CompilationUnit;
+import org.eclipse.jdt.core.dom.FieldDeclaration;
+import org.eclipse.jdt.core.dom.MethodDeclaration;
+import org.eclipse.jdt.core.dom.Modifier;
+import org.eclipse.jdt.core.dom.TypeDeclaration;
+
+import org.eclipse.jdt.internal.corext.dom.ASTNodes;
+import org.eclipse.jdt.internal.corext.dom.ModifierRewrite;
+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 removes redundant modifiers:
+ * <ul>
+ * <li>Within an interface modifiers <code>public</code>, <code>static</code> and <code>final</code>
+ * are redundant for field declarations.</li>
+ * <li>Within an interface modifier <code>public</code> and
+ * <code>abstract</code are redundant for method declarations.</li>
+ * <li>Within a final class the <code>final</code> modifier is redundant for method
+ * declarations.</li>
+ * <li>For nested interfaces the <code>static</code> modifier is redundant.</li>
+ * </ul>
+ */
+public class RedundantModifiersCleanUp extends AbstractMultiFix {
+	public RedundantModifiersCleanUp() {
+		this(Collections.emptyMap());
+	}
+
+	public RedundantModifiersCleanUp(Map<String, String> options) {
+		super(options);
+	}
+
+	@Override
+	public CleanUpRequirements getRequirements() {
+		boolean requireAST= isEnabled(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS);
+		Map<String, String> requiredOptions= null;
+		return new CleanUpRequirements(requireAST, false, false, requiredOptions);
+	}
+
+	@Override
+	public String[] getStepDescriptions() {
+		if (isEnabled(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS)) {
+			return new String[] { MultiFixMessages.RedundantModifiersCleanup_description };
+		}
+		return new String[0];
+	}
+
+	@SuppressWarnings("nls")
+	@Override
+	public String getPreview() {
+		StringBuffer buf= new StringBuffer();
+		buf.append("\n");
+		buf.append("public interface IFoo {\n");
+		if (isEnabled(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS)) {
+			buf.append("  int MAGIC_NUMBER = 646;\n");
+			buf.append("  int foo ();\n");
+			buf.append("  int bar (int bazz);\n");
+		} else {
+			buf.append("  public static final int MAGIC_NUMBER = 646;\n");
+			buf.append("  public abstract int foo ();\n");
+			buf.append("  public int bar (int bazz);\n");
+		}
+		buf.append("}\n");
+		buf.append("\n");
+		buf.append("public final class Sealed {\n");
+		if (isEnabled(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS)) {
+			buf.append("  public void foo () {};\n");
+			buf.append("  \n");
+			buf.append("  interface INested {\n");
+			buf.append("  }\n");
+
+		} else {
+			buf.append("  public final void foo () {};\n");
+			buf.append("  \n");
+			buf.append("  static interface INested {\n");
+			buf.append("  }\n");
+		}
+		buf.append("}\n");
+
+
+		return buf.toString();
+	}
+
+	@Override
+	protected ICleanUpFix createFix(CompilationUnit unit) throws CoreException {
+		if (!isEnabled(CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS)) {
+			return null;
+		}
+		final List<CompilationUnitRewriteOperation> rewriteOperations= new ArrayList<>();
+
+		unit.accept(new ASTVisitor() {
+			@Override
+			public boolean visit(FieldDeclaration node) {
+				TypeDeclaration typeDecl= ASTNodes.getParent(node, TypeDeclaration.class);
+				if (typeDecl.isInterface()) {
+					final int excluded= Modifier.PUBLIC | Modifier.STATIC | Modifier.FINAL;
+					if ((node.getModifiers() & excluded) > 0) {
+						rewriteOperations.add(new RemoveModifiersOperation(node, excluded));
+					}
+				}
+				return true;
+			}
+
+			@Override
+			public boolean visit(MethodDeclaration node) {
+				TypeDeclaration typeDecl= ASTNodes.getParent(node, TypeDeclaration.class);
+				if (typeDecl.isInterface()) {
+					rewriteOperations.add(new RemoveModifiersOperation(node, Modifier.PUBLIC | Modifier.ABSTRACT));
+				} else if (Modifier.isFinal(typeDecl.getModifiers()) && Modifier.isFinal(node.getModifiers())) {
+					rewriteOperations.add(new RemoveModifiersOperation(node, Modifier.FINAL));
+				}
+				return true;
+			}
+
+			@Override
+			public boolean visit(TypeDeclaration node) {
+				TypeDeclaration typeDecl= ASTNodes.getParent(node, TypeDeclaration.class);
+				if (node.isInterface() && Modifier.isStatic(node.getModifiers()) && typeDecl != null) {
+					rewriteOperations.add(new RemoveModifiersOperation(node, Modifier.STATIC));
+				}
+				return true;
+			}
+
+		});
+		if (rewriteOperations.isEmpty()) {
+			return null;
+		}
+		return new CompilationUnitRewriteOperationsFix(MultiFixMessages.RedundantModifiersCleanup_description, unit,
+				rewriteOperations.toArray(new CompilationUnitRewriteOperation[rewriteOperations.size()]));
+	}
+
+	@Override
+	public boolean canFix(ICompilationUnit compilationUnit, IProblemLocation problem) {
+		return false;
+	}
+
+	@Override
+	protected ICleanUpFix createFix(CompilationUnit unit, IProblemLocation[] problems) throws CoreException {
+		return null;
+	}
+
+	private static class RemoveModifiersOperation extends CompilationUnitRewriteOperation {
+		private final ASTNode node;
+
+		private final int excludedModifiers;
+
+		public RemoveModifiersOperation(ASTNode node, int excludedModifiers) {
+			this.node= node;
+			this.excludedModifiers= excludedModifiers;
+		}
+
+		@Override
+		public void rewriteAST(CompilationUnitRewrite cuRewrite, LinkedProposalModel linkedModel) throws CoreException {
+			ModifierRewrite rewrite= ModifierRewrite.create(cuRewrite.getASTRewrite(), node);
+			rewrite.setModifiers(Modifier.NONE, excludedModifiers, null);
+		}
+	}
+}
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 a4992b77ec..df71e0f3c3 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
@@ -104,6 +104,8 @@ public class CleanUpMessages extends NLS {
 	public static String UnnecessaryCodeTabPage_CheckboxName_UnnecessaryCasts;
 	public static String UnnecessaryCodeTabPage_CheckboxName_UnnecessaryNLSTags;
 	public static String UnnecessaryCodeTabPage_CheckboxName_RedundantTypeArguments;
+	public static String UnnecessaryCodeTabPage_CheckboxName_RedundantModifiers;
+	public static String UnnecessaryCodeTabPage_CheckboxName_RedundantModifiers_description;
 	public static String UnnecessaryCodeTabPage_CheckboxName_UnusedConstructors;
 	public static String UnnecessaryCodeTabPage_CheckboxName_UnusedFields;
 	public static String UnnecessaryCodeTabPage_CheckboxName_UnusedImports;
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 db316618ef..313c1f27b8 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
@@ -92,3 +92,5 @@ UnnecessaryCodeTabPage_GroupName_UnnecessaryCode=Unnecessary code
 UnnecessaryCodeTabPage_CheckboxName_UnnecessaryCasts=Remove unnecessar&y casts
 UnnecessaryCodeTabPage_CheckboxName_UnnecessaryNLSTags=Remove unnecessary '$NON-NLS$' ta&gs
 UnnecessaryCodeTabPage_CheckboxName_RedundantTypeArguments=Remove redundant type &arguments (1.7 or higher)
+UnnecessaryCodeTabPage_CheckboxName_RedundantModifiers=Remove redundant modifiers
+UnnecessaryCodeTabPage_CheckboxName_RedundantModifiers_description=Removes unnecessary modifiers on fields and methods
diff --git a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java
index f24e992168..7884085f9b 100644
--- a/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java
+++ b/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/preferences/cleanup/UnnecessaryCodeTabPage.java
@@ -21,6 +21,7 @@ import org.eclipse.jdt.internal.ui.fix.AbstractCleanUp;
 import org.eclipse.jdt.internal.ui.fix.StringCleanUp;
 import org.eclipse.jdt.internal.ui.fix.TypeParametersCleanUp;
 import org.eclipse.jdt.internal.ui.fix.UnnecessaryCodeCleanUp;
+import org.eclipse.jdt.internal.ui.fix.RedundantModifiersCleanUp;
 import org.eclipse.jdt.internal.ui.fix.UnusedCodeCleanUp;
 
 public final class UnnecessaryCodeTabPage extends AbstractCleanUpTabPage {
@@ -34,12 +35,13 @@ public final class UnnecessaryCodeTabPage extends AbstractCleanUpTabPage {
 	@Override
 	protected AbstractCleanUp[] createPreviewCleanUps(Map<String, String> values) {
 		return new AbstractCleanUp[] {
-        		new UnusedCodeCleanUp(values),
-        		new UnnecessaryCodeCleanUp(values),
-        		new StringCleanUp(values),
-        		new TypeParametersCleanUp(values)
-        };
-    }
+				new UnusedCodeCleanUp(values),
+				new UnnecessaryCodeCleanUp(values),
+				new StringCleanUp(values),
+				new TypeParametersCleanUp(values),
+				new RedundantModifiersCleanUp(values)
+		};
+	}
 
     @Override
 	protected void doCreatePreferences(Composite composite, int numColumns) {
@@ -70,6 +72,9 @@ public final class UnnecessaryCodeTabPage extends AbstractCleanUpTabPage {
 
 		CheckboxPreference typeArgs= createCheckboxPref(unnecessaryGroup, numColumns, CleanUpMessages.UnnecessaryCodeTabPage_CheckboxName_RedundantTypeArguments, CleanUpConstants.REMOVE_REDUNDANT_TYPE_ARGUMENTS, CleanUpModifyDialog.FALSE_TRUE);
 		registerPreference(typeArgs);
+
+		CheckboxPreference modifiers= createCheckboxPref(unnecessaryGroup, numColumns, CleanUpMessages.UnnecessaryCodeTabPage_CheckboxName_RedundantModifiers, CleanUpConstants.REMOVE_REDUNDANT_MODIFIERS, CleanUpModifyDialog.FALSE_TRUE);
+		registerPreference(modifiers);
     }
 
 }