Fix for #612: add quick fix resolver/proposal for override final method

ide-test/org.codehaus.groovy.eclipse.quickfix.test/META-INF/MANIFEST.MF

@@ -7,7 +7,8 @@ Bundle-Version: 3.3.0.qualifier
 Export-Package: org.codehaus.groovy.eclipse.quickfix.test,
  org.codehaus.groovy.eclipse.quickfix.test.resolvers,
  org.codehaus.groovy.eclipse.quickfix.test.templates
-Import-Package: org.codehaus.groovy.eclipse.codebrowsing.requestor
+Import-Package: org.codehaus.groovy.eclipse.codebrowsing.requestor,
+ org.eclipse.jdt.launching.environments
 Require-Bundle: org.codehaus.groovy.eclipse.quickfix;bundle-version="3.3.0",
  org.codehaus.groovy.eclipse.refactoring.test;bundle-version="3.3.0",
  org.codehaus.groovy.eclipse.tests;bundle-version="3.3.0"

ide-test/org.codehaus.groovy.eclipse.quickfix.test/src/org/codehaus/groovy/eclipse/quickfix/test/resolvers/GroovyProjectGroovyQuickFixTests.groovy

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2018 the original author or authors.
+ * Copyright 2009-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@
  */
 package org.codehaus.groovy.eclipse.quickfix.test.resolvers
 
+import static org.junit.Assert.assertEquals
 import static org.junit.Assume.assumeTrue
 
 import org.codehaus.groovy.eclipse.core.model.GroovyRuntime
@@ -25,9 +26,9 @@ import org.codehaus.groovy.eclipse.quickfix.proposals.IQuickFixResolver
 import org.codehaus.groovy.eclipse.quickfix.proposals.ProblemType
 import org.codehaus.groovy.eclipse.quickfix.proposals.AddClassCastResolver.AddClassCastProposal
 import org.eclipse.core.resources.IMarker
-import org.eclipse.core.resources.IResource
 import org.eclipse.jdt.core.ICompilationUnit
-import org.eclipse.jdt.core.IJavaModelMarker
+import org.eclipse.jdt.internal.corext.util.JavaModelUtil
+import org.eclipse.jface.text.contentassist.ICompletionProposal
 import org.junit.Before
 import org.junit.Test
 
@@ -59,7 +60,7 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
     }
 
     @Test
