Improve diagnostics on header/impl classes when scopes don't match

Try to report most of the errors on the actual members of the impl
class. In many cases, there's a 1:1 mapping of header to impl class
members, so the error "some members are not implemented" on the class
declaration itself is redundant. Exceptions include functions/properties
from supertypes (there may be no other place to report a signature
mismatch error in this case), functions/properties not marked with
'impl' (the checker is only run for declarations explicitly marked with
'impl') and default constructors (the checker is not run for them)

 #KT-18447 Fixed
This commit is contained in:
Alexander Udalov
2017-07-18 17:28:33 +03:00
parent 74ba0080b1
commit 9ecd04f628
17 changed files with 207 additions and 89 deletions
@@ -38,6 +38,7 @@ import org.jetbrains.kotlin.resolve.calls.inference.InferenceErrorData;
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall;
import org.jetbrains.kotlin.resolve.calls.tower.WrongResolutionToClassifier;
import org.jetbrains.kotlin.resolve.checkers.HeaderImplDeclarationChecker;
import org.jetbrains.kotlin.resolve.checkers.HeaderImplDeclarationChecker.Compatibility.Incompatible;
import org.jetbrains.kotlin.serialization.deserialization.IncompatibleVersionErrorData;
import org.jetbrains.kotlin.serialization.deserialization.descriptors.SinceKotlinInfo;
import org.jetbrains.kotlin.types.KotlinType;
@@ -560,10 +561,14 @@ public interface Errors {
DiagnosticFactory0<KtTypeAlias> IMPL_TYPE_ALIAS_WITH_COMPLEX_SUBSTITUTION = DiagnosticFactory0.create(ERROR, DECLARATION_SIGNATURE);
DiagnosticFactory3<KtDeclaration, MemberDescriptor, ModuleDescriptor,
Map<HeaderImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>> HEADER_WITHOUT_IMPLEMENTATION =
Map<Incompatible, Collection<MemberDescriptor>>> HEADER_WITHOUT_IMPLEMENTATION =
DiagnosticFactory3.create(ERROR, DECLARATION_SIGNATURE);
DiagnosticFactory2<KtDeclaration, MemberDescriptor,
Map<HeaderImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>> IMPLEMENTATION_WITHOUT_HEADER =
Map<Incompatible, Collection<MemberDescriptor>>> IMPLEMENTATION_WITHOUT_HEADER =
DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE);
DiagnosticFactory2<KtDeclaration, ClassDescriptor,
List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>> HEADER_CLASS_MEMBERS_ARE_NOT_IMPLEMENTED =
DiagnosticFactory2.create(ERROR, DECLARATION_SIGNATURE);
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
@@ -275,6 +275,9 @@ public class DefaultErrorMessages {
MAP.put(IMPLEMENTATION_WITHOUT_HEADER, "''impl'' {0} has no corresponding ''header'' declaration{1}", DECLARATION_NAME_WITH_KIND,
PlatformIncompatibilityDiagnosticRenderer.INSTANCE);
MAP.put(HEADER_CLASS_MEMBERS_ARE_NOT_IMPLEMENTED, "''impl'' class ''{0}'' has no implementation of ''header'' class members:{1}",
NAME, IncompatibleHeaderImplClassScopesRenderer.INSTANCE);
MAP.put(PROJECTION_ON_NON_CLASS_TYPE_ARGUMENT, "Projections are not allowed on type arguments of functions and properties");
MAP.put(SUPERTYPE_NOT_INITIALIZED, "This type has a constructor, and thus must be initialized here");
MAP.put(NOTHING_TO_OVERRIDE, "''{0}'' overrides nothing", NAME);
@@ -16,58 +16,82 @@
package org.jetbrains.kotlin.diagnostics.rendering
import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
import org.jetbrains.kotlin.descriptors.DeclarationDescriptor
import org.jetbrains.kotlin.descriptors.MemberDescriptor
import org.jetbrains.kotlin.resolve.checkers.HeaderImplDeclarationChecker
object PlatformIncompatibilityDiagnosticRenderer :
DiagnosticParameterRenderer<Map<HeaderImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>> {
private val INDENTATION_UNIT = " "
import org.jetbrains.kotlin.resolve.checkers.HeaderImplDeclarationChecker.Compatibility.Incompatible
object PlatformIncompatibilityDiagnosticRenderer : DiagnosticParameterRenderer<Map<Incompatible, Collection<MemberDescriptor>>> {
override fun render(
obj: Map<HeaderImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>,
obj: Map<Incompatible, Collection<MemberDescriptor>>,
renderingContext: RenderingContext
): String {
if (obj.isEmpty()) return ""
val renderDescriptor: (DeclarationDescriptor) -> String = { Renderers.COMPACT_WITH_MODIFIERS.render(it, renderingContext) }
return buildString {
appendln()
render(obj, "", renderDescriptor)
}
}
private fun StringBuilder.render(
map: Map<HeaderImplDeclarationChecker.Compatibility.Incompatible, Collection<MemberDescriptor>>,
indent: String,
renderDescriptor: (DeclarationDescriptor) -> String
) {
for ((incompatibility, descriptors) in map) {
append(indent)
append("The following declaration")
if (descriptors.size == 1) append(" is") else append("s are")
append(" incompatible")
incompatibility.reason?.let { appendln(" because $it:") }
for (descriptor in descriptors) {
append(indent + " ")
appendln(renderDescriptor(descriptor))
}
incompatibility.unimplemented?.let { unimplemented ->
append(indent)
appendln("No implementations are found for members listed below:")
for ((descriptor, mapping) in unimplemented) {
appendln()
append(indent + " ")
appendln(renderDescriptor(descriptor))
if (mapping.isNotEmpty()) {
appendln()
}
render(mapping, indent + INDENTATION_UNIT, renderDescriptor)
}
}
renderIncompatibilityInformation(obj, "", renderingContext)
}
}
}
object IncompatibleHeaderImplClassScopesRenderer :
DiagnosticParameterRenderer<List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>> {
override fun render(
obj: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>,
renderingContext: RenderingContext): String {
if (obj.isEmpty()) return ""
return buildString {
appendln()
renderIncompatibleClassScopes(obj, "", renderingContext)
}
}
}
private fun StringBuilder.renderIncompatibilityInformation(
map: Map<Incompatible, Collection<MemberDescriptor>>,
indent: String,
context: RenderingContext
) {
for ((incompatibility, descriptors) in map) {
append(indent)
append("The following declaration")
if (descriptors.size == 1) append(" is") else append("s are")
append(" incompatible")
incompatibility.reason?.let { appendln(" because $it:") }
for (descriptor in descriptors) {
append(indent + " ")
appendln(descriptor.render(context))
}
if (incompatibility is Incompatible.ClassScopes) {
append(indent)
appendln("No implementations are found for members listed below:")
renderIncompatibleClassScopes(incompatibility.unimplemented, indent, context)
}
}
}
private fun StringBuilder.renderIncompatibleClassScopes(
unimplemented: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>,
indent: String,
context: RenderingContext
) {
for ((descriptor, mapping) in unimplemented) {
appendln()
append(indent + " ")
appendln(descriptor.render(context))
if (mapping.isNotEmpty()) {
appendln()
}
renderIncompatibilityInformation(mapping, indent + INDENTATION_UNIT, context)
}
}
private const val INDENTATION_UNIT = " "
private fun DeclarationDescriptor.render(context: RenderingContext): String {
return Renderers.COMPACT_WITH_MODIFIERS.render(this, context)
}
@@ -26,6 +26,7 @@ import org.jetbrains.kotlin.diagnostics.Errors
import org.jetbrains.kotlin.incremental.components.NoLookupLocation
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.KtConstructor
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.resolve.BindingContext
@@ -102,7 +103,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
return when (header) {
is CallableMemberDescriptor -> {
header.findNamesakesFromModule(platformModule).filter { impl ->
header != impl &&
header != impl && !impl.isHeader &&
// TODO: support non-source definitions (e.g. from Java)
DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement
}.groupBy { impl ->
@@ -111,7 +112,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
}
is ClassDescriptor -> {
header.findClassifiersFromModule(platformModule).filter { impl ->
header != impl &&
header != impl && !impl.isHeader &&
DescriptorToSourceUtils.getSourceFromDescriptor(impl) is KtElement
}.groupBy { impl ->
areCompatibleClassifiers(header, impl, checkImpl)
@@ -129,7 +130,35 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
// TODO: use common module here
val compatibility = findHeaderForImpl(descriptor, descriptor.module) ?: return
if (Compatible !in compatibility) {
// 'firstOrNull' is needed because in diagnostic tests, common sources appear twice, so the same class is duplicated
// TODO: replace with 'singleOrNull' as soon as multi-module diagnostic tests are refactored
val singleIncompatibility = compatibility.keys.firstOrNull()
if (singleIncompatibility is Incompatible.ClassScopes) {
assert(descriptor is ClassDescriptor) { "Incompatible.ClassScopes is only possible for a class: $descriptor" }
// Do not report "header members are not implemented" for those header members, for which there's a clear
// (albeit maybe incompatible) single implementation suspect, declared in the impl class.
// This is needed only to reduce the number of errors. Incompatibility errors for those members will be reported
// later when this checker is called for them
fun hasSingleImplSuspect(
headerWithIncompatibility: Pair<MemberDescriptor, Map<Incompatible, Collection<MemberDescriptor>>>
): Boolean {
val (headerMember, incompatibility) = headerWithIncompatibility
val implMember = incompatibility.values.singleOrNull()?.singleOrNull()
return implMember != null &&
implMember.isExplicitImplDeclaration() &&
findHeaderForImpl(implMember, headerMember.module)?.values?.singleOrNull()?.singleOrNull() == headerMember
}
val nonTrivialUnimplemented = singleIncompatibility.unimplemented.filterNot(::hasSingleImplSuspect)
if (nonTrivialUnimplemented.isNotEmpty()) {
diagnosticHolder.report(Errors.HEADER_CLASS_MEMBERS_ARE_NOT_IMPLEMENTED.on(
reportOn, descriptor as ClassDescriptor, nonTrivialUnimplemented
))
}
}
else if (Compatible !in compatibility) {
assert(compatibility.keys.all { it is Incompatible })
@Suppress("UNCHECKED_CAST")
val incompatibility = compatibility as Map<Incompatible, Collection<MemberDescriptor>>
@@ -137,13 +166,24 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
}
}
// This should ideally be handled by CallableMemberDescriptor.Kind, but default constructors have kind DECLARATION and non-empty source.
// Their source is the containing KtClass instance though, as opposed to explicit constructors, whose source is KtConstructor
private fun CallableMemberDescriptor.isExplicitImplDeclaration(): Boolean =
if (this is ConstructorDescriptor) {
DescriptorToSourceUtils.getSourceFromDescriptor(this) is KtConstructor<*>
}
else {
isImpl && kind == CallableMemberDescriptor.Kind.DECLARATION
}
private fun findHeaderForImpl(impl: MemberDescriptor, commonModule: ModuleDescriptor): Map<Compatibility, List<MemberDescriptor>>? {
return when (impl) {
is CallableMemberDescriptor -> {
val container = impl.containingDeclaration
val candidates = when (container) {
is ClassDescriptor -> {
val headerClass = findHeaderForImpl(container, commonModule)?.get(Compatible)?.firstOrNull() as? ClassDescriptor
// TODO: replace with 'singleOrNull' as soon as multi-module diagnostic tests are refactored
val headerClass = findHeaderForImpl(container, commonModule)?.values?.firstOrNull()?.firstOrNull() as? ClassDescriptor
headerClass?.getMembers(impl.name).orEmpty()
}
is PackageFragmentDescriptor -> impl.findNamesakesFromModule(commonModule)
@@ -169,7 +209,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
impl != declaration &&
declaration is ClassDescriptor && declaration.isHeader
}.groupBy { header ->
areCompatibleClassifiers(header as ClassDescriptor, impl, checkImpl = false)
areCompatibleClassifiers(header as ClassDescriptor, impl, checkImpl = true)
}
}
else -> null
@@ -229,10 +269,7 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
sealed class Compatibility {
// Note that the reason is used in the diagnostic output, see PlatformIncompatibilityDiagnosticRenderer
sealed class Incompatible(
val reason: String?,
val unimplemented: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>? = null
) : Compatibility() {
sealed class Incompatible(val reason: String?) : Compatibility() {
// Callables
object ParameterShape : Incompatible("parameter shapes are different (extension vs non-extension)")
@@ -268,8 +305,8 @@ object HeaderImplDeclarationChecker : DeclarationChecker {
object Supertypes : Incompatible("some supertypes are missing in the implementation")
class ClassScopes(
unimplemented: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>
) : Incompatible("some members are not implemented", unimplemented)
val unimplemented: List<Pair<CallableMemberDescriptor, Map<Incompatible, Collection<CallableMemberDescriptor>>>>
) : Incompatible("some members are not implemented")
object EnumEntries : Incompatible("some entries from header enum are missing in the impl enum")
@@ -0,0 +1,15 @@
// !LANGUAGE: +MultiPlatformProjects
// MODULE: m1-common
// FILE: common.kt
header class Foo {
fun bar(): String
}
// MODULE: m2-jvm(m1-common)
// FILE: jvm.kt
// TODO: run HeaderImplDeclarationChecker on non-impl members of impl classes, and report something like "impl expected" on 'bar' instead
impl class <!HEADER_CLASS_MEMBERS_ARE_NOT_IMPLEMENTED!>Foo<!> {
fun bar(): String = "bar"
}
@@ -0,0 +1,22 @@
// -- Module: <m1-common> --
package
public final header class Foo {
public constructor Foo()
public final header fun bar(): kotlin.String
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
}
// -- Module: <m2-jvm> --
package
public final impl class Foo {
public constructor Foo()
public final fun bar(): kotlin.String
public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean
public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int
public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String
}
@@ -5,18 +5,9 @@ Output:
-- JVM --
Exit code: COMPILATION_ERROR
Output:
compiler/testData/multiplatform/classScopes/constructorIncorrectSignature/jvm.kt:1:12: error: 'impl' class 'Foo' has no corresponding 'header' declaration
The following declaration is incompatible because some members are not implemented:
public final header class Foo
No implementations are found for members listed below:
compiler/testData/multiplatform/classScopes/constructorIncorrectSignature/jvm.kt:2:10: error: 'impl' constructor of 'Foo' has no corresponding 'header' declaration
The following declaration is incompatible because parameter types are different:
public constructor Foo(s: String)
The following declaration is incompatible because parameter types are different:
public constructor Foo(s: Array<String>)
impl class Foo {
^
compiler/testData/multiplatform/classScopes/constructorIncorrectSignature/jvm.kt:2:10: error: 'impl' constructor of 'Foo' has no corresponding 'header' declaration
impl constructor(s: Array<String>)
^
@@ -5,18 +5,9 @@ Output:
-- JVM --
Exit code: COMPILATION_ERROR
Output:
compiler/testData/multiplatform/classScopes/functionIncorrectSignature/jvm.kt:1:12: error: 'impl' class 'Foo' has no corresponding 'header' declaration
The following declaration is incompatible because some members are not implemented:
public final header class Foo
No implementations are found for members listed below:
compiler/testData/multiplatform/classScopes/functionIncorrectSignature/jvm.kt:2:5: error: 'impl' function 'function' has no corresponding 'header' declaration
The following declaration is incompatible because return type is different:
public final header fun function(b: ByteArray): Int
The following declaration is incompatible because return type is different:
public final impl fun function(b: ByteArray): Long
impl class Foo {
^
compiler/testData/multiplatform/classScopes/functionIncorrectSignature/jvm.kt:2:5: error: 'impl' function 'function' has no corresponding 'header' declaration
impl fun function(b: ByteArray): Long = b.size.toLong()
^
@@ -0,0 +1,3 @@
header class Foo {
fun function(b: ByteArray): Int
}
@@ -0,0 +1,5 @@
open class Base {
fun function(b: ByteArray): Long = b.size.toLong()
}
impl class Foo : Base()
@@ -0,0 +1,16 @@
-- Common --
Exit code: OK
Output:
-- JVM --
Exit code: COMPILATION_ERROR
Output:
compiler/testData/multiplatform/classScopes/functionIncorrectSignatureFromSuperclass/jvm.kt:5:12: error: 'impl' class 'Foo' has no implementation of 'header' class members:
public final header fun function(b: ByteArray): Int
The following declaration is incompatible because return type is different:
public final fun function(b: ByteArray): Long
impl class Foo : Base()
^
@@ -5,10 +5,7 @@ Output:
-- JVM --
Exit code: COMPILATION_ERROR
Output:
compiler/testData/multiplatform/classScopes/missingConstructor/jvm.kt:1:12: error: 'impl' class 'Foo' has no corresponding 'header' declaration
The following declaration is incompatible because some members are not implemented:
public final header class Foo
No implementations are found for members listed below:
compiler/testData/multiplatform/classScopes/missingConstructor/jvm.kt:1:12: error: 'impl' class 'Foo' has no implementation of 'header' class members:
public constructor Foo(s: String)
@@ -5,10 +5,7 @@ Output:
-- JVM --
Exit code: COMPILATION_ERROR
Output:
compiler/testData/multiplatform/classScopes/missingFunction/jvm.kt:1:12: error: 'impl' class 'Foo' has no corresponding 'header' declaration
The following declaration is incompatible because some members are not implemented:
public final header class Foo
No implementations are found for members listed below:
compiler/testData/multiplatform/classScopes/missingFunction/jvm.kt:1:12: error: 'impl' class 'Foo' has no implementation of 'header' class members:
public final header fun function(s: String): Unit
+1 -1
View File
@@ -11,7 +11,7 @@ The following declaration is incompatible because parameter types are different:
header fun g(s: String)
^
compiler/testData/multiplatform/missingOverload/jvm.kt:1:12: error: 'impl' class 'Foo' has no implementations of 'header' class members:
compiler/testData/multiplatform/missingOverload/jvm.kt:1:12: error: 'impl' class 'Foo' has no implementation of 'header' class members:
public final header fun f(a: Any): Unit
@@ -13770,6 +13770,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
doTest(fileName);
}
@TestMetadata("noImplKeywordOnMember.kt")
public void testNoImplKeywordOnMember() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/multiplatform/headerClass/noImplKeywordOnMember.kt");
doTest(fileName);
}
@TestMetadata("simpleHeaderClass.kt")
public void testSimpleHeaderClass() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/multiplatform/headerClass/simpleHeaderClass.kt");
@@ -140,6 +140,12 @@ public class MultiPlatformIntegrationTestGenerated extends AbstractMultiPlatform
doTest(fileName);
}
@TestMetadata("functionIncorrectSignatureFromSuperclass")
public void testFunctionIncorrectSignatureFromSuperclass() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/functionIncorrectSignatureFromSuperclass/");
doTest(fileName);
}
@TestMetadata("missingConstructor")
public void testMissingConstructor() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/multiplatform/classScopes/missingConstructor/");
@@ -1,11 +1,11 @@
impl class <error>My</error> {
<error>impl fun foo()</error> = 42
impl fun foo() = 42
}
impl class <error>Your</error> {
impl class Your {
<error>impl fun foo()</error> = 13
impl fun foo() = 13
<error>impl fun bar(arg: Int)</error> = arg