Light class builder: do not generate methods delegating to DefaultImpls in kotlin classes

Class APIs from java point of view stays the same so we can avoid generating those methods
Otherwise we have to calculate all supertypes when getMethods() is called,
    which imposes severe performance penalties
We have to pretend these methods are not 'abstract' (also we consider them 'default' for safety)
    so java highlighting does not report "class should be abstract" for all inheritors
We have to manually report "class should be abstract" on some of the java inheritors,
    specifically those that are implementing interfaces directly
	    as opposed to extending kotlin classes implementing those interfaces
This commit is contained in:
Pavel V. Talanov
2017-03-30 17:44:19 +03:00
parent db294da24d
commit 4f701285b1
38 changed files with 510 additions and 45 deletions
@@ -0,0 +1,77 @@
/*
* Copyright 2010-2017 JetBrains s.r.o.
*
* 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.jetbrains.kotlin.idea.java
import com.intellij.codeInsight.ClassUtil.getAnyMethodToImplement
import com.intellij.codeInsight.daemon.JavaErrorMessages
import com.intellij.codeInsight.daemon.impl.analysis.HighlightNamesUtil.getClassDeclarationTextRange
import com.intellij.codeInsight.daemon.impl.analysis.HighlightUtil
import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil
import com.intellij.codeInsight.intention.QuickFixFactory
import com.intellij.lang.annotation.Annotation
import com.intellij.lang.annotation.AnnotationHolder
import com.intellij.lang.annotation.Annotator
import com.intellij.psi.*
import org.jetbrains.kotlin.asJava.classes.KtLightClassForSourceDeclaration
import org.jetbrains.kotlin.asJava.elements.KtLightMethod
import org.jetbrains.kotlin.idea.KotlinLanguage
import org.jetbrains.kotlin.utils.ifEmpty
class UnimplementedKotlinInterfaceMemberAnnotator : Annotator {
override fun annotate(element: PsiElement, holder: AnnotationHolder) {
if (element !is PsiClass || element.language == KotlinLanguage.INSTANCE) return
if (element.isInterface || element.hasModifierProperty(PsiModifier.ABSTRACT)) return
if (getAnyMethodToImplement(element) != null) return // reported by java default annotator
findUnimplementedMethod(element)?.let {
report(it, holder, element)
}
}
private fun findUnimplementedMethod(psiClass: PsiClass): KtLightMethod? {
val signaturesFromKotlinInterfaces = psiClass.visibleSignatures.filter { signature ->
signature.method.let { it is KtLightMethod && it.hasModifierProperty(PsiModifier.DEFAULT) }
}.ifEmpty { return null }
val kotlinSuperClass = generateSequence(psiClass) { it.superClass }.firstOrNull { it is KtLightClassForSourceDeclaration }
?: return signaturesFromKotlinInterfaces.first().method as? KtLightMethod
val signaturesVisibleThroughKotlinSuperClass = kotlinSuperClass.visibleSignatures
return signaturesFromKotlinInterfaces.firstOrNull { it !in signaturesVisibleThroughKotlinSuperClass }?.method as? KtLightMethod
}
private fun report(method: KtLightMethod, holder: AnnotationHolder, psiClass: PsiClass) {
val key = if (psiClass is PsiEnumConstantInitializer) "enum.constant.should.implement.method" else "class.must.be.abstract"
val message = JavaErrorMessages.message(key, HighlightUtil.formatClass(psiClass, false), JavaHighlightUtil.formatMethod(method),
HighlightUtil.formatClass(method.containingClass, false))
val errorAnnotation = holder.createErrorAnnotation(getClassDeclarationTextRange(psiClass), message)
registerFixes(errorAnnotation, psiClass)
}
private fun registerFixes(errorAnnotation: Annotation, psiClass: PsiClass) {
val quickFixFactory = QuickFixFactory.getInstance()
// this code is untested
// see com.intellij.codeInsight.daemon.impl.analysis.HighlightClassUtil.checkClassWithAbstractMethods
errorAnnotation.registerFix(quickFixFactory.createImplementMethodsFix(psiClass))
if (psiClass !is PsiAnonymousClass && !(psiClass.modifierList?.hasExplicitModifier(PsiModifier.FINAL) ?: false)) {
errorAnnotation.registerFix(quickFixFactory.createModifierListFix(psiClass, PsiModifier.ABSTRACT, true, false))
}
}
}
+2
View File
@@ -516,6 +516,8 @@
<annotator language="kotlin" implementationClass="org.jetbrains.kotlin.idea.highlighter.PlatformHeaderAnnotator"/>
<problemHighlightFilter implementation="org.jetbrains.kotlin.idea.highlighter.KotlinProblemHighlightFilter"/>
<annotator language="JAVA" implementationClass="org.jetbrains.kotlin.idea.java.UnimplementedKotlinInterfaceMemberAnnotator"/>
<extendWordSelectionHandler implementation="org.jetbrains.kotlin.idea.editor.wordSelection.KotlinStatementGroupSelectioner"/>
<extendWordSelectionHandler implementation="org.jetbrains.kotlin.idea.editor.wordSelection.KotlinCodeBlockSelectioner"/>
<extendWordSelectionHandler implementation="org.jetbrains.kotlin.idea.editor.wordSelection.KotlinDocCommentSelectioner"/>
@@ -1,5 +1,5 @@
interface <lineMarker descr="*">A</lineMarker> {
override fun <lineMarker descr="<html><body>Is implemented in <br>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>"><lineMarker descr="*">toString</lineMarker></lineMarker>() = "A"
override fun <lineMarker descr="<html><body>Is overridden in <br>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>"><lineMarker descr="*">toString</lineMarker></lineMarker>() = "A"
}
abstract class <lineMarker descr="*">B</lineMarker> : A
@@ -1,11 +1,9 @@
interface <lineMarker descr="*">A</lineMarker> {
fun <lineMarker descr="<html><body>Is implemented in <br>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>">foo</lineMarker>(): String = "A"
fun <lineMarker descr="<html><body>Is overridden in <br>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>">foo</lineMarker>(): String = "A"
// TODO: B shoudn't be mentioned
val <lineMarker descr="<html><body>Is implemented in <br/>&nbsp;&nbsp;&nbsp;&nbsp;B<br>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>">some</lineMarker>: String? get() = null
val <lineMarker descr="<html><body>Is implemented in <br/>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>">some</lineMarker>: String? get() = null
// TODO: B shoudn't be mentioned
var <lineMarker descr="<html><body>Is implemented in <br/>&nbsp;&nbsp;&nbsp;&nbsp;B<br>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>">other</lineMarker>: String?
var <lineMarker descr="<html><body>Is implemented in <br/>&nbsp;&nbsp;&nbsp;&nbsp;C</body></html>">other</lineMarker>: String?
get() = null
set(value) {}
}
@@ -4,8 +4,7 @@ interface <lineMarker descr="*">SkipSupport</lineMarker> {
}
public interface <lineMarker descr="*">SkipSupportWithDefaults</lineMarker> : SkipSupport {
// TODO: should be "Is overriden in SkipSupportImpl"
override fun <lineMarker descr="<html><body>Is implemented in <br>&nbsp;&nbsp;&nbsp;&nbsp;SkipSupportImpl</body></html>"><lineMarker descr="Implements function in 'SkipSupport'">skip</lineMarker></lineMarker>(why: String) {}
override fun <lineMarker descr="<html><body>Is overridden in <br>&nbsp;&nbsp;&nbsp;&nbsp;SkipSupportImpl</body></html>"><lineMarker descr="Implements function in 'SkipSupport'">skip</lineMarker></lineMarker>(why: String) {}
override fun <lineMarker descr="Implements function in 'SkipSupport'">skip</lineMarker>() {
skip("not given")
@@ -3,7 +3,7 @@ interface <lineMarker descr="*">SkipSupport</lineMarker> {
}
public interface <lineMarker descr="*">SkipSupportWithDefaults</lineMarker> : SkipSupport {
override fun <lineMarker descr="<html><body>Is implemented in <br>&nbsp;&nbsp;&nbsp;&nbsp;SkipSupportImpl1</body></html>"><lineMarker descr="Implements function in 'SkipSupport'">skip</lineMarker></lineMarker>() {}
override fun <lineMarker descr="<html><body>Is overridden in <br>&nbsp;&nbsp;&nbsp;&nbsp;SkipSupportImpl1</body></html>"><lineMarker descr="Implements function in 'SkipSupport'">skip</lineMarker></lineMarker>() {}
}
public interface SkipSupportImpl1 : SkipSupportWithDefaults {
@@ -0,0 +1,39 @@
package test;
public class ExtendClassWithDefaultImplementationComplext {
<error descr="Class 'Test1' must either be declared abstract or implement abstract method 'a()' in 'A'">public static class Test1 implements A</error> {
}
public static class Test2 extends AI implements A {
}
<error descr="Class 'Test3' must either be declared abstract or implement abstract method 'b()' in 'B'">public static class Test3 extends AI implements B</error> {
}
public static class Test4 extends BI implements B {
}
<error descr="Class 'Test5' must either be declared abstract or implement abstract method 'c()' in 'C'">public static class Test5 extends BI implements C</error> {
}
<error descr="Class 'Test6' must either be declared abstract or implement abstract method 'd()' in 'D'">public static class Test6 extends BI implements D</error> {
}
public static class Test7 extends BI implements S {
}
public static interface Test8 extends A {
}
public static abstract class Test9 implements A {
}
}
@@ -0,0 +1,26 @@
package test
interface A {
fun a() = Unit
}
open class AI : A
interface B : A {
fun b() = Unit
}
open class BI: B
interface C: B {
fun c() = Unit
}
interface D {
fun d() = Unit
}
interface S {
fun a() = Unit
fun b() = Unit
}
@@ -0,0 +1,14 @@
package test;
public class ExtendClassWithDefaultImplementation_1_6 {
public static class ExtendClass extends KotlinClass {
}
<error descr="Class 'ImplementInterface' must either be declared abstract or implement abstract method 'bar()' in 'KotlinInterface'">public static class ImplementInterface implements KotlinInterface</error> {
@Override
public void f() {
}
}
}
@@ -0,0 +1,20 @@
package test
interface KotlinInterface {
fun foo() {
}
fun bar() {
}
fun f()
}
abstract class KotlinClass : KotlinInterface {
override fun f() {
}
}
@@ -0,0 +1 @@
// LANGUAGE_LEVEL 1.6
@@ -0,0 +1,14 @@
package test;
public class ExtendClassWithDefaultImplementation_1_8 {
public static class ExtendClass extends KotlinClass {
}
<error descr="Class 'ImplementInterface' must either be declared abstract or implement abstract method 'bar()' in 'KotlinInterface'">public static class ImplementInterface implements KotlinInterface</error> {
@Override
public void f() {
}
}
}
@@ -0,0 +1,20 @@
package test
interface KotlinInterface {
fun foo() {
}
fun bar() {
}
fun f()
}
abstract class KotlinClass : KotlinInterface {
override fun f() {
}
}
@@ -0,0 +1 @@
// LANGUAGE_LEVEL 1.8
@@ -72,6 +72,24 @@ public class JavaAgainstKotlinBinariesCheckerTestGenerated extends AbstractJavaA
doTest(fileName);
}
@TestMetadata("ExtendClassWithDefaultImplementationComplex.kt")
public void testExtendClassWithDefaultImplementationComplex() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/ExtendClassWithDefaultImplementationComplex.kt");
doTest(fileName);
}
@TestMetadata("ExtendClassWithDefaultImplementation_1_6.kt")
public void testExtendClassWithDefaultImplementation_1_6() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/ExtendClassWithDefaultImplementation_1_6.kt");
doTest(fileName);
}
@TestMetadata("ExtendClassWithDefaultImplementation_1_8.kt")
public void testExtendClassWithDefaultImplementation_1_8() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/ExtendClassWithDefaultImplementation_1_8.kt");
doTest(fileName);
}
@TestMetadata("FunctionInNestedClassInDataFlowInspection.kt")
public void testFunctionInNestedClassInDataFlowInspection() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/FunctionInNestedClassInDataFlowInspection.kt");
@@ -74,6 +74,24 @@ public class JavaAgainstKotlinSourceCheckerTestGenerated extends AbstractJavaAga
doTest(fileName);
}
@TestMetadata("ExtendClassWithDefaultImplementationComplex.kt")
public void testExtendClassWithDefaultImplementationComplex() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/ExtendClassWithDefaultImplementationComplex.kt");
doTest(fileName);
}
@TestMetadata("ExtendClassWithDefaultImplementation_1_6.kt")
public void testExtendClassWithDefaultImplementation_1_6() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/ExtendClassWithDefaultImplementation_1_6.kt");
doTest(fileName);
}
@TestMetadata("ExtendClassWithDefaultImplementation_1_8.kt")
public void testExtendClassWithDefaultImplementation_1_8() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/ExtendClassWithDefaultImplementation_1_8.kt");
doTest(fileName);
}
@TestMetadata("FunctionInNestedClassInDataFlowInspection.kt")
public void testFunctionInNestedClassInDataFlowInspection() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/kotlinAndJavaChecker/javaAgainstKotlin/FunctionInNestedClassInDataFlowInspection.kt");
@@ -56,7 +56,8 @@ abstract class AbstractIdeLightClassTest : KotlinLightCodeInsightFixtureTestCase
myFixture.configureByFiles(*testFiles.toTypedArray())
val ktFile = myFixture.file as KtFile
testLightClass(testDataPath, { LightClassTestCommon.removeEmptyDefaultImpls(it) }) { fqName ->
val testData = File(testDataPath)
testLightClass(KotlinTestUtils.replaceExtension(testData, "java"), testData, { LightClassTestCommon.removeEmptyDefaultImpls(it) }, { fqName ->
val tracker = LightClassLazinessChecker.Tracker(fqName)
project.withServiceRegistered<StubComputationTracker, PsiClass?>(tracker) {
findClass(fqName, ktFile, project)?.apply {
@@ -65,7 +66,7 @@ abstract class AbstractIdeLightClassTest : KotlinLightCodeInsightFixtureTestCase
PsiElementChecker.checkPsiElementStructure(this)
}
}
}
})
}
private fun lazinessModeByFileText(testDataPath: String): LightClassLazinessChecker.Mode {
@@ -97,17 +98,22 @@ abstract class AbstractIdeCompiledLightClassTest : KotlinDaemonAnalyzerTestCase(
private fun libName() = "libFor" + getTestName(false)
fun doTest(testDataPath: String) {
testLightClass(testDataPath, { it }) {
val testDataFile = File(testDataPath)
val expectedFile = KotlinTestUtils.replaceExtension(
testDataFile, "compiled.java"
).let { if (it.exists()) it else KotlinTestUtils.replaceExtension(testDataFile, "java") }
testLightClass(expectedFile, testDataFile, { it }, {
findClass(it, null, project)?.apply {
PsiElementChecker.checkPsiElementStructure(this)
}
}
})
}
}
private fun testLightClass(testDataPath: String, normalize: (String) -> String, findLightClass: (String) -> PsiClass?) {
private fun testLightClass(expected: File, testData: File, normalize: (String) -> String, findLightClass: (String) -> PsiClass?) {
LightClassTestCommon.testLightClass(
File(testDataPath),
expected,
testData,
findLightClass,
normalizeText = { text ->
//NOTE: ide and compiler differ in names generated for parameters with unspecified names
@@ -116,6 +122,7 @@ private fun testLightClass(testDataPath: String, normalize: (String) -> String,
.replace("java.lang.String s)", "java.lang.String p)")
.replace("java.lang.String s1", "java.lang.String p1")
.replace("java.lang.String s2", "java.lang.String p2")
.replace("java.lang.Object o)", "java.lang.Object p)")
.removeLinesStartingWith("@" + JvmAnnotationNames.METADATA_FQ_NAME.asString())
.run(normalize)
}
@@ -243,11 +250,18 @@ object LightClassLazinessChecker {
private fun methodInfo(method: PsiMethod) = with(method) {
MethodInfo(
name, PsiModifier.MODIFIERS.asList().filter { modifierList.hasModifierProperty(it) },
name, relevantModifiers(),
isConstructor, method.parameterList.parametersCount, isVarArgs
)
}
private fun PsiMethod.relevantModifiers()
= when { containingClass!!.isInterface ->
PsiModifier.MODIFIERS.filter { it != PsiModifier.ABSTRACT && it != PsiModifier.DEFAULT }
else -> PsiModifier.MODIFIERS.asList()
}.filter { modifierList.hasModifierProperty(it) }
private fun Array<out PsiMember>.names() = mapTo(LinkedHashSet()) { it.name!! }
}
@@ -66,12 +66,24 @@ public class IdeCompiledLightClassTestGenerated extends AbstractIdeCompiledLight
doTest(fileName);
}
@TestMetadata("ExtendingInterfaceWithDefaultImpls.kt")
public void testExtendingInterfaceWithDefaultImpls() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/ExtendingInterfaceWithDefaultImpls.kt");
doTest(fileName);
}
@TestMetadata("HiddenDeprecated.kt")
public void testHiddenDeprecated() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/HiddenDeprecated.kt");
doTest(fileName);
}
@TestMetadata("InheritingInterfaceDefaultImpls.kt")
public void testInheritingInterfaceDefaultImpls() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/InheritingInterfaceDefaultImpls.kt");
doTest(fileName);
}
@TestMetadata("JvmNameOnMember.kt")
public void testJvmNameOnMember() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/JvmNameOnMember.kt");
@@ -66,12 +66,24 @@ public class IdeLightClassTestGenerated extends AbstractIdeLightClassTest {
doTest(fileName);
}
@TestMetadata("ExtendingInterfaceWithDefaultImpls.kt")
public void testExtendingInterfaceWithDefaultImpls() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/ExtendingInterfaceWithDefaultImpls.kt");
doTest(fileName);
}
@TestMetadata("HiddenDeprecated.kt")
public void testHiddenDeprecated() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/HiddenDeprecated.kt");
doTest(fileName);
}
@TestMetadata("InheritingInterfaceDefaultImpls.kt")
public void testInheritingInterfaceDefaultImpls() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/InheritingInterfaceDefaultImpls.kt");
doTest(fileName);
}
@TestMetadata("JvmNameOnMember.kt")
public void testJvmNameOnMember() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/asJava/lightClasses/JvmNameOnMember.kt");