-    void testBasicAddImportInnerType() {
+    void testAddImportInnerType() {
         // When an InnerType is referenced with its declaring type, for example,
         // Map.Entry,
         // 'Map' is imported. When the InnerType is referenced by it's simple
@@ -80,7 +81,7 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
     }
 
     @Test
-    void testBasicAddImportInnerType2() {
+    void testAddImportInnerType2() {
         // When an InnerType is referenced with its declaring type, for example,
         // Map.Entry,
         // 'Map' is imported. When the InnerType is referenced by it's simple
@@ -102,7 +103,7 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
     }
 
     @Test
-    void testBasicAddImportInnerInnerType() {
+    void testAddImportInnerInnerType() {
         String typeToImport = 'InnerInnerType'
         String typeToAddImport = 'BarUsingInnerInner'
         String typeToImportFullyQualified = 'com.test.subtest.TopLevelType.InnerType.InnerInnerType'
@@ -267,8 +268,8 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
         testMultipleProposalsSameTypeName('CompileDynamic', expectedProposals, 'Test', '@CompileDynamic public class Test {}')
     }
 
-    @Test
-    void testAddImportGRECLIPSE1612() {
+    @Test // GRECLIPSE-1612
+    void testAddImportClassExpression() {
         addJavaSource('''\
             public class FooJava {
               public static String getProperty() {
@@ -289,8 +290,8 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
         assert proposal.displayString == 'Import \'FooJava\' (other)'
     }
 
-    @Test
-    void testGRECLIPSE1777() {
+    @Test // GRECLIPSE-1777
+    void testAddTypecast() {
         def unit = addGroovySource('''\
             @groovy.transform.CompileStatic
             class D {
@@ -305,29 +306,118 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
 
         String expectedQuickFixDisplay = 'Add cast to Integer'
         AddClassCastResolver resolver = getAddClassCastResolver(unit)
-        assert resolver != null : 'Expected a resolver for ' + unit
+        assert resolver != null : "Expected a resolver for ${ -> unit }"
         AddClassCastProposal proposal = getAddClassCastProposal(expectedQuickFixDisplay, resolver)
-        assert proposal != null : 'Expected a quick fix proposal for ' + unit
-        assert proposal.getDisplayString() ==  expectedQuickFixDisplay: 'Actual quick fix display expression should be: ' + expectedQuickFixDisplay
+        assert proposal != null : "Expected a quick fix proposal for ${ -> unit }"
+        assert proposal.displayString ==  expectedQuickFixDisplay: "Actual quick fix display expression should be: ${ -> expectedQuickFixDisplay }"
+    }
+
+    @Test
+    void testRemoveFinalModifier0() {
+        String contents = '''\
+            package foo
+            class Bar {
+              void wait() {} // attempts to override final method
+            }
+            '''.stripIndent()
+        def unit = addGroovySource(contents, 'Baz', 'foo')
+
+        def proposals = findQuickFixes(unit, ProblemType.FINAL_METHOD_OVERRIDE)
+
+        assert proposals.isEmpty() : 'Expected no quick fix for override of final method in binary type'
+    }
+
+    @Test
+    void testRemoveFinalModifier1() {
+        String contents = '''\
+            package foo
+            class Bar {
+              final void meth() {}
+            }
+            class Baz extends Bar {
+              void meth() {} // attempts to override final method
+            }
+            '''.stripIndent()
+        def unit = addGroovySource(contents, 'Baz', 'foo')
+
+        def proposals = findQuickFixes(unit, ProblemType.FINAL_METHOD_OVERRIDE)
+
+        proposals[0].apply(null)
+        JavaModelUtil.reconcile(unit)
+        assertEquals(contents.replaceFirst('final ', ''), String.valueOf(unit.contents))
+    }
+
+    @Test
+    void testRemoveFinalModifier2() {
+        String contents = '''\
+            package foo
+            class Bar {
+              final void meth() {}
+            }
+            '''.stripIndent()
+        def unit1 = addGroovySource(contents, 'Bar', 'foo')
+
+        def unit2 = addGroovySource('''\
+            package foo
+            class Baz extends Bar {
+              void meth() {} // attempts to override final method
+            }
+            '''.stripIndent(), 'Baz', 'foo')
+
+        def proposals = findQuickFixes(unit2, ProblemType.FINAL_METHOD_OVERRIDE)
+
+        proposals[0].apply(null)
+        JavaModelUtil.reconcile(unit1)
+        assertEquals(contents.replaceFirst('final ', ''), String.valueOf(unit1.contents))
+    }
+
+    @Test
+    void testRemoveFinalModifier3() {
+        String contents = '''\
+            package foo
+            class Bar {
+              final void meth() {}
+            }
+            '''.stripIndent()
+        def unit1 = addGroovySource(contents, 'Bar', 'foo')
+
+        addGroovySource('''\
+            package foo
+            class Baz extends Bar {
+            }
+            '''.stripIndent(), 'Baz', 'foo')
+
+        def unit2 = addGroovySource('''\
+            package whatever
+            class Something extends foo.Baz {
+              void meth() {} // attempts to override final method
+            }
+            '''.stripIndent(), 'Something', 'whatever')
+
+        def proposals = findQuickFixes(unit2, ProblemType.FINAL_METHOD_OVERRIDE)
+
+        proposals[0].apply(null)
+        JavaModelUtil.reconcile(unit1)
+        assertEquals(contents.replaceFirst('final ', ''), String.valueOf(unit1.contents))
     }
 
     @Test
     void testAddGroovyRuntime() {
-        GroovyRuntime.removeGroovyClasspathContainer(topLevelUnit.getJavaProject())
-        def testProject = topLevelUnit.getJavaProject().getProject()
+        GroovyRuntime.removeGroovyClasspathContainer(topLevelUnit.javaProject)
+        def testProject = topLevelUnit.javaProject.project
         buildProject()
 
-        IMarker[] markers = testProject.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE)
+        IMarker[] markers = getJDTFailureMarkers(testProject)
         assumeTrue(markers != null && markers.length > 0)
 
         List<IQuickFixResolver> resolvers = getAllQuickFixResolversForType(markers, ProblemType.MISSING_CLASSPATH_CONTAINER_TYPE, topLevelUnit)
         assert resolvers.size() == 1 : 'Should have found exactly one resolver'
         assert resolvers.get(0) instanceof AddGroovyRuntimeResolver : 'Wrong type of resolver'
 
-        resolvers.get(0).getQuickFixProposals().get(0).apply(null)
+        resolvers[0].quickFixProposals[0].apply(null)
         buildProject()
 
-        markers = testProject.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, true, IResource.DEPTH_INFINITE)
+        markers = getJDTFailureMarkers(testProject)
         assert !markers : 'Should not have found problems in this project'
     }
 
@@ -343,4 +433,13 @@ final class GroovyProjectGroovyQuickFixTests extends QuickFixHarness {
             assert resolvers == null || resolvers.isEmpty() : 'Encountered resolvers for unknown compilation error; none expected'
         }
     }
+
+    //--------------------------------------------------------------------------
+
+    private List<? extends ICompletionProposal> findQuickFixes(ICompilationUnit unit, ProblemType type) {
+        IMarker[] markers = getCompilationUnitJDTFailureMarkers(unit)
+        getAllQuickFixResolversForType(markers, type, unit).collectMany {
+            it.quickFixProposals
+        }
+    }
 }

ide-test/org.codehaus.groovy.eclipse.quickfix.test/src/org/codehaus/groovy/eclipse/quickfix/test/resolvers/QuickFixHarness.groovy

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2018 the original author or authors.
+ * Copyright 2009-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@ import org.eclipse.jdt.core.ICompilationUnit
 import org.eclipse.jdt.core.IJavaModelMarker
 import org.eclipse.jdt.core.IType
 import org.eclipse.jdt.internal.ui.text.correction.AssistContext
+import org.eclipse.jdt.internal.ui.text.correction.ProblemLocation
 import org.eclipse.jdt.ui.text.java.IJavaCompletionProposal
 
 /**
@@ -224,19 +225,19 @@ abstract class QuickFixHarness extends QuickFixTestSuite {
         // to not solve a similar marker from another compilation unit, in case
         // a project has multiple markers
         // from different Groovy files
-        IResource markResource = marker.getResource(), unitResource = unit.getResource()
-        if (!markResource.equals(unitResource)) {
+        IResource markResource = marker.resource, unitResource = unit.resource
+        if (markResource != unitResource) {
             return null
         }
         if (((Integer) marker.getAttribute(IMarker.SEVERITY)).intValue() == IMarker.SEVERITY_ERROR) {
             String[] markerMessages = getMarkerMessages(marker)
             // NOTE: this is not the same as the marker ID
-            int problemID = ((Integer) marker.getAttribute("id")).intValue()
-            ProblemDescriptor descriptor = new GroovyQuickFixProcessor().getProblemDescriptor(problemID, marker.getType(), markerMessages)
-            if (descriptor != null && descriptor.getType() == problemType) {
+            int problemID = ((Integer) marker.getAttribute('id')).intValue()
+            ProblemDescriptor descriptor = new GroovyQuickFixProcessor().getProblemDescriptor(problemID, marker.type, markerMessages)
+            if (descriptor != null && descriptor.type == problemType) {
                 int offset = ((Integer) marker.getAttribute(IMarker.CHAR_START))
-                int length = ((Integer) marker.getAttribute(IMarker.CHAR_END))
-                return new QuickFixProblemContext(descriptor, new AssistContext(unit, offset, length), null)
+                int length = ((Integer) marker.getAttribute(IMarker.CHAR_END)) - offset
+                return new QuickFixProblemContext(descriptor, new AssistContext(unit, offset, length), new ProblemLocation(offset, length, problemID, null, true, marker.type))
             }
         }
         return null
@@ -262,12 +263,12 @@ abstract class QuickFixHarness extends QuickFixTestSuite {
         for (resolver in resolvers) {
             if (resolver instanceof AddMissingGroovyImportsResolver) {
                 AddMissingGroovyImportsResolver importResolver = (AddMissingGroovyImportsResolver) resolver
-                List<IJavaCompletionProposal> proposals = importResolver.getQuickFixProposals()
+                List<IJavaCompletionProposal> proposals = importResolver.quickFixProposals
                 if (proposals != null) {
-                    for (IJavaCompletionProposal proposal : proposals) {
+                    for (proposal in proposals) {
                         if (proposal instanceof AddMissingImportProposal) {
                             AddMissingImportProposal importProposal = (AddMissingImportProposal) proposal
-                            if (importProposal.getSuggestedJavaType().getElementName().equals(unresolvedSimpleName)) {
+                            if (importProposal.suggestedJavaType.elementName == unresolvedSimpleName) {
                                 return importResolver
                             }
                         }
@@ -292,10 +293,9 @@ abstract class QuickFixHarness extends QuickFixTestSuite {
         for (resolver in resolvers) {
             if (resolver instanceof AddClassCastResolver) {
                 AddClassCastResolver classCastResolver = (AddClassCastResolver) resolver
-                List<IJavaCompletionProposal> proposals = classCastResolver
-                        .getQuickFixProposals()
+                List<IJavaCompletionProposal> proposals = classCastResolver.quickFixProposals
                 if (proposals != null) {
-                    for (IJavaCompletionProposal proposal : proposals) {
+                    for (proposal in proposals) {
                         if (proposal instanceof AddClassCastProposal) {
                             return classCastResolver
                         }
@@ -306,9 +306,13 @@ abstract class QuickFixHarness extends QuickFixTestSuite {
         return null
     }
 
-    protected IMarker[] getCompilationUnitJDTFailureMarkers(ICompilationUnit unit) throws Exception {
+    protected IMarker[] getJDTFailureMarkers(IResource resource) {
         buildProject()
         waitForIndex()
-        return unit.resource.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE)
+        resource.findMarkers(IJavaModelMarker.JAVA_MODEL_PROBLEM_MARKER, false, IResource.DEPTH_INFINITE)
+    }
+
+    protected IMarker[] getCompilationUnitJDTFailureMarkers(ICompilationUnit unit) {
+        getJDTFailureMarkers(unit.resource)
     }
 }

ide/org.codehaus.groovy.eclipse.quickfix/src/org/codehaus/groovy/eclipse/quickfix/proposals/ChangeOverriddenModifierResolver.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2009-2019 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.codehaus.groovy.eclipse.quickfix.proposals;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.eclipse.jdt.core.JavaModelException;
+import org.eclipse.jdt.internal.ui.text.correction.ModifierCorrectionSubProcessor;
+import org.eclipse.jdt.ui.text.java.IJavaCompletionProposal;
+import org.eclipse.jdt.ui.text.java.correction.ICommandAccess;
+
+public class ChangeOverriddenModifierResolver extends AbstractQuickFixResolver {
+
+    protected ChangeOverriddenModifierResolver(QuickFixProblemContext problem) {
+        super(problem);
+    }
+
+    @Override
+    protected ProblemType[] getTypes() {
+        return new ProblemType[] {ProblemType.FINAL_METHOD_OVERRIDE};
+    }
+
+    @Override
+    public List<IJavaCompletionProposal> getQuickFixProposals() {
+        /* TODO: Other 'kind' options:
+        case IProblem.InheritedMethodReducesVisibility:
+        case IProblem.MethodReducesVisibility:
+        case IProblem.OverridingNonVisibleMethod:
+            ModifierCorrectionSubProcessor.TO_VISIBLE
+        case IProblem.FinalMethodCannotBeOverridden:
+            ModifierCorrectionSubProcessor.TO_NON_FINAL
+        case IProblem.CannotOverrideAStaticMethodWithAnInstanceMethod:
+            ModifierCorrectionSubProcessor.TO_NON_STATIC
+         */
+        try {
+            List<ICommandAccess> commands = new ArrayList<>();
+            QuickFixProblemContext context = getQuickFixProblem();
+            int kind = ModifierCorrectionSubProcessor.TO_NON_FINAL;
+            ModifierCorrectionSubProcessor.addChangeOverriddenModifierProposal(context.getContext(), context.getLocation(), commands, kind);
+            return List.class.cast(commands.stream().filter(command -> command instanceof IJavaCompletionProposal).collect(Collectors.toList()));
+        } catch (JavaModelException e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

ide/org.codehaus.groovy.eclipse.quickfix/src/org/codehaus/groovy/eclipse/quickfix/proposals/GroovyQuickFixResolverRegistry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2018 the original author or authors.
+ * Copyright 2009-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -102,6 +102,9 @@ protected static IQuickFixResolver[] getRegisteredResolvers(QuickFixProblemConte
 
             // Add class cast
             new AddClassCastResolver(problem),
+
+            // Change modifier(s) of overridden method
+            new ChangeOverriddenModifierResolver(problem),
         };
     }
 }