J2K: preserving of visibility modifier for override members when access level was higher

idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt

@@ -18,9 +18,11 @@ package org.jetbrains.kotlin.idea.j2k
 
 import com.intellij.psi.search.LocalSearchScope
 import com.intellij.psi.search.searches.ReferencesSearch
+import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
 import org.jetbrains.kotlin.diagnostics.Diagnostic
 import org.jetbrains.kotlin.diagnostics.DiagnosticFactory
 import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
 import org.jetbrains.kotlin.idea.inspections.RedundantSamConstructorInspection
 import org.jetbrains.kotlin.idea.intentions.*
 import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfThenToElvisIntention
@@ -28,9 +30,12 @@ import org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.I
 import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFix
 import org.jetbrains.kotlin.idea.quickfix.RemoveRightPartOfBinaryExpressionFix
 import org.jetbrains.kotlin.idea.references.mainReference
+import org.jetbrains.kotlin.lexer.JetTokens
 import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.visibilityModifier
+import org.jetbrains.kotlin.resolve.OverridingUtil
 import org.jetbrains.kotlin.resolve.diagnostics.Diagnostics
-import java.util.ArrayList
+import java.util.*
 
 interface J2kPostProcessing {
     fun createAction(element: JetElement, diagnostics: Diagnostics): (() -> Unit)?
@@ -44,6 +49,7 @@ object J2KPostProcessingRegistrar {
 
     init {
         _processings.add(RemoveExplicitTypeArgumentsProcessing())
+        _processings.add(RemoveRedundantOverrideVisibilityProcessing())
         _processings.add(MoveLambdaOutsideParenthesesProcessing())
         _processings.add(ConvertToStringTemplateProcessing())
         _processings.add(UsePropertyAccessSyntaxProcessing())
@@ -133,6 +139,16 @@ object J2KPostProcessingRegistrar {
         }
     }
 
+    private class RemoveRedundantOverrideVisibilityProcessing : J2kPostProcessing {
+        override fun createAction(element: JetElement, diagnostics: Diagnostics): (() -> Unit)? {
+            if (element !is JetCallableDeclaration || !element.hasModifier(JetTokens.OVERRIDE_KEYWORD)) return null
+            val descriptor = element.resolveToDescriptor() as? CallableMemberDescriptor ?: return null
+            val visibilityFromSupers = OverridingUtil.findMaxVisibility(descriptor.overriddenDescriptors)?.normalize() ?: return null
+            if (visibilityFromSupers != descriptor.visibility) return null
+            return { element.visibilityModifier()?.delete() }
+        }
+    }
+
     private class MoveLambdaOutsideParenthesesProcessing : J2kPostProcessing {
         private val intention = MoveLambdaOutsideParenthesesIntention()
 

j2k/src/org/jetbrains/kotlin/j2k/ast/Function.kt

@@ -16,7 +16,7 @@
 
 package org.jetbrains.kotlin.j2k.ast
 
-import org.jetbrains.kotlin.j2k.*
+import org.jetbrains.kotlin.j2k.CodeBuilder
 
 abstract class FunctionLike(
         annotations: Annotations,
@@ -43,7 +43,7 @@ class Function(
         }
 
         if (modifiers.contains(Modifier.OVERRIDE)) {
-            modifiers = modifiers.filter { it != Modifier.OPEN && it !in ACCESS_MODIFIERS }
+            modifiers = modifiers.filter { it != Modifier.OPEN }
         }
 
         return modifiers

j2k/src/org/jetbrains/kotlin/j2k/ast/Modifier.kt

@@ -37,9 +37,12 @@ class Modifiers(modifiers: Collection<Modifier>) : Element() {
     val modifiers = modifiers.toSet()
 
     override fun generateCode(builder: CodeBuilder) {
-        val text = modifiers
+        val modifiersToInclude = if (modifiers.contains(Modifier.OVERRIDE))
+            modifiers // for override members we remove redundant visibility modifiers in post-processing
+        else
+            modifiers.filter { it != Modifier.PUBLIC }
+        val text = modifiersToInclude
                 .sortedBy { it.ordinal() }
-                .filter { it != Modifier.PUBLIC }
                 .map { it.toKotlin() }
                 .joinToString(" ")
         builder.append(text)

j2k/testData/fileOrElement/enum/overrideToString.java

@@ -1,2 +1,2 @@
 //class
-enum Color { WHITE, BLACK, RED, YELLOW, BLUE;@Override String toString() {  return "COLOR";}}
+enum Color { WHITE, BLACK, RED, YELLOW, BLUE;@Override public String toString() {  return "COLOR";}}

j2k/testData/fileOrElement/function/overrideWithHigherVisibility.java

@@ -0,0 +1,10 @@
+class Base {
+    protected void foo(){}
+}
+
+class Derived extends Base {
+    @Override
+    public void foo() {
+        super.foo();
+    }
+}

j2k/testData/fileOrElement/function/overrideWithHigherVisibility.kt

@@ -0,0 +1,10 @@
+internal open class Base {
+    protected open fun foo() {
+    }
+}
+
+internal class Derived : Base() {
+    public override fun foo() {
+        super.foo()
+    }
+}

j2k/testData/fileOrElement/issues/kt-836.java

@@ -18,14 +18,14 @@ public class Language implements Serializable {
 
 class Base {
   void test() {}
-  String toString() {
+  public String toString() {
     return "BASE";
   }
 }
 
 class Child extends Base {
   void test() {}
-  String toString() {
+  public String toString() {
       return "Child";
   }
 }

j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterForWebDemoTestGenerated.java

@@ -2350,6 +2350,12 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo
             doTest(fileName);
         }
 
+        @TestMetadata("overrideWithHigherVisibility.java")
+        public void testOverrideWithHigherVisibility() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/function/overrideWithHigherVisibility.java");
+            doTest(fileName);
+        }
+
         @TestMetadata("ownGenericParam.java")
         public void testOwnGenericParam() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/function/ownGenericParam.java");

j2k/tests/org/jetbrains/kotlin/j2k/JavaToKotlinConverterSingleFileTestGenerated.java

@@ -2350,6 +2350,12 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo
             doTest(fileName);
         }
 
+        @TestMetadata("overrideWithHigherVisibility.java")
+        public void testOverrideWithHigherVisibility() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/function/overrideWithHigherVisibility.java");
+            doTest(fileName);
+        }
+
         @TestMetadata("ownGenericParam.java")
         public void testOwnGenericParam() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("j2k/testData/fileOrElement/function/ownGenericParam.java");